From 04a5a0414079b358d17397ae086f85a4ed3c7241 Mon Sep 17 00:00:00 2001 From: Ayush Patil <94798136+ayushpatil0810@users.noreply.github.com> Date: Thu, 18 Dec 2025 17:47:38 +0530 Subject: [PATCH] Deprecate should_ignore_error method The `should_ignore_error()` method was added in f191809 to support context preservation for debugging, but it no longer serves its original purpose and adds unnecessary overhead. Issues with the current implementation: - Always returns False by default - Called on every single request with no benefit - The original intention for error ignoring during debugging is not how context preservation works anymore - No documentation beyond API reference - No tests for the functionality - No evidence of real-world usage Changes: - Add deprecation warning to App.should_ignore_error() that will be removed in Flask 4.0 - Optimize call site to only invoke the method if it's been overridden by a subclass, eliminating the function call overhead for 99.9% of requests - Add comprehensive tests for the deprecation behavior - Update CHANGES.rst with deprecation notice Teardown handlers should manage their own error handling instead of relying on this method. Fixes #5816 --- CHANGES.rst | 3 ++ src/flask/app.py | 9 ++++- src/flask/sansio/app.py | 13 +++++++ tests/test_deprecations.py | 80 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 tests/test_deprecations.py diff --git a/CHANGES.rst b/CHANGES.rst index efd076e9..fb590aa8 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,6 +16,9 @@ Unreleased deprecation period. :issue:`5815` - ``template_filter``, ``template_test``, and ``template_global`` decorators can be used without parentheses. :issue:`5729` +- Deprecated ``App.should_ignore_error()``. This method always returned ``False`` + and added an unnecessary function call on every request. Teardown handlers should + manage their own error handling instead. :issue:`5816` Version 3.1.2 diff --git a/src/flask/app.py b/src/flask/app.py index e0c193dc..5fe128da 100644 --- a/src/flask/app.py +++ b/src/flask/app.py @@ -1576,8 +1576,13 @@ class Flask(App): if "werkzeug.debug.preserve_context" in environ: environ["werkzeug.debug.preserve_context"](ctx) - if error is not None and self.should_ignore_error(error): - error = None + # Check if should_ignore_error is overridden (deprecated) + if ( + error is not None + and type(self).should_ignore_error is not App.should_ignore_error + ): + if self.should_ignore_error(error): + error = None ctx.pop(error) diff --git a/src/flask/sansio/app.py b/src/flask/sansio/app.py index 43d20529..c3743a1a 100644 --- a/src/flask/sansio/app.py +++ b/src/flask/sansio/app.py @@ -928,8 +928,21 @@ class App(Scaffold): function returns ``True`` then the teardown handlers will not be passed the error. + .. deprecated:: 3.1 + This method is deprecated and will be removed in Flask 4.0. + Teardown functions should manage their own error handling. + .. versionadded:: 0.10 """ + import warnings + + warnings.warn( + "'should_ignore_error' is deprecated and will be removed in" + " Flask 4.0. Teardown functions should manage error handling" + " directly.", + DeprecationWarning, + stacklevel=2, + ) return False def redirect(self, location: str, code: int = 302) -> BaseResponse: diff --git a/tests/test_deprecations.py b/tests/test_deprecations.py new file mode 100644 index 00000000..febe6424 --- /dev/null +++ b/tests/test_deprecations.py @@ -0,0 +1,80 @@ +"""Tests for deprecated features.""" + +import warnings + +import pytest + +import flask + + +def test_should_ignore_error_deprecated(): + """Test that should_ignore_error emits a deprecation warning.""" + app = flask.Flask(__name__) + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + result = app.should_ignore_error(ValueError("test")) + + assert len(w) == 1 + assert issubclass(w[0].category, DeprecationWarning) + assert "should_ignore_error" in str(w[0].message) + assert "Flask 4.0" in str(w[0].message) + assert result is False + + +def test_should_ignore_error_not_called_by_default(): + """Test that should_ignore_error is not called on normal requests.""" + app = flask.Flask(__name__) + + @app.route("/") + def index(): + return "hello" + + client = app.test_client() + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + response = client.get("/") + + # Should not have any deprecation warnings + dep_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)] + assert len(dep_warnings) == 0 + assert response.status_code == 200 + + +def test_should_ignore_error_override_is_called(): + """Test that overriding should_ignore_error still works for backward compatibility.""" + + called = [] + + class MyApp(flask.Flask): + def should_ignore_error(self, error): + # Track that this was called and call parent to get deprecation warning + called.append(error) + super().should_ignore_error(error) + return False + + app = MyApp(__name__) + + @app.route("/") + def index(): + raise ValueError("test error") + + client = app.test_client() + + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter("always") + # Request will return 500 error page + response = client.get("/") + + # Should have been called with the error + assert len(called) == 1 + assert isinstance(called[0], ValueError) + + # Should have deprecation warning + dep_warnings = [x for x in w if issubclass(x.category, DeprecationWarning)] + assert len(dep_warnings) > 0 + assert any("should_ignore_error" in str(x.message) for x in dep_warnings) + + # Should return internal server error + assert response.status_code == 500