qingpeng9802 · GitHub

Background

Line 700 src/scaffold.py has a comment # old broken behavior.

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."
    )

The purpose of this code is check if the handler is an exception instance.
If so, raise an error to warn the user: the handler should be HTTPException classes or HTTP error codes.

Trigger test case:

In test/test_user_error_handler.py, add:

with pytest.raises(ValueError):
    app.register_error_handler(HTTPException(), None)

Why is this old and broken?

From version 0.7, self.register_error_handler can accept all Exception not just HTTPException so it is not accurate now. Also, the main check part for handlers is in self._get_exc_class_and_code.

Purposed Solution

Remove this part of code.
Enhance the check for handlers in self.register_error_handler.

Detail Discussion

  • Since the main check is in self._get_exc_class_and_code, new check part is written in self._get_exc_class_and_code to keep consistency.
  • isinstance(exc_class, Exception) is kept for warning error-prone cases.
  • Idea from inspect.isclass to solve Line 735 scr/scaffold.py TypeError: issubclass() arg 1 must be a class when arg1 is a instance.
    This idea uses New-style Classes feature in Python3. Since Flask needs Python>=3.7, it should not cause any compatibility issues.
  • Move except KeyError after confirm int type to improve type safety.
  • Print exc_class when AssertionError to improve traceback.

A PR will be linked.

Read the original on github.com ↗