Remove app instance from Config for error handler setting (#2320)

sanic/app.py

@@ -197,9 +197,7 @@ class Sanic(BaseSanic, metaclass=TouchUpMeta):
         self._state: ApplicationState = ApplicationState(app=self)
         self.blueprints: Dict[str, Blueprint] = {}
         self.config: Config = config or Config(
-            load_env=load_env,
+            load_env=load_env, env_prefix=env_prefix
-            env_prefix=env_prefix,
-            app=self,
         )
         self.configure_logging: bool = configure_logging
         self.ctx: Any = ctx or SimpleNamespace()
@@ -1699,9 +1697,7 @@ class Sanic(BaseSanic, metaclass=TouchUpMeta):
         self._future_registry.clear()
         self.signalize()
         self.finalize()
-        ErrorHandler.finalize(
+        ErrorHandler.finalize(self.error_handler, config=self.config)
-            self.error_handler, fallback=self.config.FALLBACK_ERROR_FORMAT
-        )
         TouchUp.run(self)
         self.state.is_started = True
 

sanic/config.py

@@ -1,28 +1,26 @@
 from __future__ import annotations
 
-from inspect import isclass
+from inspect import getmembers, isclass, isdatadescriptor
 from os import environ
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Dict, Optional, Union
+from typing import Any, Dict, Optional, Union
 from warnings import warn
 
-from sanic.errorpages import check_error_format
+from sanic.errorpages import DEFAULT_FORMAT, check_error_format
+from sanic.helpers import _default
 from sanic.http import Http
+from sanic.log import error_logger
 from sanic.utils import load_module_from_file_location, str_to_bool
 
 
-if TYPE_CHECKING:  # no cov
-    from sanic import Sanic
-
-
 SANIC_PREFIX = "SANIC_"
 
 
 DEFAULT_CONFIG = {
+    "_FALLBACK_ERROR_FORMAT": _default,
     "ACCESS_LOG": True,
     "AUTO_RELOAD": False,
     "EVENT_AUTOREGISTER": False,
-    "FALLBACK_ERROR_FORMAT": "auto",
     "FORWARDED_FOR_HEADER": "X-Forwarded-For",
     "FORWARDED_SECRET": None,
     "GRACEFUL_SHUTDOWN_TIMEOUT": 15.0,  # 15 sec
@@ -46,11 +44,19 @@ DEFAULT_CONFIG = {
 }
 
 
-class Config(dict):
+class DescriptorMeta(type):
+    def __init__(cls, *_):
+        cls.__setters__ = {name for name, _ in getmembers(cls, cls._is_setter)}
+
+    @staticmethod
+    def _is_setter(member: object):
+        return isdatadescriptor(member) and hasattr(member, "setter")
+
+
+class Config(dict, metaclass=DescriptorMeta):
     ACCESS_LOG: bool
     AUTO_RELOAD: bool
     EVENT_AUTOREGISTER: bool
-    FALLBACK_ERROR_FORMAT: str
     FORWARDED_FOR_HEADER: str
     FORWARDED_SECRET: Optional[str]
     GRACEFUL_SHUTDOWN_TIMEOUT: float
@@ -79,13 +85,10 @@ class Config(dict):
         load_env: Optional[Union[bool, str]] = True,
         env_prefix: Optional[str] = SANIC_PREFIX,
         keep_alive: Optional[bool] = None,
-        *,
-        app: Optional[Sanic] = None,
     ):
         defaults = defaults or {}
         super().__init__({**DEFAULT_CONFIG, **defaults})
 
-        self._app = app
         self._LOGO = ""
 
         if keep_alive is not None:
@@ -117,6 +120,13 @@ class Config(dict):
             raise AttributeError(f"Config has no '{ke.args[0]}'")
 
     def __setattr__(self, attr, value) -> None:
+        if attr in self.__class__.__setters__:
+            try:
+                super().__setattr__(attr, value)
+            except AttributeError:
+                ...
+            else:
+                return None
         self.update({attr: value})
 
     def __setitem__(self, attr, value) -> None:
@@ -136,16 +146,6 @@ class Config(dict):
                 "REQUEST_MAX_SIZE",
             ):
                 self._configure_header_size()
-            elif attr == "FALLBACK_ERROR_FORMAT":
-                self._check_error_format()
-                if self.app and value != self.app.error_handler.fallback:
-                    if self.app.error_handler.fallback != "auto":
-                        warn(
-                            "Overriding non-default ErrorHandler fallback "
-                            "value. Changing from "
-                            f"{self.app.error_handler.fallback} to {value}."
-                        )
-                    self.app.error_handler.fallback = value
             elif attr == "LOGO":
                 self._LOGO = value
                 warn(
@@ -154,14 +154,29 @@ class Config(dict):
                     DeprecationWarning,
                 )
 
-    @property
-    def app(self):
-        return self._app
-
     @property
     def LOGO(self):
         return self._LOGO
 
+    @property
+    def FALLBACK_ERROR_FORMAT(self) -> str:
+        if self._FALLBACK_ERROR_FORMAT is _default:
+            return DEFAULT_FORMAT
+        return self._FALLBACK_ERROR_FORMAT
+
+    @FALLBACK_ERROR_FORMAT.setter
+    def FALLBACK_ERROR_FORMAT(self, value):
+        self._check_error_format(value)
+        if (
+            self._FALLBACK_ERROR_FORMAT is not _default
+            and value != self._FALLBACK_ERROR_FORMAT
+        ):
+            error_logger.warning(
+                "Setting config.FALLBACK_ERROR_FORMAT on an already "
+                "configured value may have unintended consequences."
+            )
+        self._FALLBACK_ERROR_FORMAT = value
+
     def _configure_header_size(self):
         Http.set_header_max_size(
             self.REQUEST_MAX_HEADER_SIZE,
@@ -169,8 +184,8 @@ class Config(dict):
             self.REQUEST_MAX_SIZE,
         )
 
-    def _check_error_format(self):
+    def _check_error_format(self, format: Optional[str] = None):
-        check_error_format(self.FALLBACK_ERROR_FORMAT)
+        check_error_format(format or self.FALLBACK_ERROR_FORMAT)
 
     def load_environment_vars(self, prefix=SANIC_PREFIX):
         """

sanic/errorpages.py

@@ -34,6 +34,7 @@ except ImportError:  # noqa
     from json import dumps
 
 
+DEFAULT_FORMAT = "auto"
 FALLBACK_TEXT = (
     "The server encountered an internal error and "
     "cannot complete your request."

sanic/handlers.py

@@ -1,13 +1,23 @@
+from __future__ import annotations
+
 from inspect import signature
-from typing import Dict, List, Optional, Tuple, Type
+from typing import Dict, List, Optional, Tuple, Type, Union
 from warnings import warn
 
-from sanic.errorpages import BaseRenderer, HTMLRenderer, exception_response
+from sanic.config import Config
+from sanic.errorpages import (
+    DEFAULT_FORMAT,
+    BaseRenderer,
+    HTMLRenderer,
+    exception_response,
+)
 from sanic.exceptions import (
     ContentRangeError,
     HeaderNotFound,
     InvalidRangeType,
+    SanicException,
 )
+from sanic.helpers import Default, _default
 from sanic.log import error_logger
 from sanic.models.handler_types import RouteHandler
 from sanic.response import text
@@ -28,24 +38,91 @@ class ErrorHandler:
 
     # Beginning in v22.3, the base renderer will be TextRenderer
     def __init__(
-        self, fallback: str = "auto", base: Type[BaseRenderer] = HTMLRenderer
+        self,
+        fallback: Union[str, Default] = _default,
+        base: Type[BaseRenderer] = HTMLRenderer,
     ):
         self.handlers: List[Tuple[Type[BaseException], RouteHandler]] = []
         self.cached_handlers: Dict[
             Tuple[Type[BaseException], Optional[str]], Optional[RouteHandler]
         ] = {}
         self.debug = False
-        self.fallback = fallback
+        self._fallback = fallback
         self.base = base
 
+        if fallback is not _default:
+            self._warn_fallback_deprecation()
+
+    @property
+    def fallback(self):
+        # This is for backwards compat and can be removed in v22.6
+        if self._fallback is _default:
+            return DEFAULT_FORMAT
+        return self._fallback
+
+    @fallback.setter
+    def fallback(self, value: str):
+        self._warn_fallback_deprecation()
+        if not isinstance(value, str):
+            raise SanicException(
+                f"Cannot set error handler fallback to: value={value}"
+            )
+        self._fallback = value
+
+    @staticmethod
+    def _warn_fallback_deprecation():
+        warn(
+            "Setting the ErrorHandler fallback value directly is "
+            "deprecated and no longer supported. This feature will "
+            "be removed in v22.6. Instead, use "
+            "app.config.FALLBACK_ERROR_FORMAT.",
+            DeprecationWarning,
+        )
+
     @classmethod
-    def finalize(cls, error_handler, fallback: Optional[str] = None):
+    def _get_fallback_value(cls, error_handler: ErrorHandler, config: Config):
-        if (
+        if error_handler._fallback is not _default:
-            fallback
+            if config._FALLBACK_ERROR_FORMAT is _default:
-            and fallback != "auto"
+                return error_handler.fallback
-            and error_handler.fallback == "auto"
+
-        ):
+            error_logger.warning(
-            error_handler.fallback = fallback
+                "Conflicting error fallback values were found in the "
+                "error handler and in the app.config while handling an "
+                "exception. Using the value from app.config."
+            )
+        return config.FALLBACK_ERROR_FORMAT
+
+    @classmethod
+    def finalize(
+        cls,
+        error_handler: ErrorHandler,
+        fallback: Optional[str] = None,
+        config: Optional[Config] = None,
+    ):
+        if fallback:
+            warn(
+                "Setting the ErrorHandler fallback value via finalize() "
+                "is deprecated and no longer supported. This feature will "
+                "be removed in v22.6. Instead, use "
+                "app.config.FALLBACK_ERROR_FORMAT.",
+                DeprecationWarning,
+            )
+
+        if config is None:
+            warn(
+                "Starting in v22.3, config will be a required argument "
+                "for ErrorHandler.finalize().",
+                DeprecationWarning,
+            )
+
+        if fallback and fallback != DEFAULT_FORMAT:
+            if error_handler._fallback is not _default:
+                error_logger.warning(
+                    f"Setting the fallback value to {fallback}. This changes "
+                    "the current non-default value "
+                    f"'{error_handler._fallback}'."
+                )
+            error_handler._fallback = fallback
 
         if not isinstance(error_handler, cls):
             error_logger.warning(
@@ -64,7 +141,8 @@ class ErrorHandler:
                 "work at all.",
                 DeprecationWarning,
             )
-            error_handler._lookup = error_handler._legacy_lookup
+            legacy_lookup = error_handler._legacy_lookup
+            error_handler._lookup = legacy_lookup  # type: ignore
 
     def _full_lookup(self, exception, route_name: Optional[str] = None):
         return self.lookup(exception, route_name)
@@ -188,12 +266,13 @@ class ErrorHandler:
         :return:
         """
         self.log(request, exception)
+        fallback = ErrorHandler._get_fallback_value(self, request.app.config)
         return exception_response(
             request,
             exception,
             debug=self.debug,
             base=self.base,
-            fallback=self.fallback,
+            fallback=fallback,
         )
 
     @staticmethod

tests/test_errorpages.py

@@ -280,40 +280,20 @@ def test_allow_fallback_error_format_set_main_process_start(app):
     async def start(app, _):
         app.config.FALLBACK_ERROR_FORMAT = "text"
 
-    request, response = app.test_client.get("/error")
+    _, response = app.test_client.get("/error")
-    assert request.app.error_handler.fallback == "text"
     assert response.status == 500
     assert response.content_type == "text/plain; charset=utf-8"
 
 
-def test_setting_fallback_to_non_default_raise_warning(app):
+def test_setting_fallback_on_config_changes_as_expected(app):
-    app.error_handler = ErrorHandler(fallback="text")
+    app.error_handler = ErrorHandler()
 
-    assert app.error_handler.fallback == "text"
+    _, response = app.test_client.get("/error")
-
+    assert response.content_type == "text/html; charset=utf-8"
-    with pytest.warns(
-        UserWarning,
-        match=(
-            "Overriding non-default ErrorHandler fallback value. "
-            "Changing from text to auto."
-        ),
-    ):
-        app.config.FALLBACK_ERROR_FORMAT = "auto"
-
-    assert app.error_handler.fallback == "auto"
 
     app.config.FALLBACK_ERROR_FORMAT = "text"
-
+    _, response = app.test_client.get("/error")
-    with pytest.warns(
+    assert response.content_type == "text/plain; charset=utf-8"
-        UserWarning,
-        match=(
-            "Overriding non-default ErrorHandler fallback value. "
-            "Changing from text to json."
-        ),
-    ):
-        app.config.FALLBACK_ERROR_FORMAT = "json"
-
-    assert app.error_handler.fallback == "json"
 
 
 def test_allow_fallback_error_format_in_config_injection():
@@ -327,7 +307,6 @@ def test_allow_fallback_error_format_in_config_injection():
         raise Exception("something went wrong")
 
     request, response = app.test_client.get("/error")
-    assert request.app.error_handler.fallback == "text"
     assert response.status == 500
     assert response.content_type == "text/plain; charset=utf-8"
 
@@ -339,6 +318,23 @@ def test_allow_fallback_error_format_in_config_replacement(app):
     app.config = MyConfig()
 
     request, response = app.test_client.get("/error")
-    assert request.app.error_handler.fallback == "text"
     assert response.status == 500
     assert response.content_type == "text/plain; charset=utf-8"
+
+
+def test_config_fallback_before_and_after_startup(app):
+    app.config.FALLBACK_ERROR_FORMAT = "json"
+
+    @app.main_process_start
+    async def start(app, _):
+        app.config.FALLBACK_ERROR_FORMAT = "text"
+
+    _, response = app.test_client.get("/error")
+    assert response.status == 500
+    assert response.content_type == "text/plain; charset=utf-8"
+
+
+def test_config_fallback_bad_value(app):
+    message = "Unknown format: fake"
+    with pytest.raises(SanicException, match=message):
+        app.config.FALLBACK_ERROR_FORMAT = "fake"