From d601b4aab0e39a8e503f041d72bd805d5bc11a5a Mon Sep 17 00:00:00 2001 From: shawnhill Date: Tue, 4 Aug 2020 17:18:13 -0600 Subject: [PATCH 1/5] Add config params for websocket ping_timeout & ping_interval --- docs/sanic/config.rst | 4 ++++ docs/sanic/websocket.rst | 1 + sanic/config.py | 2 ++ sanic/websocket.py | 6 ++++++ 4 files changed, 13 insertions(+) diff --git a/docs/sanic/config.rst b/docs/sanic/config.rst index 4c24ddc4ad..0f78f63b94 100644 --- a/docs/sanic/config.rst +++ b/docs/sanic/config.rst @@ -123,6 +123,10 @@ Out of the box there are just a few predefined values which can be overwritten w +---------------------------+-------------------+-----------------------------------------------------------------------------+ | WEBSOCKET_WRITE_LIMIT | 2^16 | High-water limit of the buffer for outgoing bytes | +---------------------------+-------------------+-----------------------------------------------------------------------------+ +| WEBSOCKET_PING_INTERVAL | 20 | A Ping frame is sent every ping_interval seconds. | ++---------------------------+-------------------+-----------------------------------------------------------------------------+ +| WEBSOCKET_PING_TIMEOUT | 20 | Connection is closed when Pong is not received after ping_timeout seconds | ++---------------------------+-------------------+-----------------------------------------------------------------------------+ | GRACEFUL_SHUTDOWN_TIMEOUT | 15.0 | How long to wait to force close non-idle connection (sec) | +---------------------------+-------------------+-----------------------------------------------------------------------------+ | ACCESS_LOG | True | Disable or enable access log | diff --git a/docs/sanic/websocket.rst b/docs/sanic/websocket.rst index 3421e7346b..3a721aba83 100644 --- a/docs/sanic/websocket.rst +++ b/docs/sanic/websocket.rst @@ -51,5 +51,6 @@ You could setup your own WebSocket configuration through ``app.config``, like app.config.WEBSOCKET_MAX_QUEUE = 32 app.config.WEBSOCKET_READ_LIMIT = 2 ** 16 app.config.WEBSOCKET_WRITE_LIMIT = 2 ** 16 + app.config.WEBSOCKET_PING_INTERVAL = 20 Find more in ``Configuration`` section. diff --git a/sanic/config.py b/sanic/config.py index 307c7b5f48..3fbc157f0b 100644 --- a/sanic/config.py +++ b/sanic/config.py @@ -24,6 +24,8 @@ "WEBSOCKET_MAX_QUEUE": 32, "WEBSOCKET_READ_LIMIT": 2 ** 16, "WEBSOCKET_WRITE_LIMIT": 2 ** 16, + "WEBSOCKET_PING_TIMEOUT": 20, + "WEBSOCKET_PING_INTERVAL": 20, "GRACEFUL_SHUTDOWN_TIMEOUT": 15.0, # 15 sec "ACCESS_LOG": True, "FORWARDED_SECRET": None, diff --git a/sanic/websocket.py b/sanic/websocket.py index 9443b70429..0c7289082f 100644 --- a/sanic/websocket.py +++ b/sanic/websocket.py @@ -35,6 +35,8 @@ def __init__( websocket_max_queue=None, websocket_read_limit=2 ** 16, websocket_write_limit=2 ** 16, + websocket_ping_interval=20, + websocket_ping_timeout=20, **kwargs ): super().__init__(*args, **kwargs) @@ -45,6 +47,8 @@ def __init__( self.websocket_max_queue = websocket_max_queue self.websocket_read_limit = websocket_read_limit self.websocket_write_limit = websocket_write_limit + self.websocket_ping_interval = websocket_ping_interval + self.websocket_ping_timeout = websocket_ping_timeout # timeouts make no sense for websocket routes def request_timeout_callback(self): @@ -119,6 +123,8 @@ async def websocket_handshake(self, request, subprotocols=None): max_queue=self.websocket_max_queue, read_limit=self.websocket_read_limit, write_limit=self.websocket_write_limit, + ping_interval=self.websocket_ping_interval, + ping_timeout=self.websocket_ping_timeout, ) # Following two lines are required for websockets 8.x self.websocket.is_client = False From b820c61edf90fa424a8f38638bf13f6affa1a9e4 Mon Sep 17 00:00:00 2001 From: shawnhill Date: Tue, 4 Aug 2020 17:55:15 -0600 Subject: [PATCH 2/5] Include changelog --- changelogs/1904.feature.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/1904.feature.rst diff --git a/changelogs/1904.feature.rst b/changelogs/1904.feature.rst new file mode 100644 index 0000000000..2a668498d9 --- /dev/null +++ b/changelogs/1904.feature.rst @@ -0,0 +1,3 @@ +Adds WEBSOCKET_PING_TIMEOUT and WEBSOCKET_PING_INTERVAL configuration values + +Allows setting the ping_interval and ping_timeout arguments when initializing `WebSocketCommonProtocol`. \ No newline at end of file From cfa83593783dcd08d41bfe7f9f966b42b3172de5 Mon Sep 17 00:00:00 2001 From: shawnhill Date: Wed, 5 Aug 2020 15:21:11 -0600 Subject: [PATCH 3/5] Pass websocket config values to WebSocketProtocol init, test --- sanic/server.py | 18 +++++++++++++++++- tests/test_app.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/sanic/server.py b/sanic/server.py index 6adf5dde2f..234f50efd3 100644 --- a/sanic/server.py +++ b/sanic/server.py @@ -14,11 +14,13 @@ from signal import SIG_IGN, SIGINT, SIGTERM, Signals from signal import signal as signal_func from time import time +from typing import Type from httptools import HttpRequestParser # type: ignore from httptools.parser.errors import HttpParserError # type: ignore from sanic.compat import Header, ctrlc_workaround_for_windows +from sanic.config import Config from sanic.exceptions import ( HeaderExpectationFailed, InvalidUsage, @@ -31,7 +33,6 @@ from sanic.request import EXPECT_HEADER, Request, StreamBuffer from sanic.response import HTTPResponse - try: import uvloop # type: ignore @@ -844,6 +845,7 @@ def serve( app.asgi = False connections = connections if connections is not None else set() + protocol_kwargs = _build_protocol_parameters(protocol, app.config) server = partial( protocol, loop=loop, @@ -852,6 +854,7 @@ def serve( app=app, state=state, unix=unix, + **protocol_kwargs ) asyncio_server_kwargs = ( asyncio_server_kwargs if asyncio_server_kwargs else {} @@ -948,6 +951,19 @@ def serve( remove_unix_socket(unix) +def _build_protocol_parameters(protocol: Type[HttpProtocol], config: Config) -> dict: + if hasattr(protocol, "websocket_timeout"): + return { + "max_size": config.WEBSOCKET_MAX_SIZE, + "max_queue": config.WEBSOCKET_MAX_QUEUE, + "read_limit": config.WEBSOCKET_READ_LIMIT, + "write_limit": config.WEBSOCKET_WRITE_LIMIT, + "ping_timeout": config.WEBSOCKET_PING_TIMEOUT, + "ping_interval": config.WEBSOCKET_PING_INTERVAL, + } + return {} + + def bind_socket(host: str, port: int, *, backlog=100) -> socket.socket: """Create TCP server socket. :param host: IPv4, IPv6 or hostname may be specified diff --git a/tests/test_app.py b/tests/test_app.py index c7791394a9..6e1d28ff6f 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1,6 +1,7 @@ import asyncio import logging import sys +from unittest.mock import patch from inspect import isawaitable @@ -148,6 +149,35 @@ async def handler(request, ws): assert app.websocket_enabled == True +@patch("sanic.app.WebSocketProtocol") +def test_app_websocket_parameters(websocket_protocol_mock, app): + app.config.WEBSOCKET_MAX_SIZE = 44 + app.config.WEBSOCKET_MAX_QUEUE = 45 + app.config.WEBSOCKET_READ_LIMIT = 46 + app.config.WEBSOCKET_WRITE_LIMIT = 47 + app.config.WEBSOCKET_PING_TIMEOUT = 48 + app.config.WEBSOCKET_PING_INTERVAL = 50 + + @app.websocket("/ws") + async def handler(request, ws): + await ws.send("test") + + try: + # This will fail because WebSocketProtocol is mocked and only the call kwargs matter + app.test_client.get("/ws") + except: + pass + + websocket_protocol_call_args = websocket_protocol_mock.call_args + ws_kwargs = websocket_protocol_call_args[1] + assert ws_kwargs["max_size"] == app.config.WEBSOCKET_MAX_SIZE + assert ws_kwargs["max_queue"] == app.config.WEBSOCKET_MAX_QUEUE + assert ws_kwargs["read_limit"] == app.config.WEBSOCKET_READ_LIMIT + assert ws_kwargs["write_limit"] == app.config.WEBSOCKET_WRITE_LIMIT + assert ws_kwargs["ping_timeout"] == app.config.WEBSOCKET_PING_TIMEOUT + assert ws_kwargs["ping_interval"] == app.config.WEBSOCKET_PING_INTERVAL + + def test_handle_request_with_nested_exception(app, monkeypatch): err_msg = "Mock Exception" From d80c998ada655f3ce50d093f1f3e60fdae445fac Mon Sep 17 00:00:00 2001 From: shawnhill Date: Wed, 5 Aug 2020 17:18:43 -0600 Subject: [PATCH 4/5] Linting --- sanic/server.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sanic/server.py b/sanic/server.py index 234f50efd3..c4e08e76e8 100644 --- a/sanic/server.py +++ b/sanic/server.py @@ -33,6 +33,7 @@ from sanic.request import EXPECT_HEADER, Request, StreamBuffer from sanic.response import HTTPResponse + try: import uvloop # type: ignore @@ -845,7 +846,7 @@ def serve( app.asgi = False connections = connections if connections is not None else set() - protocol_kwargs = _build_protocol_parameters(protocol, app.config) + protocol_kwargs = _build_protocol_kwargs(protocol, app.config) server = partial( protocol, loop=loop, @@ -854,7 +855,7 @@ def serve( app=app, state=state, unix=unix, - **protocol_kwargs + **protocol_kwargs, ) asyncio_server_kwargs = ( asyncio_server_kwargs if asyncio_server_kwargs else {} @@ -951,7 +952,9 @@ def serve( remove_unix_socket(unix) -def _build_protocol_parameters(protocol: Type[HttpProtocol], config: Config) -> dict: +def _build_protocol_kwargs( + protocol: Type[HttpProtocol], config: Config +) -> dict: if hasattr(protocol, "websocket_timeout"): return { "max_size": config.WEBSOCKET_MAX_SIZE, From bcc13178ea96c8fd587abeaa0bedd290db7b0820 Mon Sep 17 00:00:00 2001 From: shawnhill Date: Thu, 6 Aug 2020 11:39:06 -0600 Subject: [PATCH 5/5] Improve docs --- docs/sanic/config.rst | 2 +- docs/sanic/websocket.rst | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/sanic/config.rst b/docs/sanic/config.rst index 0f78f63b94..a5d1baf7fc 100644 --- a/docs/sanic/config.rst +++ b/docs/sanic/config.rst @@ -98,7 +98,7 @@ The config files are regular Python files which are executed in order to load th Builtin Configuration Values ---------------------------- -Out of the box there are just a few predefined values which can be overwritten when creating the application. +Out of the box there are just a few predefined values which can be overwritten when creating the application. Note that websocket configuration values will have no impact if running in ASGI mode. +---------------------------+-------------------+-----------------------------------------------------------------------------+ | Variable | Default | Description | diff --git a/docs/sanic/websocket.rst b/docs/sanic/websocket.rst index 3a721aba83..bff53ef6a7 100644 --- a/docs/sanic/websocket.rst +++ b/docs/sanic/websocket.rst @@ -52,5 +52,8 @@ You could setup your own WebSocket configuration through ``app.config``, like app.config.WEBSOCKET_READ_LIMIT = 2 ** 16 app.config.WEBSOCKET_WRITE_LIMIT = 2 ** 16 app.config.WEBSOCKET_PING_INTERVAL = 20 + app.config.WEBSOCKET_PING_TIMEOUT = 20 + +These settings will have no impact if running in ASGI mode. Find more in ``Configuration`` section.