From 5e12edbc38400d050ee3485475172a21556b31c7 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 3 Oct 2021 01:02:56 +0300 Subject: [PATCH] Allow non-conforming ErrorHandlers (#2259) * Allow non-conforming ErrorHandlers * Rename to legacy lookup * Updated depnotice * Bump version * Fix formatting * Remove unused import * Fix error messages --- sanic/__version__.py | 2 +- sanic/app.py | 1 + sanic/handlers.py | 39 +++++++++++++++++++++++++++++--- tests/test_exceptions.py | 2 +- tests/test_exceptions_handler.py | 21 +++++++++++++++++ 5 files changed, 60 insertions(+), 5 deletions(-) diff --git a/sanic/__version__.py b/sanic/__version__.py index 32566438..529bc4a9 100644 --- a/sanic/__version__.py +++ b/sanic/__version__.py @@ -1 +1 @@ -__version__ = "21.9.0" +__version__ = "21.9.1" diff --git a/sanic/app.py b/sanic/app.py index 01aa07cb..0686f7ed 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -1474,6 +1474,7 @@ class Sanic(BaseSanic, metaclass=TouchUpMeta): async def _startup(self): self.signalize() self.finalize() + ErrorHandler.finalize(self.error_handler) TouchUp.run(self) async def _server_event( diff --git a/sanic/handlers.py b/sanic/handlers.py index ffeb76b8..fd718f06 100644 --- a/sanic/handlers.py +++ b/sanic/handlers.py @@ -1,3 +1,4 @@ +from inspect import signature from typing import Dict, List, Optional, Tuple, Type from sanic.errorpages import BaseRenderer, HTMLRenderer, exception_response @@ -25,7 +26,9 @@ class ErrorHandler: """ # Beginning in v22.3, the base renderer will be TextRenderer - def __init__(self, fallback: str, base: Type[BaseRenderer] = HTMLRenderer): + def __init__( + self, fallback: str = "auto", base: Type[BaseRenderer] = HTMLRenderer + ): self.handlers: List[Tuple[Type[BaseException], RouteHandler]] = [] self.cached_handlers: Dict[ Tuple[Type[BaseException], Optional[str]], Optional[RouteHandler] @@ -34,6 +37,34 @@ class ErrorHandler: self.fallback = fallback self.base = base + @classmethod + def finalize(cls, error_handler): + if not isinstance(error_handler, cls): + error_logger.warning( + f"Error handler is non-conforming: {type(error_handler)}" + ) + + sig = signature(error_handler.lookup) + if len(sig.parameters) == 1: + error_logger.warning( + DeprecationWarning( + "You are using a deprecated error handler. The lookup " + "method should accept two positional parameters: " + "(exception, route_name: Optional[str]). " + "Until you upgrade your ErrorHandler.lookup, Blueprint " + "specific exceptions will not work properly. Beginning " + "in v22.3, the legacy style lookup method will not " + "work at all." + ), + ) + error_handler._lookup = error_handler._legacy_lookup + + def _full_lookup(self, exception, route_name: Optional[str] = None): + return self.lookup(exception, route_name) + + def _legacy_lookup(self, exception, route_name: Optional[str] = None): + return self.lookup(exception) + def add(self, exception, handler, route_names: Optional[List[str]] = None): """ Add a new exception handler to an already existing handler object. @@ -56,7 +87,7 @@ class ErrorHandler: else: self.cached_handlers[(exception, None)] = handler - def lookup(self, exception, route_name: Optional[str]): + def lookup(self, exception, route_name: Optional[str] = None): """ Lookup the existing instance of :class:`ErrorHandler` and fetch the registered handler for a specific type of exception. @@ -94,6 +125,8 @@ class ErrorHandler: handler = None return handler + _lookup = _full_lookup + def response(self, request, exception): """Fetches and executes an exception handler and returns a response object @@ -109,7 +142,7 @@ class ErrorHandler: or registered handler for that type of exception. """ route_name = request.name if request else None - handler = self.lookup(exception, route_name) + handler = self._lookup(exception, route_name) response = None try: if handler: diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 29797e1e..503e47cb 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -4,6 +4,7 @@ import warnings import pytest from bs4 import BeautifulSoup +from websockets.version import version as websockets_version from sanic import Sanic from sanic.exceptions import ( @@ -16,7 +17,6 @@ from sanic.exceptions import ( abort, ) from sanic.response import text -from websockets.version import version as websockets_version class SanicExceptionTestException(Exception): diff --git a/tests/test_exceptions_handler.py b/tests/test_exceptions_handler.py index dbf9fcbb..9bedf7e6 100644 --- a/tests/test_exceptions_handler.py +++ b/tests/test_exceptions_handler.py @@ -1,4 +1,5 @@ import asyncio +import logging import pytest @@ -206,3 +207,23 @@ def test_exception_handler_processed_request_middleware(exception_handler_app): request, response = exception_handler_app.test_client.get("/8") assert response.status == 200 assert response.text == "Done." + + +def test_single_arg_exception_handler_notice(exception_handler_app, caplog): + class CustomErrorHandler(ErrorHandler): + def lookup(self, exception): + return super().lookup(exception, None) + + exception_handler_app.error_handler = CustomErrorHandler() + + with caplog.at_level(logging.WARNING): + _, response = exception_handler_app.test_client.get("/1") + + assert caplog.records[0].message == ( + "You are using a deprecated error handler. The lookup method should " + "accept two positional parameters: (exception, route_name: " + "Optional[str]). Until you upgrade your ErrorHandler.lookup, " + "Blueprint specific exceptions will not work properly. Beginning in " + "v22.3, the legacy style lookup method will not work at all." + ) + assert response.status == 400