refactor error checks in register_error_handler

Co-authored-by: David Lord <davidism@gmail.com>

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
 -------------

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
-
         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):
+            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(
+        if isinstance(exc_class, Exception):
-            exc_class, Exception
+            raise TypeError(
-        ), "Custom exceptions must be subclasses of Exception."
+                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

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
 

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(
+    with pytest.raises(TypeError) as exc_info:
-        AssertionError, match="Custom exceptions must be subclasses of Exception."
+        app.register_error_handler(CustomException(), None)
-    ):
+
-        app.register_error_handler(UnacceptableCustomException, 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:
+        if e.original_exception is not None:
-            return f"wrapped {type(original).__name__}"
+            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()