Repository navigation
fix: raise ConfigurationError from bootstrap() after teardown() - #347
Merged
Merged
Conversation
teardown() reset is_bootstrapped, so a later bootstrap() ran every instrument again against the application it had already changed. Litestar raised on the duplicate /health route; FastAPI, FastStream and FastMCP silently registered health and metrics routes and Prometheus middleware twice. A torn-down bootstrapper now refuses to bootstrap again. Fixes #326.
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 #326.
teardown()resetis_bootstrapped, so a laterbootstrap()ran every instrument'sbootstrap()again. Each one appended to the same application orAppConfigit had already changed, and nothing removed what it had added the first time:ImproperlyConfiguredExceptionon the duplicate/healthroute (or/metricswith health checks off).TracerProvider, because a provider can only be set once per process.The issue offered two options: make re-bootstrap work, or refuse it. This PR refuses it. Once
teardown()has run,bootstrap()raisesConfigurationErrorand tells the user to construct a new bootstrapper, with a new application if the bootstrapper takes one. This matches the one-bootstrapper-per-application rule that_attach_teardown_oncealready enforces. Supporting it would have meant making every instrument idempotent across five frameworks, and OpenTelemetry would still be stuck with its first provider.The flag is set only when
teardown()actually tears something down. Ateardown()before anybootstrap()still returns early and doesn't block a laterbootstrap(). Repeatedbootstrap()calls before teardown still return the cached application (#325).Tests, written first, all failing on main:
test_<framework>_bootstrap_after_teardown_raisesfor Free, FastAPI, FastStream, FastMCP and Litestar. Each one bootstraps with the default config, tears down, and expectsConfigurationErrorfrom the secondbootstrap(). Four failed with "DID NOT RAISE"; Litestar failed with its duplicate-route error.test_litestar_bootstrap_after_teardown_builds_a_new_applicationfrom fix: return the same application from a repeated bootstrap() #325 asserted the opposite. It ran with health checks and Prometheus off to avoid this bug. It is replaced by the Litestar test above.CONTEXT.md: the Bootstrapper entry now says a torn-down bootstrapper can't bootstrap again. The Free entry said Free has "no double-bootstrap guard", which now reads wrong, so it says "no guard against a second bootstrapper" instead.Checked locally: lint and
tyare clean, all 397 tests pass, and coverage is 100%. I didn't run the floors steps locally; the change touches no dependency APIs.Behaviour change: code that calls
bootstrap()again on the same bootstrapper afterteardown()now getsConfigurationError. On FastAPI, FastStream, FastMCP and Free it used to work, but on the frameworks it duplicated routes and metrics. Construct a new bootstrapper instead. No config changes.