Return 500 error when handler has wrong return type (#8845)

CHANGES/8845.bugfix.rst

@@ -0,0 +1 @@
+Changed behaviour when returning an invalid response to send a 500 response -- by :user:`Dreamsorcerer`.

aiohttp/web_protocol.py

@@ -39,7 +39,7 @@
 from .log import access_logger, server_logger
 from .streams import EMPTY_PAYLOAD, StreamReader
 from .tcp_helpers import tcp_keepalive
-from .web_exceptions import HTTPException
+from .web_exceptions import HTTPException, HTTPInternalServerError
 from .web_log import AccessLogger
 from .web_request import BaseRequest
 from .web_response import Response, StreamResponse
@@ -490,18 +490,18 @@ async def _handle_request(
                 status=exc.status, reason=exc.reason, text=exc.text, headers=exc.headers
             )
             resp._cookies = exc._cookies
-            reset = await self.finish_response(request, resp, start_time)
+            resp, reset = await self.finish_response(request, resp, start_time)
         except asyncio.CancelledError:
             raise
         except asyncio.TimeoutError as exc:
             self.log_debug("Request handler timed out.", exc_info=exc)
             resp = self.handle_error(request, 504)
-            reset = await self.finish_response(request, resp, start_time)
+            resp, reset = await self.finish_response(request, resp, start_time)
         except Exception as exc:
             resp = self.handle_error(request, 500, exc)
-            reset = await self.finish_response(request, resp, start_time)
+            resp, reset = await self.finish_response(request, resp, start_time)
         else:
-            reset = await self.finish_response(request, resp, start_time)
+            resp, reset = await self.finish_response(request, resp, start_time)
         finally:
             self._handler_waiter.set_result(None)
 
@@ -603,10 +603,6 @@ async def start(self) -> None:
             except asyncio.CancelledError:
                 self.log_debug("Ignored premature client disconnection ")
                 break
-            except RuntimeError as exc:
-                if self._loop.get_debug():
-                    self.log_exception("Unhandled runtime exception", exc_info=exc)
-                self.force_close()
             except Exception as exc:
                 self.log_exception("Unhandled exception", exc_info=exc)
                 self.force_close()
@@ -635,7 +631,7 @@ async def start(self) -> None:
 
     async def finish_response(
         self, request: BaseRequest, resp: StreamResponse, start_time: float
-    ) -> bool:
+    ) -> Tuple[StreamResponse, bool]:
         """Prepare the response and write_eof, then log access.
 
         This has to
@@ -654,22 +650,26 @@ async def finish_response(
             prepare_meth = resp.prepare
         except AttributeError:
             if resp is None:
-                raise RuntimeError("Missing return " "statement on request handler")
+                self.log_exception("Missing return statement on request handler")
             else:
-                raise RuntimeError(
-                    "Web-handler should return "
-                    "a response instance, "
+                self.log_exception(
+                    "Web-handler should return a response instance, "
                     "got {!r}".format(resp)
                 )
+            exc = HTTPInternalServerError()
+            resp = Response(
+                status=exc.status, reason=exc.reason, text=exc.text, headers=exc.headers
+            )
+            prepare_meth = resp.prepare
         try:
             await prepare_meth(request)
             await resp.write_eof()
         except ConnectionError:
             await self.log_access(request, resp, start_time)
-            return True
-        else:
-            await self.log_access(request, resp, start_time)
-            return False
+            return resp, True
+
+        await self.log_access(request, resp, start_time)
+        return resp, False
 
     def handle_error(
         self,

tests/test_web_functional.py

@@ -103,12 +103,8 @@ async def handler(request):
     server = await aiohttp_server(app, logger=logger)
     client = await aiohttp_client(server)
 
-    with pytest.raises(aiohttp.ServerDisconnectedError):
-        await client.get("/")
-
-    logger.exception.assert_called_with(
-        "Unhandled runtime exception", exc_info=mock.ANY
-    )
+    async with client.get("/") as resp:
+        assert resp.status == 500
 
 
 async def test_handler_returns_none(aiohttp_server: Any, aiohttp_client: Any) -> None:
@@ -123,13 +119,8 @@ async def handler(request):
     server = await aiohttp_server(app, logger=logger)
     client = await aiohttp_client(server)
 
-    with pytest.raises(aiohttp.ServerDisconnectedError):
-        await client.get("/")
-
-    # Actual error text is placed in exc_info
-    logger.exception.assert_called_with(
-        "Unhandled runtime exception", exc_info=mock.ANY
-    )
+    async with client.get("/") as resp:
+        assert resp.status == 500
 
 
 async def test_head_returns_empty_body(aiohttp_client: Any) -> None:

tests/test_web_request.py

@@ -12,7 +12,6 @@
 from yarl import URL
 
 from aiohttp import HttpVersion, web
-from aiohttp.client_exceptions import ServerDisconnectedError
 from aiohttp.http_parser import RawRequestMessage
 from aiohttp.streams import StreamReader
 from aiohttp.test_utils import make_mocked_request
@@ -820,22 +819,6 @@ def test_weakref_creation() -> None:
     weakref.ref(req)
 
 
-@pytest.mark.xfail(
-    raises=ServerDisconnectedError,
-    reason="see https://github.com/aio-libs/aiohttp/issues/4572",
-)
-async def test_handler_return_type(aiohttp_client: Any) -> None:
-    async def invalid_handler_1(request):
-        return 1
-
-    app = web.Application()
-    app.router.add_get("/1", invalid_handler_1)
-    client = await aiohttp_client(app)
-
-    async with client.get("/1") as resp:
-        assert 500 == resp.status
-
-
 @pytest.mark.parametrize(
     ["header", "header_attr"],
     [