Repository: aaugustin/websockets
Pull request #474 (2018-09-23 14:09:50)

Rename timeout to close_timeout.


Patch:
diff --git a/docs/api.rst b/docs/api.rst
index cefa89b..39b5922 100644
--- a/docs/api.rst
+++ b/docs/api.rst
@@ -32,9 +32,9 @@ Server
 
 .. automodule:: websockets.server
 
-   .. autofunction:: serve(ws_handler, host=None, port=None, *, create_protocol=None, ping_interval=20, ping_timeout=20, timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origins=None, extensions=None, subprotocols=None, extra_headers=None, compression='deflate', **kwds)
+   .. autofunction:: serve(ws_handler, host=None, port=None, *, create_protocol=None, ping_interval=20, ping_timeout=20, close_timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origins=None, extensions=None, subprotocols=None, extra_headers=None, compression='deflate', **kwds)
 
-   .. autofunction:: unix_serve(ws_handler, path, *, create_protocol=None, ping_interval=20, ping_timeout=20, timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origins=None, extensions=None, subprotocols=None, extra_headers=None, compression='deflate', **kwds)
+   .. autofunction:: unix_serve(ws_handler, path, *, create_protocol=None, ping_interval=20, ping_timeout=20, close_timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origins=None, extensions=None, subprotocols=None, extra_headers=None, compression='deflate', **kwds)
 
 
    .. autoclass:: WebSocketServer
@@ -43,7 +43,7 @@ Server
         .. automethod:: wait_closed()
         .. autoattribute:: sockets
 
-   .. autoclass:: WebSocketServerProtocol(ws_handler, ws_server, *, host=None, port=None, secure=None, ping_interval=20, ping_timeout=20, timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origins=None, extensions=None, subprotocols=None, extra_headers=None)
+   .. autoclass:: WebSocketServerProtocol(ws_handler, ws_server, *, host=None, port=None, secure=None, ping_interval=20, ping_timeout=20, close_timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origins=None, extensions=None, subprotocols=None, extra_headers=None)
 
         .. automethod:: handshake(origins=None, available_extensions=None, available_subprotocols=None, extra_headers=None)
         .. automethod:: process_request(path, request_headers)
@@ -54,9 +54,9 @@ Client
 
 .. automodule:: websockets.client
 
-   .. autofunction:: connect(uri, *, create_protocol=None, ping_interval=20, ping_timeout=20, timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origin=None, extensions=None, subprotocols=None, extra_headers=None, compression='deflate', **kwds)
+   .. autofunction:: connect(uri, *, create_protocol=None, ping_interval=20, ping_timeout=20, close_timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origin=None, extensions=None, subprotocols=None, extra_headers=None, compression='deflate', **kwds)
 
-   .. autoclass:: WebSocketClientProtocol(*, host=None, port=None, secure=None, ping_interval=20, ping_timeout=20, timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origin=None, extensions=None, subprotocols=None, extra_headers=None)
+   .. autoclass:: WebSocketClientProtocol(*, host=None, port=None, secure=None, ping_interval=20, ping_timeout=20, close_timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None, origin=None, extensions=None, subprotocols=None, extra_headers=None)
 
         .. automethod:: handshake(wsuri, origin=None, available_extensions=None, available_subprotocols=None, extra_headers=None)
 
@@ -65,7 +65,7 @@ Shared
 
 .. automodule:: websockets.protocol
 
-   .. autoclass:: WebSocketCommonProtocol(*, host=None, port=None, secure=None, ping_interval=20, ping_timeout=20, timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None)
+   .. autoclass:: WebSocketCommonProtocol(*, host=None, port=None, secure=None, ping_interval=20, ping_timeout=20, close_timeout=10, max_size=2 ** 20, max_queue=2 ** 5, read_limit=2 ** 16, write_limit=2 ** 16, loop=None)
 
         .. automethod:: close(code=1000, reason='')
         .. automethod:: wait_closed(code=1000, reason='')
diff --git a/docs/changelog.rst b/docs/changelog.rst
index a3d7448..f04054b 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -10,8 +10,18 @@ Changelog
 
 .. warning::
 
-  **Version 7.0 changes how a :meth:`~protocol.WebSocketCommonProtocol.ping`
-  that hasn't received a pong yet behaves when the connection is closed.**
+  **Version 7.0 renames the** ``timeout`` **argument of**
+  :func:`~server.serve()` **and** :func:`~client.connect()` **to**
+  ``close_timeout`` **.**
+
+  This prevents confusion with ``ping_timeout``.
+
+  For backwards compatibility, ``timeout`` is still supported.
+
+.. warning::
+
+  **Version 7.0 changes how a** :meth:`~protocol.WebSocketCommonProtocol.ping`
+  **that hasn't received a pong yet behaves when the connection is closed.**
 
   The ping — as in ``ping = await websocket.ping()`` — used to be canceled
   when the connection is closed, so that ``await ping`` raised
diff --git a/docs/conf.py b/docs/conf.py
index 04db46c..8fdf12b 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -16,7 +16,7 @@ import sys, os, datetime
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
-sys.path.insert(0, os.path.abspath('..'))
+sys.path.insert(0, os.path.join(os.path.abspath('..'), 'src'))
 
 # -- General configuration -----------------------------------------------------
 
diff --git a/docs/design.rst b/docs/design.rst
index 7ce40db..9386973 100644
--- a/docs/design.rst
+++ b/docs/design.rst
@@ -271,11 +271,12 @@ state and sends a close frame. When the other side sends a close frame,
 ``CLOSING`` state and returns ``None``, also causing
 :attr:`~protocol.WebSocketCommonProtocol.transfer_data_task` to terminate.
 
-If the other side doesn't send a close frame within the connection's timeout,
-``websockets`` :ref:`fails the connection <connection-failure>`.
+If the other side doesn't send a close frame within the connection's close
+timeout, ``websockets`` :ref:`fails the connection <connection-failure>`.
 
-The closing handshake can take up to ``2 * timeout``: one ``timeout`` to write
-a close frame and one ``timeout`` to receive a close frame.
+The closing handshake can take up to ``2 * close_timeout``: one
+``close_timeout`` to write a close frame and one ``close_timeout`` to receive
+a close frame.
 
 Then ``websockets`` terminates the TCP connection.
 
@@ -313,13 +314,13 @@ Then :attr:`~protocol.WebSocketCommonProtocol.close_connection_task` cancels
 protocol compliance responsibilities. Terminating it to avoid leaking it is
 the only concern.
 
-Terminating the TCP connection can take up to ``2 * timeout`` on the server
-side and ``3 * timeout`` on the client side. Clients start by waiting for the
-server to close the connection, hence the extra ``timeout``. Then both sides
-go through the following steps until the TCP connection is lost: half-closing
-the connection (only for non-TLS connections), closing the connection,
-aborting the connection. At this point the connection drops regardless of what
-happens on the network.
+Terminating the TCP connection can take up to ``2 * close_timeout`` on the
+server side and ``3 * close_timeout`` on the client side. Clients start by
+waiting for the server to close the connection, hence the extra
+``close_timeout``. Then both sides go through the following steps until the
+TCP connection is lost: half-closing the connection (only for non-TLS
+connections), closing the connection, aborting the connection. At this point
+the connection drops regardless of what happens on the network.
 
 
 .. _connection-failure:
diff --git a/src/websockets/client.py b/src/websockets/client.py
index 94c7617..b1b14bc 100644
--- a/src/websockets/client.py
+++ b/src/websockets/client.py
@@ -318,7 +318,7 @@ class Connect:
     a ``wss://`` URI, if this argument isn't provided explicitly, it's set to
     ``True``, which means Python's default :class:`~ssl.SSLContext` is used.
 
-    The behavior of the ``ping_interval``, ``ping_timeout``, ``timeout``,
+    The behavior of the ``ping_interval``, ``ping_timeout``, ``close_timeout``,
     ``max_size``, ``max_queue``, ``read_limit``, and ``write_limit`` optional
     arguments is described in the documentation of
     :class:`~websockets.protocol.WebSocketCommonProtocol`.
@@ -357,14 +357,15 @@ class Connect:
         create_protocol=None,
         ping_interval=20,
         ping_timeout=20,
-        timeout=10,
+        close_timeout=None,
         max_size=2 ** 20,
         max_queue=2 ** 5,
         read_limit=2 ** 16,
         write_limit=2 ** 16,
         loop=None,
         legacy_recv=False,
-        klass=None,
+        klass=WebSocketClientProtocol,
+        timeout=10,
         origin=None,
         extensions=None,
         subprotocols=None,
@@ -375,14 +376,16 @@ class Connect:
         if loop is None:
             loop = asyncio.get_event_loop()
 
+        # Backwards-compatibility: close_timeout used to be called timeout.
+        # If both are specified, timeout is ignored.
+        if close_timeout is None:
+            close_timeout = timeout
+
         # Backwards-compatibility: create_protocol used to be called klass.
-        # In the unlikely event that both are specified, klass is ignored.
+        # If both are specified, klass is ignored.
         if create_protocol is None:
             create_protocol = klass
 
-        if create_protocol is None:
-            create_protocol = WebSocketClientProtocol
-
         wsuri = parse_uri(uri)
         if wsuri.secure:
             kwds.setdefault('ssl', True)
@@ -411,7 +414,7 @@ class Connect:
             secure=wsuri.secure,
             ping_interval=ping_interval,
             ping_timeout=ping_timeout,
-            timeout=timeout,
+            close_timeout=close_timeout,
             max_size=max_size,
             max_queue=max_queue,
             read_limit=read_limit,
diff --git a/src/websockets/protocol.py b/src/websockets/protocol.py
index f59ddd7..d95c26b 100644
--- a/src/websockets/protocol.py
+++ b/src/websockets/protocol.py
@@ -96,13 +96,13 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
 
     .. _Pong frame: https://tools.ietf.org/html/rfc6455#section-5.5.3
 
-    The ``timeout`` parameter defines a maximum wait time in seconds for
+    The ``close_timeout`` parameter defines a maximum wait time in seconds for
     completing the closing handshake and terminating the TCP connection.
-    :meth:`close()` completes in at most ``4 * timeout`` on the server side
-    and ``5 * timeout`` on the client side.
+    :meth:`close()` completes in at most ``4 * close_timeout`` on the server
+    side and ``5 * close_timeout`` on the client side.
 
-    ``timeout`` needs to be a parameter of the protocol because websockets
-    usually calls :meth:`close()` implicitly:
+    ``close_timeout`` needs to be a parameter of the protocol because
+    websockets usually calls :meth:`close()` implicitly:
 
     - on the server side, when the connection handler terminates,
     - on the client side, when exiting the context manager for the connection.
@@ -173,20 +173,26 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
         secure=None,
         ping_interval=20,
         ping_timeout=20,
-        timeout=10,
+        close_timeout=None,
         max_size=2 ** 20,
         max_queue=2 ** 5,
         read_limit=2 ** 16,
         write_limit=2 ** 16,
         loop=None,
-        legacy_recv=False
+        legacy_recv=False,
+        timeout=10
     ):
+        # Backwards-compatibility: close_timeout used to be called timeout.
+        # If both are specified, timeout is ignored.
+        if close_timeout is None:
+            close_timeout = timeout
+
         self.host = host
         self.port = port
         self.secure = secure
         self.ping_interval = ping_interval
         self.ping_timeout = ping_timeout
-        self.timeout = timeout
+        self.close_timeout = close_timeout
         self.max_size = max_size
         self.max_queue = max_queue
         self.read_limit = read_limit
@@ -458,7 +464,7 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
         try:
             yield from asyncio.wait_for(
                 self.write_close_frame(serialize_close(code, reason)),
-                self.timeout,
+                self.close_timeout,
                 loop=self.loop,
             )
         except asyncio.TimeoutError:
@@ -480,7 +486,7 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
             # is canceled before the timeout elapses (on Python ≥ 3.4.3).
             # This helps closing connections when shutting down a server.
             yield from asyncio.wait_for(
-                self.transfer_data_task, self.timeout, loop=self.loop
+                self.transfer_data_task, self.close_timeout, loop=self.loop
             )
         except (asyncio.TimeoutError, asyncio.CancelledError):
             pass
@@ -574,9 +580,9 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
         if self.state is State.CLOSING:
             # If we started the closing handshake, wait for its completion to
             # get the proper close code and status. self.close_connection_task
-            # will complete within 4 or 5 * timeout after calling close().
-            # The CLOSING state also occurs when failing the connection. In
-            # that case self.close_connection_task will complete even faster.
+            # will complete within 4 or 5 * close_timeout after close(). The
+            # CLOSING state also occurs when failing the connection. In that
+            # case self.close_connection_task will complete even faster.
             if self.close_code is None:
                 yield from asyncio.shield(self.close_connection_task)
             raise ConnectionClosed(
@@ -975,7 +981,7 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
     @asyncio.coroutine
     def wait_for_connection_lost(self):
         """
-        Wait until the TCP connection is closed or ``self.timeout`` elapses.
+        Wait until the TCP connection is closed or ``self.close_timeout`` elapses.
 
         Return ``True`` if the connection is closed and ``False`` otherwise.
 
@@ -984,7 +990,7 @@ class WebSocketCommonProtocol(asyncio.StreamReaderProtocol):
             try:
                 yield from asyncio.wait_for(
                     asyncio.shield(self.connection_lost_waiter),
-                    self.timeout,
+                    self.close_timeout,
                     loop=self.loop,
                 )
             except asyncio.TimeoutError:
diff --git a/src/websockets/server.py b/src/websockets/server.py
index 4284d09..a349fea 100644
--- a/src/websockets/server.py
+++ b/src/websockets/server.py
@@ -667,7 +667,7 @@ class Serve:
     :class:`WebSocketServerProtocol` instance. It defaults to
     :class:`WebSocketServerProtocol`.
 
-    The behavior of the ``ping_interval``, ``ping_timeout``, ``timeout``,
+    The behavior of the ``ping_interval``, ``ping_timeout``, ``close_timeout``,
     ``max_size``, ``max_queue``, ``read_limit``, and ``write_limit`` optional
     arguments is described in the documentation of
     :class:`~websockets.protocol.WebSocketCommonProtocol`.
@@ -721,14 +721,15 @@ class Serve:
         create_protocol=None,
         ping_interval=20,
         ping_timeout=20,
-        timeout=10,
+        close_timeout=None,
         max_size=2 ** 20,
         max_queue=2 ** 5,
         read_limit=2 ** 16,
         write_limit=2 ** 16,
         loop=None,
         legacy_recv=False,
-        klass=None,
+        klass=WebSocketServerProtocol,
+        timeout=10,
         origins=None,
         extensions=None,
         subprotocols=None,
@@ -736,14 +737,16 @@ class Serve:
         compression='deflate',
         **kwds
     ):
+        # Backwards-compatibility: close_timeout used to be called timeout.
+        # If both are specified, timeout is ignored.
+        if close_timeout is None:
+            close_timeout = timeout
+
         # Backwards-compatibility: create_protocol used to be called klass.
-        # In the unlikely event that both are specified, klass is ignored.
+        # If both are specified, klass is ignored.
         if create_protocol is None:
             create_protocol = klass
 
-        if create_protocol is None:
-            create_protocol = WebSocketServerProtocol
-
         if loop is None:
             loop = asyncio.get_event_loop()
 
@@ -770,7 +773,7 @@ class Serve:
             secure=secure,
             ping_interval=ping_interval,
             ping_timeout=ping_timeout,
-            timeout=timeout,
+            close_timeout=close_timeout,
             max_size=max_size,
             max_queue=max_queue,
             read_limit=read_limit,


Test patch:
diff --git a/tests/test_client_server.py b/tests/test_client_server.py
index 83bd6ad..55fee73 100644
--- a/tests/test_client_server.py
+++ b/tests/test_client_server.py
@@ -52,6 +52,8 @@ testcert = bytes(pathlib.Path(__file__).with_name('test_localhost.pem'))
 def handler(ws, path):
     if path == '/attributes':
         yield from ws.send(repr((ws.host, ws.port, ws.secure)))
+    elif path == '/close_timeout':
+        yield from ws.send(repr(ws.close_timeout))
     elif path == '/path':
         yield from ws.send(str(ws.path))
     elif path == '/headers':
@@ -554,7 +556,7 @@ class ClientServerTests(unittest.TestCase):
         self.assert_client_raises_code(401)
 
     @with_server(klass=UnauthorizedServerProtocol)
-    def test_server_klass(self):
+    def test_server_klass_backwards_compatibility(self):
         self.assert_client_raises_code(401)
 
     @with_server(
@@ -586,6 +588,39 @@ class ClientServerTests(unittest.TestCase):
     def test_client_create_protocol_over_klass(self):
         self.assertIsInstance(self.client, BarClientProtocol)
 
+    @with_server(close_timeout=7)
+    @with_client('/close_timeout')
+    def test_server_close_timeout(self):
+        close_timeout = self.loop.run_until_complete(self.client.recv())
+        self.assertEqual(eval(close_timeout), 7)
+
+    @with_server(timeout=6)
+    @with_client('/close_timeout')
+    def test_server_timeout_backwards_compatibility(self):
+        close_timeout = self.loop.run_until_complete(self.client.recv())
+        self.assertEqual(eval(close_timeout), 6)
+
+    @with_server(close_timeout=7, timeout=6)
+    @with_client('/close_timeout')
+    def test_server_close_timeout_over_timeout(self):
+        close_timeout = self.loop.run_until_complete(self.client.recv())
+        self.assertEqual(eval(close_timeout), 7)
+
+    @with_server()
+    @with_client('/close_timeout', close_timeout=7)
+    def test_client_close_timeout(self):
+        self.assertEqual(self.client.close_timeout, 7)
+
+    @with_server()
+    @with_client('/close_timeout', timeout=6)
+    def test_client_timeout_backwards_compatibility(self):
+        self.assertEqual(self.client.close_timeout, 6)
+
+    @with_server()
+    @with_client('/close_timeout', close_timeout=7, timeout=6)
+    def test_client_close_timeout_over_timeout(self):
+        self.assertEqual(self.client.close_timeout, 7)
+
     @with_server()
     @with_client('/extensions')
     def test_no_extension(self):
diff --git a/tests/test_protocol.py b/tests/test_protocol.py
index 6c0a432..27a5c11 100644
--- a/tests/test_protocol.py
+++ b/tests/test_protocol.py
@@ -1010,7 +1010,7 @@ class ServerTests(CommonTests, unittest.TestCase):
         self.protocol.side = 'server'
 
     def test_local_close_send_close_frame_timeout(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         self.make_drain_slow(50 * MS)
         # If we can't send a close frame, time out in 10ms.
         # Check the timing within -1/+9ms for robustness.
@@ -1019,7 +1019,7 @@ class ServerTests(CommonTests, unittest.TestCase):
         self.assertConnectionClosed(1006, '')
 
     def test_local_close_receive_close_frame_timeout(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         # If the client doesn't send a close frame, time out in 10ms.
         # Check the timing within -1/+9ms for robustness.
         with self.assertCompletesWithin(9 * MS, 19 * MS):
@@ -1027,7 +1027,7 @@ class ServerTests(CommonTests, unittest.TestCase):
         self.assertConnectionClosed(1006, '')
 
     def test_local_close_connection_lost_timeout_after_write_eof(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         # If the client doesn't close its side of the TCP connection after we
         # half-close our side with write_eof(), time out in 10ms.
         # Check the timing within -1/+9ms for robustness.
@@ -1039,7 +1039,7 @@ class ServerTests(CommonTests, unittest.TestCase):
         self.assertConnectionClosed(1000, 'close')
 
     def test_local_close_connection_lost_timeout_after_close(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         # If the client doesn't close its side of the TCP connection after we
         # half-close our side with write_eof() and close it with close(), time
         # out in 20ms.
@@ -1061,7 +1061,7 @@ class ClientTests(CommonTests, unittest.TestCase):
         self.protocol.side = 'client'
 
     def test_local_close_send_close_frame_timeout(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         self.make_drain_slow(50 * MS)
         # If we can't send a close frame, time out in 20ms.
         # - 10ms waiting for sending a close frame
@@ -1072,7 +1072,7 @@ class ClientTests(CommonTests, unittest.TestCase):
         self.assertConnectionClosed(1006, '')
 
     def test_local_close_receive_close_frame_timeout(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         # If the server doesn't send a close frame, time out in 20ms:
         # - 10ms waiting for receiving a close frame
         # - 10ms waiting for receiving a half-close
@@ -1082,7 +1082,7 @@ class ClientTests(CommonTests, unittest.TestCase):
         self.assertConnectionClosed(1006, '')
 
     def test_local_close_connection_lost_timeout_after_write_eof(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         # If the server doesn't half-close its side of the TCP connection
         # after we send a close frame, time out in 20ms:
         # - 10ms waiting for receiving a half-close
@@ -1096,7 +1096,7 @@ class ClientTests(CommonTests, unittest.TestCase):
         self.assertConnectionClosed(1000, 'close')
 
     def test_local_close_connection_lost_timeout_after_close(self):
-        self.protocol.timeout = 10 * MS
+        self.protocol.close_timeout = 10 * MS
         # If the client doesn't close its side of the TCP connection after we
         # half-close our side with write_eof() and close it with close(), time
         # out in 20ms.