8.1.1 removed ability to introspect teardown exceptions in pytest_fixture_post_finalizer #12306

Activity

1. axelande commented on May 21, 2024

   

   axelande

   on May 21, 2024 · Hidden as off-topic

   Last edited by axelande

   show commentMore actions
2. The-Compiler commented on May 21, 2024

   

   The-Compiler

   on May 21, 2024

   Member

   More actions

   @axelande This is completely unrelated, and happens because PTVS is accessing private pytest API which was moved in 3ba4095.
3. Zac-HD

   added

   type: regressionindicates a problem that was introduced in a release which was working previouslyindicates a problem that was introduced in a release which was working previously

   on Jul 6, 2024
4. Zac-HD commented on Jul 6, 2024

   

   Zac-HD

   on Jul 6, 2024

   Member

   More actions

   I'd be happy to accept a PR based on your branch to restore this behavior 🙂
5. Zac-HD

   added

   topic: fixturesanything involving fixtures directly or indirectlyanything involving fixtures directly or indirectly

   on Jul 6, 2024
6. EternalRights commented on May 9, 2026

   

   EternalRights

   on May 9, 2026

   Contributor

   More actions

   Hi @Zac-HD @bluetech,

   I've been looking into issue #12306 about the regression in pytest_fixture_post_finalizer where sys.exc_info() no longer returns the teardown exception. After digging through the commit history and current code, I'd like to propose a fix and get your feedback before submitting a PR.

   Timeline & root cause

   1. Commit 434282e (pytest 7.3) introduced ExceptionGroup collection for teardown exceptions and moved the hook call from inside the try/finally block to before the exception re‑raise. This broke sys.exc_info() because the exception is already caught when the hook runs.

   2. Commit 719f1ec changed the architecture: the hook is no longer called directly in finish() but registered as a finalizer (self.addfinalizer(lambda: hook.pytest_fixture_post_finalizer(...))). This was done so that a thrown exception from the hook does not interrupt the cleanup logic.

   3. Commit 96728d5 added a guard if cached_result is None: return to prevent the hook from being called multiple times.

   Current behavior

   In fixtures.py, finish() pops finalizers from self._finalizers in a loop, catches any exception, and stores it in an exceptions list. The hook is the first finalizer registered, so it is executed last—after the teardown exception has already been caught. At that point sys.exc_info() is (None, None, None).

   Why the existing branch (mchase‑nasuni) can't be merged directly

   That branch is based on the pre‑719f1ec9 architecture where the hook was called directly in finish(). Since 719f1ec the hook lives in _finalizers. Applying the old diff would cause the hook to run twice (once as a finalizer, once in finally) and would reintroduce the problem that 719f1ec fixed (hook exceptions interfering with cleanup).

   Proposed solution

   The core requirement is that the hook must see an active exception (so sys.exc_info() works) while still isolating hook‑thrown exceptions from the main cleanup.

   I suggest:

   1. Remove the hook from _finalizers – In execute(), do not register pytest_fixture_post_finalizer as a finalizer.
   2. Call the hook inside the re‑raise block – In finish(), after the while loop that runs the other finalizers, wrap the re‑raise logic in a try/finally and invoke the hook in the finally clause.
   3. Catch hook exceptions – Inside that finally, wrap the hook call in a try/except and ignore any exception from the hook (so it doesn't disturb the re‑raise of the original teardown exception).

   Pseudocode:

   def finish(self, request):
       # ... run all finalizers except the hook, collect exceptions ...
       try:
           if len(exceptions) == 1:
               raise exceptions[0]
           elif len(exceptions) > 1:
               raise BaseExceptionGroup(...)
       finally:
           try:
               node.ihook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
           except BaseException:
               pass  # hook exception does not affect the main flow

   This preserves the isolation that 719f1ec intended (hook exceptions don't break cleanup) while making sys.exc_info() valid again.

   Benefits

   • sys.exc_info() works in the hook (exception is being re‑raised).
   • Hook exceptions are caught and ignored, not interfering with the original teardown exception.
   • The guard from 96728d5 is no longer needed because the hook is never registered as a finalizer.
   • No double‑invocation of the hook.

   Questions for maintainers

   1. Does this approach align with the intent behind 719f1ec? (I believe it does – we still isolate hook failures.)
   2. Are there any other reasons the hook was moved to a finalizer that I might have missed?
   3. Is there a preference for where the hook call should sit (e.g., inside the try block before the re‑raise, or in the finally as above)?

   If you agree on the direction, I'll prepare a PR with the changes and appropriate tests (verifying sys.exc_info() works, hook exceptions are swallowed, and the hook isn't called multiple times).

   Thanks for your time!