diff --git a/CHANGES.rst b/CHANGES.rst index aa2bddc1..285d6326 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,6 +5,9 @@ Version 2.2.0 Unreleased +- Refactor ``register_error_handler`` to consolidate error checking. + Rewrite some error messages to be more consistent. :issue:`4559` + Version 2.1.2 ------------- diff --git a/src/flask/scaffold.py b/src/flask/scaffold.py index acf8708b..f8819ccf 100644 --- a/src/flask/scaffold.py +++ b/src/flask/scaffold.py @@ -697,22 +697,7 @@ class Scaffold: .. versionadded:: 0.7 """ - if isinstance(code_or_exception, HTTPException): # old broken behavior - raise ValueError( - "Tried to register a handler for an exception instance" - f" {code_or_exception!r}. Handlers can only be" - " registered for exception classes or HTTP error codes." - ) - - try: - exc_class, code = self._get_exc_class_and_code(code_or_exception) - except KeyError: - raise KeyError( - f"'{code_or_exception}' is not a recognized HTTP error" - " code. Use a subclass of HTTPException with that code" - " instead." - ) from None - + exc_class, code = self._get_exc_class_and_code(code_or_exception) self.error_handler_spec[None][code][exc_class] = f @staticmethod @@ -727,14 +712,32 @@ class Scaffold: code as an integer. """ exc_class: t.Type[Exception] + if isinstance(exc_class_or_code, int): - exc_class = default_exceptions[exc_class_or_code] + try: + exc_class = default_exceptions[exc_class_or_code] + except KeyError: + raise ValueError( + f"'{exc_class_or_code}' is not a recognized HTTP" + " error code. Use a subclass of HTTPException with" + " that code instead." + ) from None else: exc_class = exc_class_or_code - assert issubclass( - exc_class, Exception - ), "Custom exceptions must be subclasses of Exception." + if isinstance(exc_class, Exception): + raise TypeError( + f"{exc_class!r} is an instance, not a class. Handlers" + " can only be registered for Exception classes or HTTP" + " error codes." + ) + + if not issubclass(exc_class, Exception): + raise ValueError( + f"'{exc_class.__name__}' is not a subclass of Exception." + " Handlers can only be registered for Exception classes" + " or HTTP error codes." + ) if issubclass(exc_class, HTTPException): return exc_class, exc_class.code diff --git a/tests/test_basic.py b/tests/test_basic.py index 3dc3a0e9..c4be6621 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -899,13 +899,6 @@ def test_error_handling(app, client): assert b"forbidden" == rv.data -def test_error_handler_unknown_code(app): - with pytest.raises(KeyError) as exc_info: - app.register_error_handler(999, lambda e: ("999", 999)) - - assert "Use a subclass" in exc_info.value.args[0] - - def test_error_handling_processing(app, client): app.testing = False diff --git a/tests/test_user_error_handler.py b/tests/test_user_error_handler.py index d9f94a3f..79c5a73c 100644 --- a/tests/test_user_error_handler.py +++ b/tests/test_user_error_handler.py @@ -11,29 +11,35 @@ def test_error_handler_no_match(app, client): class CustomException(Exception): pass - class UnacceptableCustomException(BaseException): - pass - @app.errorhandler(CustomException) def custom_exception_handler(e): assert isinstance(e, CustomException) return "custom" - with pytest.raises( - AssertionError, match="Custom exceptions must be subclasses of Exception." - ): - app.register_error_handler(UnacceptableCustomException, None) + with pytest.raises(TypeError) as exc_info: + app.register_error_handler(CustomException(), None) + + assert "CustomException() is an instance, not a class." in str(exc_info.value) + + with pytest.raises(ValueError) as exc_info: + app.register_error_handler(list, None) + + assert "'list' is not a subclass of Exception." in str(exc_info.value) @app.errorhandler(500) def handle_500(e): assert isinstance(e, InternalServerError) - original = getattr(e, "original_exception", None) - if original is not None: - return f"wrapped {type(original).__name__}" + if e.original_exception is not None: + return f"wrapped {type(e.original_exception).__name__}" return "direct" + with pytest.raises(ValueError) as exc_info: + app.register_error_handler(999, None) + + assert "Use a subclass of HTTPException" in str(exc_info.value) + @app.route("/custom") def custom_test(): raise CustomException()