Skip to content

Commit

Permalink
Allow non-conforming ErrorHandlers (#2259)
Browse files Browse the repository at this point in the history
* Allow non-conforming ErrorHandlers

* Rename to legacy lookup

* Updated depnotice

* Bump version

* Fix formatting

* Remove unused import

* Fix error messages
  • Loading branch information
ahopkins authored Oct 2, 2021
1 parent 50a606a commit 5e12edb
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 5 deletions.
2 changes: 1 addition & 1 deletion sanic/__version__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "21.9.0"
__version__ = "21.9.1"
1 change: 1 addition & 0 deletions sanic/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,7 @@ def signalize(self):
async def _startup(self):
self.signalize()
self.finalize()
ErrorHandler.finalize(self.error_handler)
TouchUp.run(self)

async def _server_event(
Expand Down
39 changes: 36 additions & 3 deletions sanic/handlers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from inspect import signature
from typing import Dict, List, Optional, Tuple, Type

from sanic.errorpages import BaseRenderer, HTMLRenderer, exception_response
Expand Down Expand Up @@ -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]
Expand All @@ -34,6 +37,34 @@ def __init__(self, fallback: str, base: Type[BaseRenderer] = HTMLRenderer):
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.
Expand All @@ -56,7 +87,7 @@ def add(self, exception, handler, route_names: Optional[List[str]] = None):
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.
Expand Down Expand Up @@ -94,6 +125,8 @@ def lookup(self, exception, route_name: Optional[str]):
handler = None
return handler

_lookup = _full_lookup

def response(self, request, exception):
"""Fetches and executes an exception handler and returns a response
object
Expand All @@ -109,7 +142,7 @@ def response(self, request, exception):
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:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytest

from bs4 import BeautifulSoup
from websockets.version import version as websockets_version

from sanic import Sanic
from sanic.exceptions import (
Expand All @@ -16,7 +17,6 @@
abort,
)
from sanic.response import text
from websockets.version import version as websockets_version


class SanicExceptionTestException(Exception):
Expand Down
21 changes: 21 additions & 0 deletions tests/test_exceptions_handler.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import asyncio
import logging

import pytest

Expand Down Expand Up @@ -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

0 comments on commit 5e12edb

Please sign in to comment.