fix: don't freeze Problem so it survives yield-dependency unwinding (#9)#12
Merged
Merged
Conversation
…ing (#9) A `Problem` is also an `Exception`. FastAPI runs `yield` dependencies inside an `AsyncExitStack`; as an exception unwinds back out, `contextlib` reassigns `exc.__traceback__` at the Python level. While `Problem` was a frozen Pydantic dataclass that assignment raised `FrozenInstanceError`, masking the raised problem as a generic 500. Routes without `yield` dependencies never hit the Python-level reassignment (CPython sets `__traceback__` at the C level on the initial raise), which is why it went unnoticed. Make the dataclass non-frozen (`frozen=False, eq=False`) and drop the `frozen_default=True` static hint, rather than special-casing `__setattr__`: an exception must be writable at the Python level by design. `eq=False` keeps an exception's normal identity-based equality and hashing. The handlers already treat a raised problem as read-only, so reusing a shared module-level constant stays safe. Tests: raise through single and nested `yield` dependencies (no field/payload lost), re-raise as a different problem type through a guarded route, and a unit test that the BaseException dunders are reassignable with no information lost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Removed unnecessary comments about the dataclass behavior.
Updated docstring to clarify instance resolution behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9.
Problem
Raising a
Problemthrough (or behind) a FastAPIyielddependency returned a generic 500 instead of the intended status:Root cause
A
Problemis also anException. FastAPI runsyielddependencies inside anAsyncExitStack; as the exception unwinds back out,contextlibreassignsexc.__traceback__at the Python level:Problemwas afrozen=TruePydantic dataclass, so that assignment raisedFrozenInstanceError, which masked the real problem and fell through to the catch-all 500 handler. Routes withoutyielddependencies never hit the Python-level reassignment (CPython sets__traceback__at the C level on the initial raise), which is why it went unnoticed.Fix
An exception must be writable at the Python level by design, so rather than special-casing
__setattr__, theProblemdataclass is simply no longer frozen:frozen=False, eq=Falsein_ProblemMeta—eq=Falsekeeps an exception's normal identity-based equality and hashing (a non-frozen dataclass would otherwise synthesize value-based, unhashable instances).frozen_default=True@dataclass_transformhint so the static contract matches runtime (no "frozen" claim the runtime doesn't enforce).The handlers already treat a raised problem as read-only (
build_wirenever mutates it), so reusing a shared module-level constant across requests stays safe — now verified bytest_state_safety_shared_problem_across_pathsrather than relying on immutability.Tests (TDD — written first, watched fail with
FrozenInstanceError/500, then fixed)test_yield_dependency.pyyielddependency → real status, not 500yielddeps → detail,instance, and typed extension fields all survive (no information lost)test_problem.pytest_fields_are_mutable_problem_is_not_frozen(replaces the old frozen-mutation test)test_exception_dunders_reassignable_with_no_info_lost—__traceback__/__cause__/__context__/__suppress_context__/add_noteall reassignable, values intactFull suite: 92 passed.
ruff check,ruff format, andpyrightall pass.🤖 Generated with Claude Code