diff --git a/CLAUDE.md b/CLAUDE.md index c8ab359..d7ef09d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,9 +54,10 @@ Invariants (what must not break) — see the capability page for the full accoun instance is ignored. Construct exactly one `OpenTelemetryInstrument` per process. → `architecture/instruments.md` - **Teardown attaches once.** `_attach_teardown_once` guards against double-attach via - the `_lite_bootstrap_teardown_attached` marker; `_lite_bootstrap_*`-prefixed - attributes are the sanctioned way to tag user-supplied apps. → - `architecture/bootstrappers.md` + the `_lite_bootstrap_teardown_attached` marker; a second bootstrapper on an already-marked + target warns at construction and its `bootstrap()` raises `ConfigurationError`. + `_lite_bootstrap_*`-prefixed attributes are the sanctioned way to tag user-supplied + apps. → `architecture/bootstrappers.md` Capability index (all of `architecture/`): diff --git a/architecture/bootstrappers.md b/architecture/bootstrappers.md index 1adbf21..e3c9ca5 100644 --- a/architecture/bootstrappers.md +++ b/architecture/bootstrappers.md @@ -87,6 +87,25 @@ The guard is uniform: the same marker and warning apply to all four app-bearing frameworks. `attach` is typed `Callable[[], object]` because some hooks (FastStream's `on_shutdown`) return the callback. +The marker gates more than the teardown hook: it also gates instrument +application. `_attach_teardown_once` records the skip on `self._attach_skipped` +before returning, and `bootstrap()` checks that flag first — the losing +bootstrapper raises `ConfigurationError` naming itself rather than re-applying +every instrument against an application another bootstrapper already owns. +Construction only warns; the losing bootstrapper is unusable from that point — +only the failure itself is deferred to `bootstrap()`. `FreeBootstrapper` never +calls `_attach_teardown_once` — it has no attach target — so it is unaffected; +two `FreeBootstrapper`s bootstrap independently. + +Nothing clears `_TEARDOWN_MARKER`, including `teardown()`. So once an +application has been bootstrapped, it stays owned for the life of the process — +a fresh bootstrapper constructed on it later still warns at construction and +raises at `bootstrap()`. Clearing the marker on teardown is not the fix: for +FastAPI, the lifespan wrapper the first bootstrapper installed via `_wrap_lifespan` +stays merged into the app regardless of the marker, so a second bootstrapper would +still be stacking its teardown behind one that's already there. The remedy is to +construct a fresh application. + Litestar's `attach` thunk wraps `_apply_config`, which also normalizes the `AppConfig` it is handed before `Litestar.from_config()` builds the app: it sets `debug` from `service_debug`, and fills `request_max_body_size` with Litestar's own 10 MB default diff --git a/lite_bootstrap/bootstrappers/base.py b/lite_bootstrap/bootstrappers/base.py index acfcbc7..27c702e 100644 --- a/lite_bootstrap/bootstrappers/base.py +++ b/lite_bootstrap/bootstrappers/base.py @@ -5,6 +5,7 @@ from lite_bootstrap.exceptions import ( BootstrapperNotReadyError, + ConfigurationError, InstrumentDependencyMissingWarning, TeardownError, ) @@ -39,10 +40,11 @@ def _attach_teardown_once(self, target: object, attach: typing.Callable[[], obje if getattr(target, self._TEARDOWN_MARKER, False): warnings.warn( f"{type(self).__name__} already has a lite-bootstrap teardown hook attached to this " - f"application or its configuration; skipping. This {type(self).__name__}'s teardown " - f"will not run on shutdown — construct one {type(self).__name__} per application.", + f"application or its configuration; skipping. This {type(self).__name__} cannot be used — " + f"its bootstrap() will raise — so construct one {type(self).__name__} per application.", stacklevel=3, ) + self._attach_skipped = True return # Mark only after a successful attach: if attach() raises, the target stays untagged # so a retry can re-attach rather than silently warning-and-skipping forever. @@ -69,6 +71,8 @@ def build_summary(self) -> str: def __init__(self, bootstrap_config: BaseConfig) -> None: self.is_bootstrapped = False + # Set when another bootstrapper already owns this application; bootstrap() then refuses. + self._attach_skipped = False if not self.is_ready(): msg = f"{type(self).__name__} is not ready: {self.not_ready_message}" raise BootstrapperNotReadyError(msg) @@ -112,6 +116,13 @@ def _prepare_application(self) -> ApplicationT: ... def is_ready(self) -> bool: ... def bootstrap(self) -> ApplicationT: + if self._attach_skipped: + msg = ( + f"{type(self).__name__} shares its application with another lite-bootstrap " + f"bootstrapper, which already owns it. Construct one " + f"{type(self).__name__} per application." + ) + raise ConfigurationError(msg) if self.is_bootstrapped: return self._prepare_application() self.is_bootstrapped = True diff --git a/lite_bootstrap/exceptions.py b/lite_bootstrap/exceptions.py index 6b8a0db..0e721ef 100644 --- a/lite_bootstrap/exceptions.py +++ b/lite_bootstrap/exceptions.py @@ -7,7 +7,11 @@ class BootstrapperNotReadyError(LiteBootstrapError): class ConfigurationError(LiteBootstrapError): - """Raised when a config is invalid or a required optional dependency is missing.""" + """Raised when a config is invalid or a required optional dependency is missing. + + Also raised when a bootstrapper is constructed on an application another bootstrapper + already owns. + """ class TeardownError(LiteBootstrapError): diff --git a/planning/changes/2026-08-10.04-double-bootstrap-guard.md b/planning/changes/2026-08-10.04-double-bootstrap-guard.md index 703eff0..978f8fe 100644 --- a/planning/changes/2026-08-10.04-double-bootstrap-guard.md +++ b/planning/changes/2026-08-10.04-double-bootstrap-guard.md @@ -1,5 +1,5 @@ --- -summary: Make a second bootstrapper on an already-bootstrapped application fail loudly — `bootstrap()` raises `ConfigurationError` instead of dying inside Litestar with a duplicate-route error or silently double-registering FastAPI's routes. +summary: A second bootstrapper on the same application now fails loudly — `bootstrap()` raises `ConfigurationError` on the bootstrapper whose teardown attach was skipped, instead of dying inside Litestar with a duplicate-route error or silently double-registering FastAPI's routes. The construction-time warning is unchanged in kind, reworded to say the second bootstrapper's `bootstrap()` will raise. `FreeBootstrapper` has no attach target and is unaffected. --- # Design: Fail fast when a second bootstrapper targets the same application diff --git a/planning/decisions/2026-06-24-teardown-marker-accepted-limits.md b/planning/decisions/2026-06-24-teardown-marker-accepted-limits.md index 723c2f8..3b89f29 100644 --- a/planning/decisions/2026-06-24-teardown-marker-accepted-limits.md +++ b/planning/decisions/2026-06-24-teardown-marker-accepted-limits.md @@ -65,3 +65,13 @@ accepted; it is out of scope for this decision. - **Litestar:** sharing one `AppConfig` across multiple apps becomes a supported, documented pattern, or the attach is restructured to run at `bootstrap()` time (when the app exists). Then tag the built `Litestar` app instead of the config. + +## Update (1.4.0) + +[double-bootstrap-guard](../changes/2026-08-10.04-double-bootstrap-guard.md) changed +the consequence both scenarios above describe. The FastMCP case is no longer "a +second bootstrapper warns-and-skips instead of re-attaching" — its `bootstrap()` +now raises `ConfigurationError`. The Litestar case is no longer "the second warns +and skips, and its teardown never runs" — its `bootstrap()` raises before any +instrument is applied, so there is nothing left half-wired. The marker and its two +accepted limits are unchanged; only what happens once the marker is hit got louder. diff --git a/planning/releases/1.4.0.md b/planning/releases/1.4.0.md index f741d5a..05961ef 100644 --- a/planning/releases/1.4.0.md +++ b/planning/releases/1.4.0.md @@ -1,6 +1,8 @@ # lite-bootstrap 1.4.0 — Litestar access logging off by default -**1.4.0 is a minor release with a behavior change for Litestar services.** +**1.4.0 is a minor release with a behavior change for Litestar services.** A +second bootstrapper sharing an application also now fails loudly instead of +corrupting it — see [Bug fixes](#bug-fixes) below. ## Behavior change @@ -101,8 +103,22 @@ warning — set the flag to actually turn logging on. `litestar.middleware.ASGIMiddleware`, which the OpenTelemetry middleware already subclassed before this release, was only added in litestar 2.15. `>=2.9` was never actually supported for the OTel path — this just makes the declared floor honest. +- **A second bootstrapper sharing an application now fails loudly instead of corrupting it.** + Constructing two bootstrappers (FastAPI, Litestar, FastStream, or FastMCP) against the same + application already warned at construction time, but `bootstrap()` on the second one applied + every instrument again anyway. Litestar died with an unrelated-looking + `ImproperlyConfiguredException: Handler already registered for path '/health' and http method + OPTIONS`; FastAPI did not fail at all — the app's route count silently grew (e.g. from 6 to 8 + for a default config), a shadowed duplicate of the health-check and metrics routes. + `bootstrap()` on the losing bootstrapper now raises `ConfigurationError` naming itself. If your + code relied on the FastAPI case appearing to "work", it will now raise. The ownership marker + behind this is never cleared, including by `teardown()` — once an application has been + bootstrapped, it stays owned for the life of the process; construct a fresh application rather + than reusing one that was already bootstrapped. ## References - `planning/changes/2026-08-10.01-litestar-middleware-logging.md` +- `planning/changes/2026-08-10.02-log-stream-bind-at-bootstrap.md` - `planning/changes/2026-08-10.03-litestar-request-max-body-size.md` +- `planning/changes/2026-08-10.04-double-bootstrap-guard.md` diff --git a/tests/test_fastapi_bootstrap.py b/tests/test_fastapi_bootstrap.py index 3b24e05..b41b2c0 100644 --- a/tests/test_fastapi_bootstrap.py +++ b/tests/test_fastapi_bootstrap.py @@ -10,6 +10,7 @@ from lite_bootstrap import FastAPIBootstrapper, FastAPIConfig from lite_bootstrap.bootstrappers.fastapi_bootstrapper import _narrow_app +from lite_bootstrap.exceptions import ConfigurationError from lite_bootstrap.types import UNSET from tests.conftest import CustomInstrumentor, SentryTestTransport, emulate_package_missing @@ -166,3 +167,24 @@ def test_fastapi_config_inherits_otel_insecure_warning() -> None: ) matching = [w for w in caught if "unencrypted" in str(w.message)] assert matching, [str(w.message) for w in caught] + + +def test_second_fastapi_bootstrapper_bootstrap_raises(fastapi_config: FastAPIConfig) -> None: + application = fastapi.FastAPI() + first = FastAPIBootstrapper(bootstrap_config=dataclasses.replace(fastapi_config, application=application)) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + second = FastAPIBootstrapper(bootstrap_config=dataclasses.replace(fastapi_config, application=application)) + + try: + first.bootstrap() + routes_after_first = len(application.routes) + + with pytest.raises(ConfigurationError, match="FastAPIBootstrapper"): + second.bootstrap() + + assert len(application.routes) == routes_after_first, ( + "a refused second bootstrap must not register duplicate routes" + ) + finally: + first.teardown() diff --git a/tests/test_fastmcp_bootstrap.py b/tests/test_fastmcp_bootstrap.py index c775e25..328b8d0 100644 --- a/tests/test_fastmcp_bootstrap.py +++ b/tests/test_fastmcp_bootstrap.py @@ -12,6 +12,7 @@ from lite_bootstrap import BootstrapperNotReadyError, FastMcpBootstrapper, FastMcpConfig from lite_bootstrap.bootstrappers.fastmcp_bootstrapper import FastMcpLoggingMiddleware +from lite_bootstrap.exceptions import ConfigurationError from tests.conftest import emulate_package_missing, emulate_package_missing_with_module_reload @@ -302,3 +303,18 @@ def test_fastmcp_bootstrap_without_structlog() -> None: bootstrapper = FastMcpBootstrapper(bootstrap_config=FastMcpConfig()) bootstrapper.bootstrap() bootstrapper.teardown() + + +def test_second_fastmcp_bootstrapper_bootstrap_raises() -> None: + application = FastMCP() + first = FastMcpBootstrapper(bootstrap_config=FastMcpConfig(application=application, service_name="a")) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + second = FastMcpBootstrapper(bootstrap_config=FastMcpConfig(application=application, service_name="b")) + + try: + first.bootstrap() + with pytest.raises(ConfigurationError, match="FastMcpBootstrapper"): + second.bootstrap() + finally: + first.teardown() diff --git a/tests/test_faststream_bootstrap.py b/tests/test_faststream_bootstrap.py index 29cb29b..4099f2b 100644 --- a/tests/test_faststream_bootstrap.py +++ b/tests/test_faststream_bootstrap.py @@ -22,6 +22,7 @@ FastStreamLoggingInstrument, FastStreamOpenTelemetryInstrument, ) +from lite_bootstrap.exceptions import ConfigurationError from tests.conftest import ( CustomInstrumentor, SentryTestTransport, @@ -299,3 +300,18 @@ def params_storage(self, _value: object) -> None: instrument.teardown() # If super().teardown() ran, structlog defaults were reset — no exception below. structlog.get_logger("verify-reset") + + +def test_second_faststream_bootstrapper_bootstrap_raises(broker: RedisBroker) -> None: + config_a = build_faststream_config(broker=broker) + first = FastStreamBootstrapper(bootstrap_config=config_a) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + second = FastStreamBootstrapper(bootstrap_config=dataclasses.replace(config_a)) + + try: + first.bootstrap() + with pytest.raises(ConfigurationError, match="FastStreamBootstrapper"): + second.bootstrap() + finally: + first.teardown() diff --git a/tests/test_free_bootstrap.py b/tests/test_free_bootstrap.py index e0e3a50..46d331a 100644 --- a/tests/test_free_bootstrap.py +++ b/tests/test_free_bootstrap.py @@ -251,3 +251,19 @@ def test_build_summary_renders_none_for_empty_sections() -> None: bootstrapper.skipped_instruments = [] summary = bootstrapper.build_summary() assert summary == "FreeBootstrapper:\n configured:\n (none)\n skipped:\n (none)" + + +def test_two_free_bootstrappers_both_bootstrap(free_bootstrapper_config: FreeConfig) -> None: + """FreeBootstrapper has no application to own, so the double-bootstrap guard must not fire.""" + first = FreeBootstrapper(bootstrap_config=free_bootstrapper_config) + second = FreeBootstrapper(bootstrap_config=free_bootstrapper_config) + + try: + first.bootstrap() + second.bootstrap() + + assert first.is_bootstrapped + assert second.is_bootstrapped + finally: + second.teardown() + first.teardown() diff --git a/tests/test_litestar_bootstrap.py b/tests/test_litestar_bootstrap.py index 5c3850e..df7b7c0 100644 --- a/tests/test_litestar_bootstrap.py +++ b/tests/test_litestar_bootstrap.py @@ -31,6 +31,7 @@ build_litestar_route_details_from_scope, build_span_name, ) +from lite_bootstrap.exceptions import ConfigurationError from tests.conftest import ( CustomInstrumentor, SentryTestTransport, @@ -74,6 +75,8 @@ def test_second_litestar_bootstrapper_on_same_config_warns_not_stacks(litestar_c matching = [w for w in caught if "already has a lite-bootstrap teardown hook" in str(w.message)] assert matching, "expected warning about existing lite-bootstrap teardown hook" + assert "cannot be used" in str(matching[0].message) + assert "bootstrap() will raise" in str(matching[0].message) assert len(config_a.application_config.on_shutdown) == on_shutdown_after_first, ( "second bootstrapper must not stack another on_shutdown teardown" ) @@ -510,3 +513,21 @@ def test_litestar_default_request_max_body_size_matches_litestar() -> None: litestar_default = inspect.signature(litestar.Litestar.__init__).parameters["request_max_body_size"].default assert litestar_default == _LITESTAR_DEFAULT_REQUEST_MAX_BODY_SIZE + + +def test_second_litestar_bootstrapper_bootstrap_raises(litestar_config: LitestarConfig) -> None: + first = LitestarBootstrapper(bootstrap_config=litestar_config) + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + second = LitestarBootstrapper(bootstrap_config=dataclasses.replace(litestar_config)) + + try: + application = first.bootstrap() + + with pytest.raises(ConfigurationError, match="LitestarBootstrapper"): + second.bootstrap() + + with TestClient(app=application) as client: + assert client.get(litestar_config.health_checks_path).status_code == status_codes.HTTP_200_OK + finally: + first.teardown()