all teardown callbacks are called despite errors

CHANGES.rst

@@ -14,6 +14,7 @@ Unreleased
     If subclasses were overriding these methods, the old signature is detected,
     shows a deprecation warning, and will continue to work during the
     deprecation period. :issue:`5815`
+-   All teardown callbacks are called, even if any raise an error. :pr:`5928`
 -   The ``should_ignore_error`` is deprecated. Handle errors as needed in
     teardown handlers instead. :issue:`5816`
 -   ``template_filter``, ``template_test``, and ``template_global`` decorators

docs/appcontext.rst

@@ -163,7 +163,8 @@ The teardown callbacks are called by the context when it is popped. They are
 called even if there is an unhandled exception during dispatch. They may be
 called multiple times in some test scenarios. This means there is no guarantee
 that any other parts of the request dispatch have run. Be sure to write these
-functions in a way that does not depend on other callbacks and will not fail.
+functions in a way that does not depend on other callbacks. All callbacks are
+called even if any raise an error.
 
 
 How the Context Works

src/flask/app.py

@@ -36,6 +36,7 @@
 from .globals import g
 from .globals import request
 from .globals import session
+from .helpers import _CollectErrors
 from .helpers import get_debug_flag
 from .helpers import get_flashed_messages
 from .helpers import get_load_dotenv
@@ -1430,15 +1431,24 @@ def do_teardown_request(
         :param exc: An unhandled exception raised while dispatching the request.
             Passed to each teardown function.
 
+        .. versionchanged:: 3.2
+            All callbacks are called rather than stopping on the first error.
+
         .. versionchanged:: 0.9
             Added the ``exc`` argument.
         """
+        collect_errors = _CollectErrors()
+
         for name in chain(ctx.request.blueprints, (None,)):
             if name in self.teardown_request_funcs:
                 for func in reversed(self.teardown_request_funcs[name]):
-                    self.ensure_sync(func)(exc)
+                    with collect_errors:
+                        self.ensure_sync(func)(exc)
+
+        with collect_errors:
+            request_tearing_down.send(self, _async_wrapper=self.ensure_sync, exc=exc)
 
-        request_tearing_down.send(self, _async_wrapper=self.ensure_sync, exc=exc)
+        collect_errors.raise_any("Errors during request teardown")
 
     def do_teardown_appcontext(
         self, ctx: AppContext, exc: BaseException | None = None
@@ -1452,12 +1462,21 @@ def do_teardown_appcontext(
         :param exc: An unhandled exception raised while the context was active.
             Passed to each teardown function.
 
+        .. versionchanged:: 3.2
+            All callbacks are called rather than stopping on the first error.
+
         .. versionadded:: 0.9
         """
+        collect_errors = _CollectErrors()
+
         for func in reversed(self.teardown_appcontext_funcs):
-            self.ensure_sync(func)(exc)
+            with collect_errors:
+                self.ensure_sync(func)(exc)
+
+        with collect_errors:
+            appcontext_tearing_down.send(self, _async_wrapper=self.ensure_sync, exc=exc)
 
-        appcontext_tearing_down.send(self, _async_wrapper=self.ensure_sync, exc=exc)
+        collect_errors.raise_any("Errors during app teardown")
 
     def app_context(self) -> AppContext:
         """Create an :class:`.AppContext`. When the context is pushed,

src/flask/ctx.py

@@ -10,6 +10,7 @@
 
 from . import typing as ft
 from .globals import _cv_app
+from .helpers import _CollectErrors
 from .signals import appcontext_popped
 from .signals import appcontext_pushed
 
@@ -482,16 +483,26 @@ def pop(self, exc: BaseException | None = None) -> None:
         if self._push_count > 0:
             return
 
-        try:
-            if self._request is not None:
+        collect_errors = _CollectErrors()
+
+        if self._request is not None:
+            with collect_errors:
                 self.app.do_teardown_request(self, exc)
+
+            with collect_errors:
                 self._request.close()
-        finally:
+
+        with collect_errors:
             self.app.do_teardown_appcontext(self, exc)
-            _cv_app.reset(self._cv_token)
-            self._cv_token = None
+
+        _cv_app.reset(self._cv_token)
+        self._cv_token = None
+
+        with collect_errors:
             appcontext_popped.send(self.app, _async_wrapper=self.app.ensure_sync)
 
+        collect_errors.raise_any("Errors during context teardown")
+
     def __enter__(self) -> te.Self:
         self.push()
         return self

src/flask/helpers.py

@@ -7,6 +7,7 @@
 from datetime import datetime
 from functools import cache
 from functools import update_wrapper
+from types import TracebackType
 
 import werkzeug.utils
 from werkzeug.exceptions import abort as _wz_abort
@@ -636,3 +637,34 @@ def _split_blueprint_path(name: str) -> list[str]:
         out.extend(_split_blueprint_path(name.rpartition(".")[0]))
 
     return out
+
+
+class _CollectErrors:
+    """A context manager that records and silences an error raised within it.
+    Used to run all teardown functions, then raise any errors afterward.
+    """
+
+    def __init__(self) -> None:
+        self.errors: list[BaseException] = []
+
+    def __enter__(self) -> None:
+        pass
+
+    def __exit__(
+        self,
+        exc_type: type[BaseException] | None,
+        exc_val: BaseException | None,
+        exc_tb: TracebackType | None,
+    ) -> bool:
+        if exc_val is not None:
+            self.errors.append(exc_val)
+
+        return True
+
+    def raise_any(self, message: str) -> None:
+        """Raise if any errors were collected."""
+        if self.errors:
+            if sys.version_info >= (3, 11):
+                raise BaseExceptionGroup(message, self.errors)  # noqa: F821
+            else:
+                raise self.errors[0]

tests/test_appctx.py

@@ -1,7 +1,10 @@
+import sys
+
 import pytest
 
 import flask
 from flask.globals import app_ctx
+from flask.testing import FlaskClient
 
 
 def test_basic_url_generation(app):
@@ -208,3 +211,55 @@ def teardown_app(error=None):
 
     assert called == ["flask_test", "TEARDOWN"]
     assert not flask.current_app
+
+
+def test_robust_teardown(app: flask.Flask, client: FlaskClient) -> None:
+    count = 0
+
+    @app.teardown_request
+    def request_teardown(e: Exception | None) -> None:
+        nonlocal count
+        count += 1
+        raise ValueError("request_teardown")
+
+    @app.teardown_appcontext
+    def app_teardown(e: Exception | None) -> None:
+        nonlocal count
+        count += 1
+        raise ValueError("app_teardown")
+
+    @app.get("/")
+    def index() -> str:
+        return ""
+
+    def request_signal(sender: flask.Flask, exc: Exception | None) -> None:
+        nonlocal count
+        count += 1
+        raise ValueError("request_signal")
+
+    def app_signal(sender: flask.Flask, exc: Exception | None) -> None:
+        nonlocal count
+        count += 1
+        raise ValueError("app_signal")
+
+    with (
+        flask.request_tearing_down.connected_to(request_signal, app),
+        flask.appcontext_tearing_down.connected_to(app_signal, app),
+    ):
+        if sys.version_info >= (3, 11):
+            with pytest.raises(ExceptionGroup, match="context teardown") as exc_info:  # noqa: F821
+                client.get()
+
+            assert len(exc_info.value.exceptions) == 2
+            eg1, eg2 = exc_info.value.exceptions
+            assert isinstance(eg1, ExceptionGroup)  # noqa: F821
+            assert "request teardown" in eg1.message
+            assert len(eg1.exceptions) == 2
+            assert isinstance(eg2, ExceptionGroup)  # noqa: F821
+            assert "app teardown" in eg2.message
+            assert len(eg2.exceptions) == 2
+        else:
+            with pytest.raises(ValueError, match="request_teardown"):
+                client.get()
+
+    assert count == 4

tests/test_basic.py

@@ -1420,20 +1420,21 @@ def handler(error, endpoint, values):
 
 
 def test_static_files(app, client):
-    rv = client.get("/static/index.html")
-    assert rv.status_code == 200
-    assert rv.data.strip() == b"<h1>Hello World!</h1>"
-    with app.test_request_context():
-        assert flask.url_for("static", filename="index.html") == "/static/index.html"
-    rv.close()
+    with client.get("/static/index.html") as rv:
+        assert rv.status_code == 200
+        assert rv.data.strip() == b"<h1>Hello World!</h1>"
+        with app.test_request_context():
+            assert (
+                flask.url_for("static", filename="index.html") == "/static/index.html"
+            )
 
 
 def test_static_url_path():
     app = flask.Flask(__name__, static_url_path="/foo")
     app.testing = True
-    rv = app.test_client().get("/foo/index.html")
-    assert rv.status_code == 200
-    rv.close()
+
+    with app.test_client().get("/foo/index.html") as rv:
+        assert rv.status_code == 200
 
     with app.test_request_context():
         assert flask.url_for("static", filename="index.html") == "/foo/index.html"
@@ -1442,35 +1443,35 @@ def test_static_url_path():
 def test_static_url_path_with_ending_slash():
     app = flask.Flask(__name__, static_url_path="/foo/")
     app.testing = True
-    rv = app.test_client().get("/foo/index.html")
-    assert rv.status_code == 200
-    rv.close()
+
+    with app.test_client().get("/foo/index.html") as rv:
+        assert rv.status_code == 200
 
     with app.test_request_context():
         assert flask.url_for("static", filename="index.html") == "/foo/index.html"
 
 
 def test_static_url_empty_path(app):
     app = flask.Flask(__name__, static_folder="", static_url_path="")
-    rv = app.test_client().open("/static/index.html", method="GET")
-    assert rv.status_code == 200
-    rv.close()
+
+    with app.test_client().open("/static/index.html", method="GET") as rv:
+        assert rv.status_code == 200
 
 
 def test_static_url_empty_path_default(app):
     app = flask.Flask(__name__, static_folder="")
-    rv = app.test_client().open("/static/index.html", method="GET")
-    assert rv.status_code == 200
-    rv.close()
+
+    with app.test_client().open("/static/index.html", method="GET") as rv:
+        assert rv.status_code == 200
 
 
 def test_static_folder_with_pathlib_path(app):
     from pathlib import Path
 
     app = flask.Flask(__name__, static_folder=Path("static"))
-    rv = app.test_client().open("/static/index.html", method="GET")
-    assert rv.status_code == 200
-    rv.close()
+
+    with app.test_client().open("/static/index.html", method="GET") as rv:
+        assert rv.status_code == 200
 
 
 def test_static_folder_with_ending_slash():
@@ -1487,9 +1488,10 @@ def catch_all(path):
 def test_static_route_with_host_matching():
     app = flask.Flask(__name__, host_matching=True, static_host="example.com")
     c = app.test_client()
-    rv = c.get("http://example.com/static/index.html")
-    assert rv.status_code == 200
-    rv.close()
+
+    with c.get("http://example.com/static/index.html") as rv:
+        assert rv.status_code == 200
+
     with app.test_request_context():
         rv = flask.url_for("static", filename="index.html", _external=True)
         assert rv == "http://example.com/static/index.html"

tests/test_blueprints.py

@@ -184,12 +184,12 @@ def test_templates_and_static(test_apps):
     assert rv.data == b"Hello from the Admin"
     rv = client.get("/admin/index2")
     assert rv.data == b"Hello from the Admin"
-    rv = client.get("/admin/static/test.txt")
-    assert rv.data.strip() == b"Admin File"
-    rv.close()
-    rv = client.get("/admin/static/css/test.css")
-    assert rv.data.strip() == b"/* nested file */"
-    rv.close()
+
+    with client.get("/admin/static/test.txt") as rv:
+        assert rv.data.strip() == b"Admin File"
+
+    with client.get("/admin/static/css/test.css") as rv:
+        assert rv.data.strip() == b"/* nested file */"
 
     # try/finally, in case other tests use this app for Blueprint tests.
     max_age_default = app.config["SEND_FILE_MAX_AGE_DEFAULT"]
@@ -198,10 +198,10 @@ def test_templates_and_static(test_apps):
         if app.config["SEND_FILE_MAX_AGE_DEFAULT"] == expected_max_age:
             expected_max_age = 7200
         app.config["SEND_FILE_MAX_AGE_DEFAULT"] = expected_max_age
-        rv = client.get("/admin/static/css/test.css")
-        cc = parse_cache_control_header(rv.headers["Cache-Control"])
-        assert cc.max_age == expected_max_age
-        rv.close()
+
+        with client.get("/admin/static/css/test.css") as rv:
+            cc = parse_cache_control_header(rv.headers["Cache-Control"])
+            assert cc.max_age == expected_max_age
     finally:
         app.config["SEND_FILE_MAX_AGE_DEFAULT"] = max_age_default