Skip to content

Fix old behavior in register_error_handler()  #4559

@qingpeng9802

Description

@qingpeng9802

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions