diff --git a/sanic/app.py b/sanic/app.py index 437f8eb0..763ccee5 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -21,7 +21,6 @@ from functools import partial from inspect import isawaitable from os import environ from socket import socket -from traceback import format_exc from types import SimpleNamespace from typing import ( TYPE_CHECKING, @@ -54,13 +53,8 @@ from sanic.blueprint_group import BlueprintGroup from sanic.blueprints import Blueprint from sanic.compat import OS_IS_WINDOWS, enable_windows_color_support from sanic.config import SANIC_PREFIX, Config -from sanic.exceptions import ( - BadRequest, - SanicException, - ServerError, - URLBuildError, -) -from sanic.handlers import ErrorHandler, RequestManager +from sanic.exceptions import BadRequest, SanicException, URLBuildError +from sanic.handlers import ErrorHandler from sanic.helpers import _default from sanic.http import Stage from sanic.log import ( @@ -83,7 +77,7 @@ from sanic.models.futures import ( from sanic.models.handler_types import ListenerType, MiddlewareType from sanic.models.handler_types import Sanic as SanicVar from sanic.request import Request -from sanic.response import BaseHTTPResponse, HTTPResponse, ResponseStream +from sanic.response import BaseHTTPResponse from sanic.router import Router from sanic.server.websockets.impl import ConnectionClosed from sanic.signals import Signal, SignalRouter @@ -716,284 +710,8 @@ class Sanic(BaseSanic, StartupMixin, metaclass=TouchUpMeta): ): # no cov raise NotImplementedError - async def _handle_exception( - self, - request: Request, - exception: BaseException, - run_middleware: bool = True, - ): # no cov - """ - A handler that catches specific exceptions and outputs a response. - - :param request: The current request object - :param exception: The exception that was raised - :raises ServerError: response 500 - """ - response = None - await self.dispatch( - "http.lifecycle.exception", - inline=True, - context={"request": request, "exception": exception}, - ) - - if ( - request.stream is not None - and request.stream.stage is not Stage.HANDLER - ): - error_logger.exception(exception, exc_info=True) - logger.error( - "The error response will not be sent to the client for " - f'the following exception:"{exception}". A previous response ' - "has at least partially been sent." - ) - - handler = self.error_handler._lookup( - exception, request.name if request else None - ) - if handler: - logger.warning( - "An error occurred while handling the request after at " - "least some part of the response was sent to the client. " - "The response from your custom exception handler " - f"{handler.__name__} will not be sent to the client." - "Exception handlers should only be used to generate the " - "exception responses. If you would like to perform any " - "other action on a raised exception, consider using a " - "signal handler like " - '`@app.signal("http.lifecycle.exception")`\n' - "For further information, please see the docs: " - "https://sanicframework.org/en/guide/advanced/" - "signals.html", - ) - 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: - 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 request.stream: - 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 - if isinstance(response, BaseHTTPResponse): - await self.dispatch( - "http.lifecycle.response", - inline=True, - context={ - "request": request, - "response": response, - }, - ) - await response.send(end_stream=True) - elif isinstance(response, ResponseStream): - resp = await response(request) - await self.dispatch( - "http.lifecycle.response", - inline=True, - context={ - "request": request, - "response": resp, - }, - ) - await response.eof() - else: - raise ServerError( - f"Invalid response type {response!r} (need HTTPResponse)" - ) - async def handle_request(self, request: Request): # no cov - """Take a request from the HTTP Server and return a response object - to be sent back The HTTP Server only expects a response object, so - exception handling must be done here - - :param request: HTTP Request object - :return: Nothing - """ - - async def _handle_request(self, request: Request): # no cov - await self.dispatch( - "http.lifecycle.handle", - inline=True, - context={"request": request}, - ) - - # Define `response` var here to remove warnings about - # allocation before assignment below. - response: Optional[ - Union[ - BaseHTTPResponse, - Coroutine[Any, Any, Optional[BaseHTTPResponse]], - ] - ] = None - run_middleware = True - try: - - await self.dispatch( - "http.routing.before", - inline=True, - context={"request": request}, - ) - # Fetch handler from router - route, handler, kwargs = self.router.get( - request.path, - request.method, - request.headers.getone("host", None), - ) - - request._match_info = {**kwargs} - request.route = route - - await self.dispatch( - "http.routing.after", - inline=True, - context={ - "request": request, - "route": route, - "kwargs": kwargs, - "handler": handler, - }, - ) - - if ( - request.stream - and request.stream.request_body - and not route.ctx.ignore_body - ): - - if hasattr(handler, "is_stream"): - # Streaming handler: lift the size limit - request.stream.request_max_size = float("inf") - else: - # Non-streaming handler: preload body - await request.receive_body() - - # -------------------------------------------- # - # Request Middleware - # -------------------------------------------- # - run_middleware = False - if request.route.extra.request_middleware: - response = await self._run_request_middleware( - request, request.route.extra.request_middleware - ) - - # No middleware results - if not response: - # -------------------------------------------- # - # Execute Handler - # -------------------------------------------- # - - if handler is None: - raise ServerError( - ( - "'None' was returned while requesting a " - "handler from the router" - ) - ) - - # Run response handler - await self.dispatch( - "http.handler.before", - inline=True, - context={"request": request}, - ) - response = handler(request, **request.match_info) - if isawaitable(response): - response = await response - await self.dispatch( - "http.handler.after", - inline=True, - context={"request": request}, - ) - - if request.responded: - if response is not None: - error_logger.error( - "The response object returned by the route handler " - "will not be sent to client. The request has already " - "been responded to." - ) - if request.stream is not None: - response = request.stream.response - elif response is not None: - response = await request.respond(response) # type: ignore - elif not hasattr(handler, "is_websocket"): - response = request.stream.response # type: ignore - - # Marked for cleanup and DRY with handle_request/handle_exception - # when ResponseStream is no longer supporder - if isinstance(response, BaseHTTPResponse): - await self.dispatch( - "http.lifecycle.response", - inline=True, - context={ - "request": request, - "response": response, - }, - ) - ... - await response.send(end_stream=True) - elif isinstance(response, ResponseStream): - resp = await response(request) # type: ignore - await self.dispatch( - "http.lifecycle.response", - inline=True, - context={ - "request": request, - "response": resp, - }, - ) - await response.eof() # type: ignore - else: - if not hasattr(handler, "is_websocket"): - raise ServerError( - f"Invalid response type {response!r} " - "(need HTTPResponse)" - ) - - except CancelledError: - raise - except Exception as e: - # Response Generation Failed - await self.handle_exception( - request, e, run_middleware=run_middleware - ) + raise NotImplementedError async def _websocket_handler( self, handler, request, *args, subprotocols=None, **kwargs diff --git a/sanic/handlers.py b/sanic/handlers.py index 75f04f5b..6570b7a7 100644 --- a/sanic/handlers.py +++ b/sanic/handlers.py @@ -20,11 +20,12 @@ from sanic.log import deprecation, error_logger, logger from sanic.models.handler_types import RouteHandler from sanic.request import Request from sanic.response import BaseHTTPResponse, HTTPResponse, ResponseStream, text +from sanic.touchup import TouchUpMeta class RequestHandler: def __init__(self, func, request_middleware, response_middleware): - self.func = func + self.func = func.func if isinstance(func, RequestHandler) else func self.request_middleware = request_middleware self.response_middleware = response_middleware @@ -32,7 +33,20 @@ class RequestHandler: return self.func(*args, **kwargs) -class RequestManager: +class RequestManager(metaclass=TouchUpMeta): + __touchup__ = ( + "cleanup", + "run_request_middleware", + "run_response_middleware", + ) + __slots__ = ( + "handler", + "request_middleware_run", + "request_middleware", + "request", + "response_middleware_run", + "response_middleware", + ) request: Request def __init__(self, request: Request): @@ -76,28 +90,35 @@ class RequestManager: partial(self.handler, self.request, **self.request.match_info) ) - async def lifecycle(self, handler): + async def lifecycle(self, handler, raise_exception: bool = False): response: Optional[BaseHTTPResponse] = None if not self.request_middleware_run and self.request_middleware: - response = await self.run(self.run_request_middleware) + response = await self.run( + self.run_request_middleware, raise_exception + ) if not response: # Run response handler - response = await self.run(handler) + response = await self.run(handler, raise_exception) if not self.response_middleware_run and self.response_middleware: response = await self.run( - partial(self.run_response_middleware, response) + partial(self.run_response_middleware, response), + raise_exception, ) await self.cleanup(response) - async def run(self, operation) -> Optional[BaseHTTPResponse]: + async def run( + self, operation, raise_exception: bool = False + ) -> Optional[BaseHTTPResponse]: try: response = operation() if isawaitable(response): response = await response except Exception as e: + if raise_exception: + raise response = await self.error(e) return response @@ -136,12 +157,9 @@ class RequestManager: try: await self.lifecycle( - partial(error_handler.response, self.request, exception) + partial(error_handler.response, self.request, exception), True ) except Exception as e: - await self.lifecycle( - partial(error_handler.default, self.request, e) - ) if isinstance(e, SanicException): response = error_handler.default(self.request, e) elif self.request.app.debug: @@ -153,6 +171,7 @@ class RequestManager: status=500, ) else: + error_logger.exception(e) response = HTTPResponse( "An error occurred while handling an error", status=500 ) @@ -175,30 +194,21 @@ class RequestManager: elif not hasattr(self.handler, "is_websocket"): response = self.request.stream.response # type: ignore - # Marked for cleanup and DRY with handle_request/handle_exception - # when ResponseStream is no longer supporder if isinstance(response, BaseHTTPResponse): - # await self.dispatch( - # "http.lifecycle.response", - # inline=True, - # context={ - # "request": self.request, - # "response": response, - # }, - # ) - ... + await self.request.app.dispatch( + "http.lifecycle.response", + inline=True, + context={"request": self.request, "response": response}, + ) await response.send(end_stream=True) elif isinstance(response, ResponseStream): await response(self.request) # type: ignore - # await self.dispatch( - # "http.lifecycle.response", - # inline=True, - # context={ - # "request": self.request, - # "response": resp, - # }, - # ) await response.eof() # type: ignore + await self.request.app.dispatch( + "http.lifecycle.response", + inline=True, + context={"request": self.request, "response": response}, + ) else: if not hasattr(self.handler, "is_websocket"): raise ServerError( @@ -219,27 +229,27 @@ class RequestManager: self.request_middleware_run = True for middleware in self.request_middleware: - # await self.dispatch( - # "http.middleware.before", - # inline=True, - # context={ - # "request": request, - # "response": None, - # }, - # condition={"attach_to": "request"}, - # ) + await self.request.app.dispatch( + "http.middleware.before", + inline=True, + context={"request": self.request, "response": None}, + condition={"attach_to": "request"}, + ) - response = await self.run(partial(middleware, self.request)) + try: + response = await self.run(partial(middleware, self.request)) + except Exception: + error_logger.exception( + "Exception occurred in one of request middleware handlers" + ) + raise - # await self.dispatch( - # "http.middleware.after", - # inline=True, - # context={ - # "request": request, - # "response": None, - # }, - # condition={"attach_to": "request"}, - # ) + await self.request.app.dispatch( + "http.middleware.after", + inline=True, + context={"request": self.request, "response": None}, + condition={"attach_to": "request"}, + ) if response: return response @@ -250,46 +260,34 @@ class RequestManager: ) -> BaseHTTPResponse: self.response_middleware_run = True for middleware in self.response_middleware: - # await self.dispatch( - # "http.middleware.before", - # inline=True, - # context={ - # "request": request, - # "response": None, - # }, - # condition={"attach_to": "request"}, - # ) + await self.request.app.dispatch( + "http.middleware.before", + inline=True, + context={"request": self.request, "response": None}, + condition={"attach_to": "request"}, + ) - resp = await self.run(partial(middleware, self.request, response)) + try: + resp = await self.run( + partial(middleware, self.request, response), True + ) + except Exception as e: + error_logger.exception( + "Exception occurred in one of response middleware handlers" + ) + await self.error(e) + resp = None - # await self.dispatch( - # "http.middleware.after", - # inline=True, - # context={ - # "request": request, - # "response": None, - # }, - # condition={"attach_to": "request"}, - # ) + await self.request.app.dispatch( + "http.middleware.after", + inline=True, + context={"request": self.request, "response": None}, + condition={"attach_to": "request"}, + ) if resp: return resp return response - # try: - # 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, middleware - # ) - # except CancelledErrors: - # raise - # except Exception: - # error_logger.exception( - # "Exception occurred in one of response middleware handlers" - # ) - # return None def resolve_route(self) -> Route: # Fetch handler from router @@ -303,11 +301,11 @@ class RequestManager: self.request.route = route self.handler = handler - if route.handler and route.handler.request_middleware: - self.request_middleware = route.handler.request_middleware + if handler and handler.request_middleware: + self.request_middleware = handler.request_middleware - if route.handler and route.handler.response_middleware: - self.response_middleware = route.handler.response_middleware + if handler and handler.response_middleware: + self.response_middleware = handler.response_middleware return route diff --git a/sanic/middleware.py b/sanic/middleware.py index 377bb9c0..0319a68a 100644 --- a/sanic/middleware.py +++ b/sanic/middleware.py @@ -14,7 +14,7 @@ class MiddlewareLocation(IntEnum): class Middleware: - counter = count() + _counter = count() __slots__ = ("func", "priority", "location", "definition") @@ -27,7 +27,7 @@ class Middleware: 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) @@ -60,3 +60,7 @@ class Middleware: for middleware in collection ] ) + + @classmethod + def reset_count(cls): + cls._counter = count() diff --git a/sanic/router.py b/sanic/router.py index ec4d852f..f033e9ea 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -13,6 +13,7 @@ from sanic_routing.route import Route from sanic.constants import HTTP_METHODS from sanic.errorpages import check_error_format from sanic.exceptions import MethodNotAllowed, NotFound, SanicException +from sanic.handlers import RequestHandler from sanic.models.handler_types import RouteHandler @@ -31,9 +32,11 @@ class Router(BaseRouter): def _get( self, path: str, method: str, host: Optional[str] - ) -> Tuple[Route, RouteHandler, Dict[str, Any]]: + ) -> Tuple[Route, RequestHandler, Dict[str, Any]]: try: - return self.resolve( + # We know this will always be RequestHandler, so we can ignore + # typing issue here + return self.resolve( # type: ignore path=path, method=method, extra={"host": host} if host else None, @@ -50,7 +53,7 @@ class Router(BaseRouter): @lru_cache(maxsize=ROUTER_CACHE_SIZE) def get( # type: ignore self, path: str, method: str, host: Optional[str] - ) -> Tuple[Route, RouteHandler, Dict[str, Any]]: + ) -> Tuple[Route, RequestHandler, Dict[str, Any]]: """ Retrieve a `Route` object containing the details about how to handle a response for a given request @@ -59,7 +62,7 @@ class Router(BaseRouter): :type request: Request :return: details needed for handling the request and returning the correct response - :rtype: Tuple[ Route, RouteHandler, Dict[str, Any]] + :rtype: Tuple[ Route, RequestHandler, Dict[str, Any]] """ return self._get(path, method, host) @@ -114,7 +117,7 @@ class Router(BaseRouter): params = dict( path=uri, - handler=handler, + handler=RequestHandler(handler, [], []), methods=frozenset(map(str, methods)) if methods else None, name=name, strict=strict_slashes, diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 3924bf17..e9fd2aee 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -4,12 +4,20 @@ from asyncio import CancelledError from itertools import count from unittest.mock import Mock +import pytest + from sanic.exceptions import NotFound -from sanic.middleware import Middleware, MiddlewareLocation +from sanic.middleware import Middleware from sanic.request import Request from sanic.response import HTTPResponse, json, text +@pytest.fixture(autouse=True) +def reset_middleware(): + yield + Middleware.reset_count() + + # ------------------------------------------------------------ # # GET # ------------------------------------------------------------ # @@ -185,7 +193,7 @@ def test_middleware_response_raise_exception(app, caplog): with caplog.at_level(logging.ERROR): reqrequest, response = app.test_client.get("/fail") - assert response.status == 404 + assert response.status == 500 # 404 errors are not logged assert ( "sanic.error",