polish: TeardownError aggregation, cascade tests, README lifecycle#111
Merged
Conversation
PR1's final reviewer flagged that only LoggingInstrument used TeardownError aggregation; other instruments raised on first failure and skipped subsequent cleanup. Generalize the pattern: OpenTelemetryInstrument.teardown aggregates instrumentor uninstrument errors and tracer provider shutdown errors; FastStreamLoggingInstrument wraps the broker write in try/finally so super().teardown() always runs; SentryInstrument wraps flush in try/finally so init() always runs to disable the SDK. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…configs PR2's final reviewer noted the __post_init__ super() cascade was tested only through FastAPIConfig. Add a parametrized test covering FreeConfig, LitestarConfig, FastStreamConfig, and FastAPIConfig (redundant safety net) so future MRO refactors can't silently break the cascade for any config. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four constraints earned by the 2026-06-05 audit cycle that aren't obvious from the API: one-bootstrapper-per-application-instance, one-OpenTelemetryInstrument-per-process, teardown idempotency, and TeardownError aggregation on partial failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
lesnik512
added a commit
that referenced
this pull request
Jun 5, 2026
Bug-audit-v2 cycle: 26 findings across 4 PRs (#108-#111). Lifecycle hardening, config validation, CI gate, generalized TeardownError aggregation. Two behavior changes called out: FastAPIConfig no longer stomps user app fields; CorsConfig wildcard+credentials now raises. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Closes the three non-blocking follow-ups flagged during PR1/PR2 final reviews. All small, all earned by the 2026-06-05 audit cycle, none blocking.
fix:GeneralizeTeardownErroraggregation across instruments. PR1's final reviewer noted that onlyLoggingInstrument.teardown()aggregated failures intoTeardownError; the other instruments raised on first failure and skipped later cleanup steps. Now uniform:OpenTelemetryInstrument.teardown()— eachuninstrument()call is wrapped in its own try/except; tracer providershutdown()is also protected; failures aggregate into a singleTeardownError(errors)raised at the end. Logger restoration always runs even if an instrumentor raised.FastStreamLoggingInstrument.teardown()— wraps the brokerparams_storagewrite in try/finally sosuper().teardown()always runs.SentryInstrument.teardown()— wrapssentry_sdk.flush(timeout=2)in try/finally sosentry_sdk.init()(the SDK reset) always runs.test:Parametrize the OTel-insecure-endpoint cascade test across all framework configs. PR2's final reviewer noted the__post_init__super() cascade was tested only throughFastAPIConfig. The new test coversFreeConfig,LitestarConfig,FastStreamConfig, andFastAPIConfig(redundant safety net) so future inheritance refactors can't silently drop the cascade for any config.docs:Add a "Lifecycle constraints" section to README.md. Four constraints earned by the audit cycle that aren't obvious from the API: one bootstrapper per application instance, oneOpenTelemetryInstrumentper process, idempotentteardown(), andTeardownErroraggregation on partial failures.Test plan
just test— 194 passed (187 baseline + 3 instrument-agg tests + 4 cascade-parametrize cases), 100% coverage.just lint-ci— clean (ruff, eof-fixer, ty).Audit cycle status
This wraps up the 2026-06-05 bug-audit-v2 work. The audit closed 26 findings across PR1 (#108), PR2 (#109), and PR3 (#110). The retrospective (
planning/specs/2026-06-05-bug-audit-v2-retro.md) flagged three deferred items as non-blocking follow-ups — all three land here.🤖 Generated with Claude Code