From c72cbe432661bcefa6b3359aeb2a5583b857db53 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 7 Aug 2022 22:31:26 +0300 Subject: [PATCH 01/10] Begin middleware revamp --- sanic/app.py | 162 ++++++++++++++++++------------------- sanic/middleware.py | 61 ++++++++++++++ sanic/mixins/middleware.py | 58 +++++++++++-- sanic/request.py | 23 ++++-- sanic/signals.py | 2 +- 5 files changed, 208 insertions(+), 98 deletions(-) create mode 100644 sanic/middleware.py diff --git a/sanic/app.py b/sanic/app.py index 59056eb228..4926edd4b6 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -701,7 +701,10 @@ def url_for(self, view_name: str, **kwargs): # -------------------------------------------------------------------- # async def handle_exception( - self, request: Request, exception: BaseException + self, + request: Request, + exception: BaseException, + run_middleware: bool = True, ): # no cov """ A handler that catches specific exceptions and outputs a response. @@ -710,6 +713,7 @@ async def handle_exception( :param exception: The exception that was raised :raises ServerError: response 500 """ + response = None await self.dispatch( "http.lifecycle.exception", inline=True, @@ -750,9 +754,12 @@ async def handle_exception( # -------------------------------------------- # # Request Middleware # -------------------------------------------- # - response = await self._run_request_middleware( - request, request_name=None - ) + if ( + run_middleware + and request.route + and request.route.extra.request_middleware + ): + response = await self._run_request_middleware(request) # No middleware results if not response: try: @@ -832,7 +839,12 @@ async def handle_request(self, request: Request): # no cov # Define `response` var here to remove warnings about # allocation before assignment below. - response = None + response: Optional[ + Union[ + BaseHTTPResponse, + Coroutine[Any, Any, Optional[BaseHTTPResponse]], + ] + ] = None try: await self.dispatch( @@ -877,9 +889,8 @@ async def handle_request(self, request: Request): # no cov # -------------------------------------------- # # Request Middleware # -------------------------------------------- # - response = await self._run_request_middleware( - request, request_name=route.name - ) + if request.route.extra.request_middleware: + response = await self._run_request_middleware(request) # No middleware results if not response: @@ -910,7 +921,7 @@ async def handle_request(self, request: Request): # no cov if request.stream is not None: response = request.stream.response elif response is not None: - response = await request.respond(response) + response = await request.respond(response) # type: ignore elif not hasattr(handler, "is_websocket"): response = request.stream.response # type: ignore @@ -928,7 +939,7 @@ async def handle_request(self, request: Request): # no cov ... await response.send(end_stream=True) elif isinstance(response, ResponseStream): - resp = await response(request) + resp = await response(request) # type: ignore await self.dispatch( "http.lifecycle.response", inline=True, @@ -937,7 +948,7 @@ async def handle_request(self, request: Request): # no cov "response": resp, }, ) - await response.eof() + await response.eof() # type: ignore else: if not hasattr(handler, "is_websocket"): raise ServerError( @@ -949,7 +960,7 @@ async def handle_request(self, request: Request): # no cov raise except Exception as e: # Response Generation Failed - await self.handle_exception(request, e) + await self.handle_exception(request, e, run_middleware=False) async def _websocket_handler( self, handler, request, *args, subprotocols=None, **kwargs @@ -1017,87 +1028,69 @@ def asgi_client(self): # noqa # Execution # -------------------------------------------------------------------- # - async def _run_request_middleware( - self, request, request_name=None - ): # no cov - # The if improves speed. I don't know why - named_middleware = self.named_request_middleware.get( - request_name, deque() - ) - applicable_middleware = self.request_middleware + named_middleware - - # request.request_middleware_started is meant as a stop-gap solution - # until RFC 1630 is adopted - if applicable_middleware and not request.request_middleware_started: - request.request_middleware_started = True + async def _run_request_middleware(self, request): # no cov + request._request_middleware_started = True - for middleware in applicable_middleware: - await self.dispatch( - "http.middleware.before", - inline=True, - context={ - "request": request, - "response": None, - }, - condition={"attach_to": "request"}, - ) + for middleware in request.route.extra.request_middleware: + await self.dispatch( + "http.middleware.before", + inline=True, + context={ + "request": request, + "response": None, + }, + condition={"attach_to": "request"}, + ) - response = middleware(request) - if isawaitable(response): - response = await response + response = middleware(request) + if isawaitable(response): + response = await response - await self.dispatch( - "http.middleware.after", - inline=True, - context={ - "request": request, - "response": None, - }, - condition={"attach_to": "request"}, - ) + await self.dispatch( + "http.middleware.after", + inline=True, + context={ + "request": request, + "response": None, + }, + condition={"attach_to": "request"}, + ) - if response: - return response + if response: + return response return None - async def _run_response_middleware( - self, request, response, request_name=None - ): # no cov - named_middleware = self.named_response_middleware.get( - request_name, deque() - ) - applicable_middleware = self.response_middleware + named_middleware - if applicable_middleware: - for middleware in applicable_middleware: - await self.dispatch( - "http.middleware.before", - inline=True, - context={ - "request": request, - "response": response, - }, - condition={"attach_to": "response"}, - ) + async def _run_response_middleware(self, request, response): # no cov + for middleware in request.route.extra.response_middleware: + await self.dispatch( + "http.middleware.before", + inline=True, + context={ + "request": request, + "response": response, + }, + condition={"attach_to": "response"}, + ) - _response = middleware(request, response) - if isawaitable(_response): - _response = await _response + _response = middleware(request, response) + if isawaitable(_response): + _response = await _response - await self.dispatch( - "http.middleware.after", - inline=True, - context={ - "request": request, - "response": _response if _response else response, - }, - condition={"attach_to": "response"}, - ) + await self.dispatch( + "http.middleware.after", + inline=True, + context={ + "request": request, + "response": _response if _response else response, + }, + condition={"attach_to": "response"}, + ) - if _response: - response = _response - if isinstance(response, BaseHTTPResponse): - response = request.stream.respond(response) - break + if _response: + response = _response + if isinstance(response, BaseHTTPResponse): + response = request.stream.respond(response) + break return response def _build_endpoint_name(self, *parts): @@ -1495,6 +1488,7 @@ def finalize(self): except FinalizationError as e: if not Sanic.test_mode: raise e + self.finalize_middleware() def signalize(self, allow_fail_builtin=True): self.signal_router.allow_fail_builtin = allow_fail_builtin diff --git a/sanic/middleware.py b/sanic/middleware.py new file mode 100644 index 0000000000..e3214c0526 --- /dev/null +++ b/sanic/middleware.py @@ -0,0 +1,61 @@ +from __future__ import annotations + +from collections import deque +from enum import IntEnum, auto +from itertools import count +from typing import Deque, Optional, Sequence, Union + +from sanic.models.handler_types import MiddlewareType + + +class MiddlewareLocation(IntEnum): + REQUEST = auto() + RESPONSE = auto() + + +class Middleware: + counter = count() + + def __init__( + self, + func: MiddlewareType, + location: MiddlewareLocation, + priority: int = 0, + ) -> None: + self.func = func + self.priority = priority + self.location = location + self.definition = next(Middleware.counter) + + def __call__(self, *args, **kwargs): + return self.func(*args, **kwargs) + + def __repr__(self) -> str: + return ( + f"{self.__class__.__name__}(" + f"func=, " + f"priority={self.priority}, " + f"location={self.location.name})" + ) + + @property + def order(self): + return (self.priority, -self.definition) + + @classmethod + def convert( + cls, + *middleware_collections: Sequence[ + Optional[Union[Middleware, MiddlewareType]] + ], + location: MiddlewareLocation, + ) -> Deque[Middleware]: + return deque( + [ + middleware + if isinstance(middleware, Middleware) + else Middleware(middleware, location) + for collection in middleware_collections + for middleware in collection + ] + ) diff --git a/sanic/mixins/middleware.py b/sanic/mixins/middleware.py index 5ef9dc77a1..7615477c0f 100644 --- a/sanic/mixins/middleware.py +++ b/sanic/mixins/middleware.py @@ -1,11 +1,17 @@ +from collections import deque from functools import partial +from operator import attrgetter from typing import List from sanic.base.meta import SanicMeta +from sanic.middleware import Middleware, MiddlewareLocation from sanic.models.futures import FutureMiddleware +from sanic.router import Router class MiddlewareMixin(metaclass=SanicMeta): + router: Router + def __init__(self, *args, **kwargs) -> None: self._future_middleware: List[FutureMiddleware] = [] @@ -13,7 +19,12 @@ def _apply_middleware(self, middleware: FutureMiddleware): raise NotImplementedError # noqa def middleware( - self, middleware_or_request, attach_to="request", apply=True + self, + middleware_or_request, + attach_to="request", + apply=True, + *, + priority=0 ): """ Decorate and register middleware to be called before a request @@ -30,6 +41,12 @@ def middleware( def register_middleware(middleware, attach_to="request"): nonlocal apply + location = ( + MiddlewareLocation.REQUEST + if attach_to == "request" + else MiddlewareLocation.RESPONSE + ) + middleware = Middleware(middleware, location, priority=priority) future_middleware = FutureMiddleware(middleware, attach_to) self._future_middleware.append(future_middleware) if apply: @@ -46,7 +63,7 @@ def register_middleware(middleware, attach_to="request"): register_middleware, attach_to=middleware_or_request ) - def on_request(self, middleware=None): + def on_request(self, middleware=None, *, priority=0): """Register a middleware to be called before a request is handled. This is the same as *@app.middleware('request')*. @@ -54,11 +71,13 @@ def on_request(self, middleware=None): :param: middleware: A callable that takes in request. """ if callable(middleware): - return self.middleware(middleware, "request") + return self.middleware(middleware, "request", priority=priority) else: - return partial(self.middleware, attach_to="request") + return partial( + self.middleware, attach_to="request", priority=priority + ) - def on_response(self, middleware=None): + def on_response(self, middleware=None, *, priority=0): """Register a middleware to be called after a response is created. This is the same as *@app.middleware('response')*. @@ -67,6 +86,31 @@ def on_response(self, middleware=None): A callable that takes in a request and its response. """ if callable(middleware): - return self.middleware(middleware, "response") + return self.middleware(middleware, "response", priority=priority) else: - return partial(self.middleware, attach_to="response") + return partial( + self.middleware, attach_to="response", priority=priority + ) + + def finalize_middleware(self): + for route in self.router.routes: + request_middleware = Middleware.convert( + self.request_middleware, + self.named_request_middleware.get(route.name, deque()), + location=MiddlewareLocation.REQUEST, + ) + response_middleware = Middleware.convert( + self.response_middleware, + self.named_response_middleware.get(route.name, deque()), + location=MiddlewareLocation.RESPONSE, + ) + route.extra.request_middleware = sorted( + request_middleware, + key=attrgetter("order"), + reverse=True, + ) + route.extra.response_middleware = sorted( + response_middleware, + key=attrgetter("order"), + reverse=True, + )[::-1] diff --git a/sanic/request.py b/sanic/request.py index 927c124a8b..17016c976a 100644 --- a/sanic/request.py +++ b/sanic/request.py @@ -51,7 +51,7 @@ parse_xforwarded, ) from sanic.http import Stage -from sanic.log import error_logger, logger +from sanic.log import deprecation, error_logger, logger from sanic.models.protocol_types import TransportProtocol from sanic.response import BaseHTTPResponse, HTTPResponse @@ -98,6 +98,7 @@ class Request: "_port", "_protocol", "_remote_addr", + "_request_middleware_started", "_scheme", "_socket", "_stream_id", @@ -121,7 +122,6 @@ class Request: "parsed_token", "raw_url", "responded", - "request_middleware_started", "route", "stream", "transport", @@ -173,7 +173,7 @@ def __init__( self.parsed_not_grouped_args: DefaultDict[ Tuple[bool, bool, str, str], List[Tuple[str, str]] ] = defaultdict(list) - self.request_middleware_started = False + self._request_middleware_started = False self.responded: bool = False self.route: Optional[Route] = None self.stream: Optional[Stream] = None @@ -214,6 +214,16 @@ def get_current(cls) -> Request: def generate_id(*_): return uuid.uuid4() + @property + def request_middleware_started(self): + deprecation( + "Request.request_middleware_started has been deprecated and will" + "be removed. You should set a flag on the request context using" + "either middleware or signals if you need this feature.", + 22.3, + ) + return self._request_middleware_started + @property def stream_id(self): """ @@ -319,9 +329,10 @@ async def add_header(_, response: HTTPResponse): response = await response # type: ignore # Run response middleware try: - response = await self.app._run_response_middleware( - self, response, request_name=self.name - ) + if self.route and self.route.extra.response_middleware: + response = await self.app._run_response_middleware( + self, response + ) except CancelledErrors: raise except Exception: diff --git a/sanic/signals.py b/sanic/signals.py index 80c6300b73..302bec2d2f 100644 --- a/sanic/signals.py +++ b/sanic/signals.py @@ -187,7 +187,7 @@ async def dispatch( fail_not_found=fail_not_found and inline, reverse=reverse, ) - logger.debug(f"Dispatching signal: {event}") + logger.debug(f"Dispatching signal: {event}", extra={"verbosity": 1}) if inline: return await dispatch From 782e0881e5cd7b7943efc082ce559a02fa9b111a Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 7 Aug 2022 22:38:25 +0300 Subject: [PATCH 02/10] Slots to Middleware --- sanic/middleware.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sanic/middleware.py b/sanic/middleware.py index e3214c0526..3ab093dcf4 100644 --- a/sanic/middleware.py +++ b/sanic/middleware.py @@ -16,6 +16,8 @@ class MiddlewareLocation(IntEnum): class Middleware: counter = count() + __slots__ = ("func", "priority", "location", "definition") + def __init__( self, func: MiddlewareType, From 78bc475bb18b9647b326c206f080e571cb33ab12 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 17 Aug 2022 15:23:30 +0300 Subject: [PATCH 03/10] Add test case --- tests/test_middleware_priority.py | 83 +++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 tests/test_middleware_priority.py diff --git a/tests/test_middleware_priority.py b/tests/test_middleware_priority.py new file mode 100644 index 0000000000..fe40e2cce9 --- /dev/null +++ b/tests/test_middleware_priority.py @@ -0,0 +1,83 @@ +from functools import partial + +import pytest + +from sanic import Sanic +from sanic.response import json + + +PRIORITY_TEST_CASES = ( + ([0, 1, 2], [1, 1, 1]), + ([0, 1, 2], [1, 1, None]), + ([0, 1, 2], [1, None, None]), + ([0, 1, 2], [2, 1, None]), + ([0, 1, 2], [2, 2, None]), + ([0, 1, 2], [3, 2, 1]), + ([0, 1, 2], [None, None, None]), + ([0, 2, 1], [1, None, 1]), + ([0, 2, 1], [2, None, 1]), + ([0, 2, 1], [2, None, 2]), + ([0, 2, 1], [3, 1, 2]), + ([1, 0, 2], [1, 2, None]), + ([1, 0, 2], [2, 3, 1]), + ([1, 0, 2], [None, 1, None]), + ([1, 2, 0], [1, 3, 2]), + ([1, 2, 0], [None, 1, 1]), + ([1, 2, 0], [None, 2, 1]), + ([1, 2, 0], [None, 2, 2]), + ([2, 0, 1], [1, None, 2]), + ([2, 0, 1], [2, 1, 3]), + ([2, 0, 1], [None, None, 1]), + ([2, 1, 0], [1, 2, 3]), + ([2, 1, 0], [None, 1, 2]), +) + + +@pytest.mark.parametrize( + "expected,priorities", + PRIORITY_TEST_CASES, +) +def test_request_middleware_order_priority(app: Sanic, expected, priorities): + order = [] + + def add_ident(request, ident): + order.append(ident) + + @app.get("/") + def handler(request): + return json(None) + + for ident, priority in enumerate(priorities): + kwargs = {} + if priority is not None: + kwargs["priority"] = priority + app.on_request(partial(add_ident, ident=ident), **kwargs) + + app.test_client.get("/") + + assert order == expected + + +@pytest.mark.parametrize( + "expected,priorities", + PRIORITY_TEST_CASES, +) +def test_response_middleware_order_priority(app: Sanic, expected, priorities): + order = [] + + def add_ident(request, response, ident): + order.append(ident) + + @app.get("/") + def handler(request): + return json(None) + + for ident, priority in enumerate(priorities): + kwargs = {} + if priority is not None: + kwargs["priority"] = priority + app.on_response(partial(add_ident, ident=ident), **kwargs) + + app.test_client.get("/") + + assert order[::-1] == expected From 09b59d34fee9f48e6496729f870286b61ffc2e26 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 17 Aug 2022 15:26:59 +0300 Subject: [PATCH 04/10] Fix typing error --- sanic/middleware.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sanic/middleware.py b/sanic/middleware.py index 3ab093dcf4..02a2ce9b87 100644 --- a/sanic/middleware.py +++ b/sanic/middleware.py @@ -47,9 +47,7 @@ def order(self): @classmethod def convert( cls, - *middleware_collections: Sequence[ - Optional[Union[Middleware, MiddlewareType]] - ], + *middleware_collections: Sequence[Union[Middleware, MiddlewareType]], location: MiddlewareLocation, ) -> Deque[Middleware]: return deque( From beb5c627671ceedf5b3b82e7270a30024a47ba60 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 17 Aug 2022 21:57:07 +0300 Subject: [PATCH 05/10] Add global middleware ordering --- sanic/app.py | 33 +++++++++++++++++++++------------ sanic/mixins/middleware.py | 30 ++++++++++++++++++++++++++++-- sanic/request.py | 7 +++++-- setup.py | 2 +- 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 237f818c14..26256b9d13 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -754,12 +754,11 @@ async def handle_exception( # -------------------------------------------- # # Request Middleware # -------------------------------------------- # - if ( - run_middleware - and request.route - and request.route.extra.request_middleware - ): - response = await self._run_request_middleware(request) + if run_middleware: + middleware = ( + request.route and request.route.extra.request_middleware + ) or self.request_middleware + response = await self._run_request_middleware(request, middleware) # No middleware results if not response: try: @@ -845,6 +844,7 @@ async def handle_request(self, request: Request): # no cov Coroutine[Any, Any, Optional[BaseHTTPResponse]], ] ] = None + run_middleware = True try: await self.dispatch( @@ -889,8 +889,11 @@ async def handle_request(self, request: Request): # no cov # -------------------------------------------- # # Request Middleware # -------------------------------------------- # + run_middleware = False if request.route.extra.request_middleware: - response = await self._run_request_middleware(request) + response = await self._run_request_middleware( + request, request.route.extra.request_middleware + ) # No middleware results if not response: @@ -960,7 +963,9 @@ async def handle_request(self, request: Request): # no cov raise except Exception as e: # Response Generation Failed - await self.handle_exception(request, e, run_middleware=False) + await self.handle_exception( + request, e, run_middleware=run_middleware + ) async def _websocket_handler( self, handler, request, *args, subprotocols=None, **kwargs @@ -1028,10 +1033,12 @@ def asgi_client(self): # noqa # Execution # -------------------------------------------------------------------- # - async def _run_request_middleware(self, request): # no cov + async def _run_request_middleware( + self, request, middleware_collection + ): # no cov request._request_middleware_started = True - for middleware in request.route.extra.request_middleware: + for middleware in middleware_collection: await self.dispatch( "http.middleware.before", inline=True, @@ -1060,8 +1067,10 @@ async def _run_request_middleware(self, request): # no cov return response return None - async def _run_response_middleware(self, request, response): # no cov - for middleware in request.route.extra.response_middleware: + async def _run_response_middleware( + self, request, response, middleware_collection + ): # no cov + for middleware in middleware_collection: await self.dispatch( "http.middleware.before", inline=True, diff --git a/sanic/mixins/middleware.py b/sanic/mixins/middleware.py index 7615477c0f..bea3976f26 100644 --- a/sanic/mixins/middleware.py +++ b/sanic/mixins/middleware.py @@ -104,13 +104,39 @@ def finalize_middleware(self): self.named_response_middleware.get(route.name, deque()), location=MiddlewareLocation.RESPONSE, ) - route.extra.request_middleware = sorted( + route.extra.request_middleware = deque( + sorted( + request_middleware, + key=attrgetter("order"), + reverse=True, + ) + ) + route.extra.response_middleware = deque( + sorted( + response_middleware, + key=attrgetter("order"), + reverse=True, + )[::-1] + ) + request_middleware = Middleware.convert( + self.request_middleware, + location=MiddlewareLocation.REQUEST, + ) + response_middleware = Middleware.convert( + self.response_middleware, + location=MiddlewareLocation.RESPONSE, + ) + self.request_middleware = deque( + sorted( request_middleware, key=attrgetter("order"), reverse=True, ) - route.extra.response_middleware = sorted( + ) + self.response_middleware = deque( + sorted( response_middleware, key=attrgetter("order"), reverse=True, )[::-1] + ) diff --git a/sanic/request.py b/sanic/request.py index ae872d10ce..552ea95567 100644 --- a/sanic/request.py +++ b/sanic/request.py @@ -334,9 +334,12 @@ async def add_header(_, response: HTTPResponse): response = await response # type: ignore # Run response middleware try: - if self.route and self.route.extra.response_middleware: + middleware = ( + self.route and self.route.extra.response_middleware + ) or self.app.response_middleware + if middleware: response = await self.app._run_response_middleware( - self, response + self, response, middleware ) except CancelledErrors: raise diff --git a/setup.py b/setup.py index f752a62e82..f0a149a8a1 100644 --- a/setup.py +++ b/setup.py @@ -84,7 +84,7 @@ def open_local(paths, mode="r", encoding="utf8"): uvloop = "uvloop>=0.5.3" + env_dependency types_ujson = "types-ujson" + env_dependency requirements = [ - "sanic-routing>=22.3.0,<22.6.0", + "sanic-routing>=22.8.0", "httptools>=0.0.10", uvloop, ujson, From 2c0168681ed1a77caa08723e167b081eb2a853f3 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 21 Sep 2022 01:14:50 +0300 Subject: [PATCH 06/10] WIP --- sanic/app.py | 79 ++++++++++++++++--------------- sanic/middleware.py | 11 +++-- tests/test_app.py | 8 +++- tests/test_middleware_priority.py | 7 +++ 4 files changed, 63 insertions(+), 42 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 26f1b9d91f..246e7c403e 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -759,48 +759,50 @@ async def handle_exception( ) return - # -------------------------------------------- # - # Request Middleware - # -------------------------------------------- # - if run_middleware: - middleware = ( - request.route and request.route.extra.request_middleware - ) or self.request_middleware - response = await self._run_request_middleware(request, middleware) - # No middleware results - if not response: - try: + try: + # -------------------------------------------- # + # Request Middleware + # -------------------------------------------- # + if run_middleware: + middleware = ( + request.route and request.route.extra.request_middleware + ) or self.request_middleware + response = await self._run_request_middleware( + request, middleware + ) + # No middleware results + if not response: response = self.error_handler.response(request, exception) if isawaitable(response): response = await response - except Exception as e: - if isinstance(e, SanicException): - response = self.error_handler.default(request, e) - elif self.debug: - response = HTTPResponse( - ( - f"Error while handling error: {e}\n" - f"Stack: {format_exc()}" - ), - status=500, - ) - else: - response = HTTPResponse( - "An error occurred while handling an error", status=500 - ) - if response is not None: - try: - request.reset_response() - response = await request.respond(response) - except BaseException: - # Skip response middleware + if response is not None: + try: + request.reset_response() + response = await request.respond(response) + except BaseException: + # Skip response middleware + if request.stream: + request.stream.respond(response) + await response.send(end_stream=True) + raise + else: if request.stream: - request.stream.respond(response) - await response.send(end_stream=True) - raise - else: - if request.stream: - response = request.stream.response + response = request.stream.response + except Exception as e: + if isinstance(e, SanicException): + response = self.error_handler.default(request, e) + elif self.debug: + response = HTTPResponse( + ( + f"Error while handling error: {e}\n" + f"Stack: {format_exc()}" + ), + status=500, + ) + else: + response = HTTPResponse( + "An error occurred while handling an error", status=500 + ) # Marked for cleanup and DRY with handle_request/handle_exception # when ResponseStream is no longer supporder @@ -980,6 +982,7 @@ async def handle_request(self, request: Request): # no cov except CancelledError: raise except Exception as e: + print(f">>>>>>>>>>>>>>>>> {run_middleware=}") # Response Generation Failed await self.handle_exception( request, e, run_middleware=run_middleware diff --git a/sanic/middleware.py b/sanic/middleware.py index 02a2ce9b87..5bbd777b6c 100644 --- a/sanic/middleware.py +++ b/sanic/middleware.py @@ -3,7 +3,7 @@ from collections import deque from enum import IntEnum, auto from itertools import count -from typing import Deque, Optional, Sequence, Union +from typing import Deque, Sequence, Union from sanic.models.handler_types import MiddlewareType @@ -14,7 +14,7 @@ class MiddlewareLocation(IntEnum): class Middleware: - counter = count() + _counter = count() __slots__ = ("func", "priority", "location", "definition") @@ -27,7 +27,7 @@ def __init__( self.func = func self.priority = priority self.location = location - self.definition = next(Middleware.counter) + self.definition = next(Middleware._counter) def __call__(self, *args, **kwargs): return self.func(*args, **kwargs) @@ -59,3 +59,8 @@ def convert( for middleware in collection ] ) + + @classmethod + def reset_count(cls): + cls._counter = count() + cls.count = next(cls._counter) diff --git a/tests/test_app.py b/tests/test_app.py index df7f238ff6..333ea51a3b 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -18,6 +18,7 @@ from sanic.helpers import _default from sanic.log import LOGGING_CONFIG_DEFAULTS from sanic.response import text +from sanic.router import Route @pytest.fixture(autouse=True) @@ -152,8 +153,13 @@ async def handler(): def test_app_handle_request_handler_is_none(app: Sanic, monkeypatch): + app.config.TOUCHUP = False + route = Mock(spec=Route) + route.extra.request_middleware = [] + route.extra.response_middleware = [] + def mockreturn(*args, **kwargs): - return Mock(), None, {} + return route, None, {} monkeypatch.setattr(app.router, "get", mockreturn) diff --git a/tests/test_middleware_priority.py b/tests/test_middleware_priority.py index fe40e2cce9..9646f6d0f9 100644 --- a/tests/test_middleware_priority.py +++ b/tests/test_middleware_priority.py @@ -3,6 +3,7 @@ import pytest from sanic import Sanic +from sanic.middleware import Middleware from sanic.response import json @@ -33,6 +34,12 @@ ) +@pytest.fixture(autouse=True) +def reset_middleware(): + yield + Middleware.reset_count() + + @pytest.mark.parametrize( "expected,priorities", PRIORITY_TEST_CASES, From 485b615f5ec1c95c3429a23095a933c94a4adae0 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 21 Sep 2022 09:58:25 +0300 Subject: [PATCH 07/10] Fix tests --- sanic/app.py | 124 +++++++++++++++++++++++++++++--------------- sanic/asgi.py | 5 +- sanic/http/http1.py | 5 +- 3 files changed, 91 insertions(+), 43 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 246e7c403e..14a5b59a71 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -759,50 +759,93 @@ async def handle_exception( ) return - try: - # -------------------------------------------- # - # Request Middleware - # -------------------------------------------- # - if run_middleware: - middleware = ( - request.route and request.route.extra.request_middleware - ) or self.request_middleware - response = await self._run_request_middleware( - request, middleware - ) - # No middleware results - if not response: + # try: + # # -------------------------------------------- # + # # Request Middleware + # # -------------------------------------------- # + # if run_middleware: + # middleware = ( + # request.route and request.route.extra.request_middleware + # ) or self.request_middleware + # response = await self._run_request_middleware( + # request, middleware + # ) + # # No middleware results + # if not response: + # response = self.error_handler.response(request, exception) + # if isawaitable(response): + # response = await response + # if response is not None: + # try: + # request.reset_response() + # response = await request.respond(response) + # except BaseException: + # # Skip response middleware + # if request.stream: + # request.stream.respond(response) + # await response.send(end_stream=True) + # raise + # else: + # if request.stream: + # response = request.stream.response + # except Exception as e: + # if isinstance(e, SanicException): + # response = self.error_handler.default(request, e) + # elif self.debug: + # response = HTTPResponse( + # ( + # f"Error while handling error: {e}\n" + # f"Stack: {format_exc()}" + # ), + # status=500, + # ) + # else: + # response = HTTPResponse( + # "An error occurred while handling an error", status=500 + # ) + + # -------------------------------------------- # + # Request Middleware + # -------------------------------------------- # + if run_middleware: + middleware = ( + request.route and request.route.extra.request_middleware + ) or self.request_middleware + response = await self._run_request_middleware(request, middleware) + # No middleware results + if not response: + try: response = self.error_handler.response(request, exception) if isawaitable(response): response = await response - if response is not None: - try: - request.reset_response() - response = await request.respond(response) - except BaseException: - # Skip response middleware - if request.stream: - request.stream.respond(response) - await response.send(end_stream=True) - raise - else: + except Exception as e: + if isinstance(e, SanicException): + response = self.error_handler.default(request, e) + elif self.debug: + response = HTTPResponse( + ( + f"Error while handling error: {e}\n" + f"Stack: {format_exc()}" + ), + status=500, + ) + else: + response = HTTPResponse( + "An error occurred while handling an error", status=500 + ) + if response is not None: + try: + request.reset_response() + response = await request.respond(response) + except BaseException: + # Skip response middleware if request.stream: - response = request.stream.response - except Exception as e: - if isinstance(e, SanicException): - response = self.error_handler.default(request, e) - elif self.debug: - response = HTTPResponse( - ( - f"Error while handling error: {e}\n" - f"Stack: {format_exc()}" - ), - status=500, - ) - else: - response = HTTPResponse( - "An error occurred while handling an error", status=500 - ) + request.stream.respond(response) + await response.send(end_stream=True) + raise + else: + if request.stream: + response = request.stream.response # Marked for cleanup and DRY with handle_request/handle_exception # when ResponseStream is no longer supporder @@ -982,7 +1025,6 @@ async def handle_request(self, request: Request): # no cov except CancelledError: raise except Exception as e: - print(f">>>>>>>>>>>>>>>>> {run_middleware=}") # Response Generation Failed await self.handle_exception( request, e, run_middleware=run_middleware diff --git a/sanic/asgi.py b/sanic/asgi.py index 10357ae876..de61ed68a5 100644 --- a/sanic/asgi.py +++ b/sanic/asgi.py @@ -234,4 +234,7 @@ async def __call__(self) -> None: self.stage = Stage.HANDLER await self.sanic_app.handle_request(self.request) except Exception as e: - await self.sanic_app.handle_exception(self.request, e) + try: + await self.sanic_app.handle_exception(self.request, e) + except Exception as exc: + await self.sanic_app.handle_exception(self.request, exc, False) diff --git a/sanic/http/http1.py b/sanic/http/http1.py index 1f2423ad07..ccfae75de6 100644 --- a/sanic/http/http1.py +++ b/sanic/http/http1.py @@ -428,7 +428,10 @@ async def error_response(self, exception: Exception) -> None: if self.request is None: self.create_empty_request() - await app.handle_exception(self.request, exception) + try: + await app.handle_exception(self.request, exception) + except Exception as e: + await app.handle_exception(self.request, e, False) def create_empty_request(self) -> None: """ From 6813c48f083e9af43962fdb77c0e191c7e2cf04c Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 21 Sep 2022 10:02:54 +0300 Subject: [PATCH 08/10] squash --- sanic/app.py | 45 --------------------------------------------- 1 file changed, 45 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 14a5b59a71..26f1b9d91f 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -759,51 +759,6 @@ async def handle_exception( ) return - # try: - # # -------------------------------------------- # - # # Request Middleware - # # -------------------------------------------- # - # if run_middleware: - # middleware = ( - # request.route and request.route.extra.request_middleware - # ) or self.request_middleware - # response = await self._run_request_middleware( - # request, middleware - # ) - # # No middleware results - # if not response: - # response = self.error_handler.response(request, exception) - # if isawaitable(response): - # response = await response - # if response is not None: - # try: - # request.reset_response() - # response = await request.respond(response) - # except BaseException: - # # Skip response middleware - # if request.stream: - # request.stream.respond(response) - # await response.send(end_stream=True) - # raise - # else: - # if request.stream: - # response = request.stream.response - # except Exception as e: - # if isinstance(e, SanicException): - # response = self.error_handler.default(request, e) - # elif self.debug: - # response = HTTPResponse( - # ( - # f"Error while handling error: {e}\n" - # f"Stack: {format_exc()}" - # ), - # status=500, - # ) - # else: - # response = HTTPResponse( - # "An error occurred while handling an error", status=500 - # ) - # -------------------------------------------- # # Request Middleware # -------------------------------------------- # From 4e271c5040e697ebc8b61b57bd4eee21a0d6352a Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Wed, 21 Sep 2022 10:09:25 +0300 Subject: [PATCH 09/10] Update sanic-testing version --- sanic/request.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sanic/request.py b/sanic/request.py index 552ea95567..f7b3b9993e 100644 --- a/sanic/request.py +++ b/sanic/request.py @@ -225,7 +225,7 @@ def request_middleware_started(self): "Request.request_middleware_started has been deprecated and will" "be removed. You should set a flag on the request context using" "either middleware or signals if you need this feature.", - 22.3, + 23.3, ) return self._request_middleware_started diff --git a/setup.py b/setup.py index d75b315a2f..c7d2c0e267 100644 --- a/setup.py +++ b/setup.py @@ -94,7 +94,7 @@ def open_local(paths, mode="r", encoding="utf8"): ] tests_require = [ - "sanic-testing>=22.9.0b1", + "sanic-testing>=22.9.0b2", "pytest", "coverage", "beautifulsoup4", From 2c41fc57d0fd34504ff8be32de4774969b63901a Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Thu, 22 Sep 2022 00:32:03 +0300 Subject: [PATCH 10/10] Rename inspector method --- sanic/worker/inspector.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sanic/worker/inspector.py b/sanic/worker/inspector.py index 6c9869ee18..e49b6851f4 100644 --- a/sanic/worker/inspector.py +++ b/sanic/worker/inspector.py @@ -73,7 +73,7 @@ def stop(self, *_): def state_to_json(self): output = {"info": self.app_info} - output["workers"] = self._make_safe(dict(self.worker_state)) + output["workers"] = self.make_safe(dict(self.worker_state)) return output def reload(self): @@ -84,10 +84,11 @@ def shutdown(self): message = "__TERMINATE__" self._publisher.send(message) - def _make_safe(self, obj: Dict[str, Any]) -> Dict[str, Any]: + @staticmethod + def make_safe(obj: Dict[str, Any]) -> Dict[str, Any]: for key, value in obj.items(): if isinstance(value, dict): - obj[key] = self._make_safe(value) + obj[key] = Inspector.make_safe(value) elif isinstance(value, datetime): obj[key] = value.isoformat() return obj