From 389363ab711fb57ec9461547e37c91eb7ebb72a0 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Mon, 19 Sep 2022 16:04:09 +0300 Subject: [PATCH] Better request cancel handling (#2513) --- sanic/exceptions.py | 5 +++++ sanic/http/http1.py | 10 +++++++--- sanic/models/server_types.py | 2 ++ sanic/server/protocols/base_protocol.py | 6 ++++-- sanic/server/protocols/http_protocol.py | 8 ++++++-- tests/test_cancellederror.py | 21 +++++++++++++++++++++ tests/test_middleware.py | 2 +- 7 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 tests/test_cancellederror.py diff --git a/sanic/exceptions.py b/sanic/exceptions.py index 720a5834..1190e36e 100644 --- a/sanic/exceptions.py +++ b/sanic/exceptions.py @@ -1,8 +1,13 @@ +from asyncio import CancelledError from typing import Any, Dict, Optional, Union from sanic.helpers import STATUS_CODES +class RequestCancelled(CancelledError): + quiet = True + + class SanicException(Exception): message: str = "" diff --git a/sanic/http/http1.py b/sanic/http/http1.py index fefa6eeb..1f2423ad 100644 --- a/sanic/http/http1.py +++ b/sanic/http/http1.py @@ -14,8 +14,8 @@ from sanic.exceptions import ( BadRequest, ExpectationFailed, PayloadTooLarge, + RequestCancelled, ServerError, - ServiceUnavailable, ) from sanic.headers import format_http1_response from sanic.helpers import has_message_body @@ -132,7 +132,7 @@ class Http(Stream, metaclass=TouchUpMeta): if self.stage is Stage.RESPONSE: await self.response.send(end_stream=True) - except CancelledError: + except CancelledError as exc: # Write an appropriate response before exiting if not self.protocol.transport: logger.info( @@ -140,7 +140,11 @@ class Http(Stream, metaclass=TouchUpMeta): "stopped. Transport is closed." ) return - e = self.exception or ServiceUnavailable("Cancelled") + e = ( + RequestCancelled() + if self.protocol.conn_info.lost + else (self.exception or exc) + ) self.exception = None self.keep_alive = False await self.error_response(e) diff --git a/sanic/models/server_types.py b/sanic/models/server_types.py index da88a8ff..f6546e7d 100644 --- a/sanic/models/server_types.py +++ b/sanic/models/server_types.py @@ -21,6 +21,7 @@ class ConnInfo: "client", "client_ip", "ctx", + "lost", "peername", "server_port", "server", @@ -33,6 +34,7 @@ class ConnInfo: def __init__(self, transport: TransportProtocol, unix=None): self.ctx = SimpleNamespace() + self.lost = False self.peername = None self.server = self.client = "" self.server_port = self.client_port = 0 diff --git a/sanic/server/protocols/base_protocol.py b/sanic/server/protocols/base_protocol.py index 63d4bfb5..bf1b3c15 100644 --- a/sanic/server/protocols/base_protocol.py +++ b/sanic/server/protocols/base_protocol.py @@ -2,13 +2,14 @@ from __future__ import annotations from typing import TYPE_CHECKING, Optional +from sanic.exceptions import RequestCancelled + if TYPE_CHECKING: from sanic.app import Sanic import asyncio -from asyncio import CancelledError from asyncio.transports import Transport from time import monotonic as current_time @@ -69,7 +70,7 @@ class SanicProtocol(asyncio.Protocol): """ await self._can_write.wait() if self.transport.is_closing(): - raise CancelledError + raise RequestCancelled self.transport.write(data) self._time = current_time() @@ -120,6 +121,7 @@ class SanicProtocol(asyncio.Protocol): try: self.connections.discard(self) self.resume_writing() + self.conn_info.lost = True if self._task: self._task.cancel() except BaseException: diff --git a/sanic/server/protocols/http_protocol.py b/sanic/server/protocols/http_protocol.py index 18f85e67..6e30fc0f 100644 --- a/sanic/server/protocols/http_protocol.py +++ b/sanic/server/protocols/http_protocol.py @@ -15,7 +15,11 @@ import sys from asyncio import CancelledError from time import monotonic as current_time -from sanic.exceptions import RequestTimeout, ServiceUnavailable +from sanic.exceptions import ( + RequestCancelled, + RequestTimeout, + ServiceUnavailable, +) from sanic.http import Http, Stage from sanic.log import Colors, error_logger, logger from sanic.models.server_types import ConnInfo @@ -225,7 +229,7 @@ class HttpProtocol(HttpProtocolMixin, SanicProtocol, metaclass=TouchUpMeta): """ await self._can_write.wait() if self.transport.is_closing(): - raise CancelledError + raise RequestCancelled await self.app.dispatch( "http.lifecycle.send", inline=True, diff --git a/tests/test_cancellederror.py b/tests/test_cancellederror.py new file mode 100644 index 00000000..b61fd596 --- /dev/null +++ b/tests/test_cancellederror.py @@ -0,0 +1,21 @@ +import asyncio + +from asyncio import CancelledError + +import pytest + +from sanic import Request, Sanic, json + + +def test_can_raise_in_handler(app: Sanic): + @app.get("/") + async def handler(request: Request): + raise CancelledError("STOP!!") + + @app.exception(CancelledError) + async def handle_cancel(request: Request, exc: CancelledError): + return json({"message": exc.args[0]}, status=418) + + _, response = app.test_client.get("/") + assert response.status == 418 + assert response.json["message"] == "STOP!!" diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 2163e47c..a2034450 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -166,7 +166,7 @@ def test_middleware_response_raise_cancelled_error(app, caplog): with caplog.at_level(logging.ERROR): reqrequest, response = app.test_client.get("/") - assert response.status == 503 + assert response.status == 500 assert ( "sanic.error", logging.ERROR,