diff --git a/sanic/handlers.py b/sanic/handlers.py index 13bcff94..d77b183e 100644 --- a/sanic/handlers.py +++ b/sanic/handlers.py @@ -47,6 +47,28 @@ class ErrorHandler: def _full_lookup(self, exception, route_name: Optional[str] = None): return self.lookup(exception, route_name) + def _add( + self, + key: Tuple[Type[BaseException], Optional[str]], + handler: RouteHandler, + ) -> None: + if key in self.cached_handlers: + exc, name = key + if name is None: + name = "__ALL_ROUTES__" + + error_logger.warning( + f"Duplicate exception handler definition on: route={name} " + f"and exception={exc}" + ) + deprecation( + "A duplicate exception handler definition was discovered. " + "This may cause unintended consequences. A warning has been " + "issued now, but it will not be allowed starting in v23.3.", + 23.3, + ) + self.cached_handlers[key] = handler + def add(self, exception, handler, route_names: Optional[List[str]] = None): """ Add a new exception handler to an already existing handler object. @@ -62,9 +84,9 @@ class ErrorHandler: """ if route_names: for route in route_names: - self.cached_handlers[(exception, route)] = handler + self._add((exception, route), handler) else: - self.cached_handlers[(exception, None)] = handler + self._add((exception, None), handler) def lookup(self, exception, route_name: Optional[str] = None): """ diff --git a/tests/test_exceptions_handler.py b/tests/test_exceptions_handler.py index c505cead..56dabbfb 100644 --- a/tests/test_exceptions_handler.py +++ b/tests/test_exceptions_handler.py @@ -7,7 +7,7 @@ from unittest.mock import Mock import pytest from bs4 import BeautifulSoup -from pytest import LogCaptureFixture, MonkeyPatch +from pytest import LogCaptureFixture, MonkeyPatch, WarningsRecorder from sanic import Sanic, handlers from sanic.exceptions import BadRequest, Forbidden, NotFound, ServerError @@ -266,3 +266,22 @@ def test_exception_handler_response_was_sent( _, response = app.test_client.get("/2") assert "Error" in response.text + + +def test_warn_on_duplicate( + app: Sanic, caplog: LogCaptureFixture, recwarn: WarningsRecorder +): + @app.exception(ServerError) + async def exception_handler_1(request, exception): + ... + + @app.exception(ServerError) + async def exception_handler_2(request, exception): + ... + + assert len(caplog.records) == 1 + assert len(recwarn) == 1 + assert caplog.records[0].message == ( + "Duplicate exception handler definition on: route=__ALL_ROUTES__ and " + "exception=" + )