Lots of CancelledError when requests are interupted #1565

aio-libs · Jan 30, 2017 · 7ab213b · 7ab213b
1 parent 4a59c4f
commit 7ab213b
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 56 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -16,6 +16,8 @@ CHANGES
 
 - Lingering close doesn't terminate before timeout #1559
 
+- Lots of CancelledError when requests are interupted #1565
+
 - Allow users to specify what should happen to decoding errors
   when calling a responses `text()` method #1542
 

diff --git a/aiohttp/test_utils.py b/aiohttp/test_utils.py
@@ -163,8 +163,9 @@ def __init__(self, handler,
         super().__init__(scheme=scheme, host=host)
 
     @asyncio.coroutine
-    def _make_factory(self, **kwargs):
-        self.handler = Server(self._handler, loop=self._loop, **kwargs)
+    def _make_factory(self, debug=True, **kwargs):
+        self.handler = Server(
+            self._handler, loop=self._loop, debug=True, **kwargs)
         return self.handler
 
     @asyncio.coroutine

diff --git a/aiohttp/web_server.py b/aiohttp/web_server.py
@@ -4,6 +4,7 @@
 import traceback
 from html import escape as html_escape
 
+from . import errors
 from .helpers import TimeService
 from .server import ServerHttpProtocol
 from .web_exceptions import HTTPException, HTTPInternalServerError
@@ -57,36 +58,37 @@ def handle_request(self, message, payload):
         self._request = request
 
         try:
-            try:
-                resp = yield from self._handler(request)
-            except HTTPException as exc:
-                resp = exc
-            except Exception as exc:
-                msg = "<h1>500 Internal Server Error</h1>"
-                if self.debug:
-                    try:
-                        tb = traceback.format_exc()
-                        tb = html_escape(tb)
-                        msg += '<br><h2>Traceback:</h2>\n<pre>'
-                        msg += tb
-                        msg += '</pre>'
-                    except:  # pragma: no cover
-                        pass
-                else:
-                    msg += "Server got itself in trouble"
-                msg = ("<html><head><title>500 Internal Server Error</title>"
-                       "</head><body>" + msg + "</body></html>")
-                resp = HTTPInternalServerError(text=msg,
-                                               content_type='text/html')
-                self.logger.exception(
-                    "Error handling request",
-                    exc_info=exc)
-
-            if not resp.prepared:
-                yield from resp.prepare(request)
-            yield from resp.write_eof()
-        finally:
-            resp._task = None
+            resp = yield from self._handler(request)
+        except (asyncio.CancelledError,
+                asyncio.TimeoutError,
+                errors.ClientDisconnectedError) as exc:
+            raise
+        except HTTPException as exc:
+            resp = exc
+        except Exception as exc:
+            msg = "<h1>500 Internal Server Error</h1>"
+            if self.debug:
+                try:
+                    tb = traceback.format_exc()
+                    tb = html_escape(tb)
+                    msg += '<br><h2>Traceback:</h2>\n<pre>'
+                    msg += tb
+                    msg += '</pre>'
+                except:  # pragma: no cover
+                    pass
+            else:
+                msg += "Server got itself in trouble"
+            msg = ("<html><head><title>500 Internal Server Error</title>"
+                   "</head><body>" + msg + "</body></html>")
+            resp = HTTPInternalServerError(
+                text=msg, content_type='text/html')
+            self.logger.exception(
+                "Error handling request",
+                exc_info=exc)
+
+        if not resp.prepared:
+            yield from resp.prepare(request)
+        yield from resp.write_eof()
 
         # notify server about keep-alive
         # assign to parent class attr

diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py
@@ -1187,29 +1187,6 @@ def handler(request):
     assert resp.headers['Date'] == 'Sun, 30 Oct 2016 03:13:52 GMT'
 
 
-@asyncio.coroutine
-def test_response_task(loop, test_client):
-
-    srv_resp = None
-
-    @asyncio.coroutine
-    def handler(request):
-        nonlocal srv_resp
-        srv_resp = web.StreamResponse()
-        assert srv_resp.task is None
-        yield from srv_resp.prepare(request)
-        assert srv_resp.task is not None
-        return srv_resp
-
-    app = web.Application(loop=loop)
-    app.router.add_get('/', handler)
-    client = yield from test_client(app)
-
-    resp = yield from client.get('/')
-    assert 200 == resp.status
-    assert srv_resp.task is None
-
-
 @asyncio.coroutine
 def test_response_prepared_with_clone(loop, test_client):
 

diff --git a/tests/test_web_server.py b/tests/test_web_server.py
@@ -1,7 +1,8 @@
 import asyncio
+import pytest
 from unittest import mock
 
-from aiohttp import web
+from aiohttp import web, errors
 
 
 @asyncio.coroutine
@@ -40,6 +41,30 @@ def handler(request):
         exc_info=exc)
 
 
+@asyncio.coroutine
+def test_raw_server_do_not_swallow_exceptions(raw_test_server, test_client):
+    exc = None
+
+    @asyncio.coroutine
+    def handler(request):
+        raise exc
+
+    logger = mock.Mock()
+    server = yield from raw_test_server(handler, logger=logger)
+    client = yield from test_client(server)
+
+    for _exc, msg in (
+            (asyncio.TimeoutError("error"), 'Request handler timed out.'),
+            (asyncio.CancelledError("error"), 'Request handler cancelled.'),
+            (errors.ClientDisconnectedError("error"),
+             'Ignored premature client disconnection #1.')):
+        exc = _exc
+        with pytest.raises(errors.ClientResponseError):
+            yield from client.get('/path/to')
+
+        logger.debug.assert_called_with(msg)
+
+
 @asyncio.coroutine
 def test_raw_server_not_http_exception_debug(raw_test_server, test_client):
     exc = RuntimeError("custom runtime error")