diff --git a/planning/changes/2026-08-10.04-double-bootstrap-guard.md b/planning/changes/2026-08-10.04-double-bootstrap-guard.md new file mode 100644 index 0000000..703eff0 --- /dev/null +++ b/planning/changes/2026-08-10.04-double-bootstrap-guard.md @@ -0,0 +1,132 @@ +--- +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. +--- + +# Design: Fail fast when a second bootstrapper targets the same application + +## Summary + +`_attach_teardown_once` guards teardown attachment, and only that. A second +bootstrapper constructed on the same application still applies every instrument +again when `bootstrap()` is called, because nothing stops it. Litestar dies with +a confusing error from deep inside the framework; FastAPI silently registers a +second copy of the health and metrics routes. Make `bootstrap()` raise +`ConfigurationError` on the bootstrapper whose attach was skipped. + +## Motivation + +The construction-time warning already tells the user what the supported model +is — "construct one `` per application" — but nothing enforces it, +and the two frameworks fail in opposite, equally unhelpful ways. Both +reproduced on the current tree: + +```python +first = LitestarBootstrapper(bootstrap_config=config_a) +second = LitestarBootstrapper(bootstrap_config=dataclasses.replace(config_a)) # warns +first.bootstrap() +second.bootstrap() +``` + +``` +ImproperlyConfiguredException: 500: Handler already registered for path '/health' and http method OPTIONS +``` + +Nothing in that message points at the real mistake. The FastAPI equivalent is +worse, because it does not fail at all: the second `bootstrap()` returns the +same app object with its route count grown from 6 to 8 — a shadowed duplicate +of the health-check and metrics routes, a second `PrometheusInstrument` +registering against the same collector registry, and a second set of instrument +state whose teardown is not wired to anything. + +The warning is emitted at construction, but the damage happens at `bootstrap()`, +which is exactly where nothing checks. + +## Design + +`_attach_teardown_once` already detects the case; it just does not record it. +Have it remember, and have `bootstrap()` refuse: + +```python +# BaseBootstrapper +_attach_skipped: bool = False +``` + +```python +# BaseBootstrapper._attach_teardown_once +def _attach_teardown_once(self, target: object, attach: typing.Callable[[], object]) -> None: + if getattr(target, self._TEARDOWN_MARKER, False): + warnings.warn(...) # unchanged + self._attach_skipped = True + return + ... +``` + +```python +# BaseBootstrapper.bootstrap +def bootstrap(self) -> ApplicationT: + if self._attach_skipped: + msg = ( + f"{type(self).__name__} shares its application with another lite-bootstrap " + f"bootstrapper, which has already applied its instruments. Construct one " + f"{type(self).__name__} per application." + ) + raise ConfigurationError(msg) + ... +``` + +`ConfigurationError` already exists in `lite_bootstrap/exceptions.py` and is what +`FastAPIConfig.__post_init__` raises for a comparable misuse. + +The construction-time warning stays, so the documented warn-and-skip invariant +for the teardown seam is unchanged — this adds a second, louder gate at the +point where instruments would actually be applied. `FreeBootstrapper` never +calls `_attach_teardown_once`, so it is unaffected. + +The warning's wording needs a small update: it currently ends by saying this +bootstrapper's teardown "will not run on shutdown", which understates the new +behavior — the bootstrapper cannot be used at all. + +## Non-goals + +- Making instrument application idempotent so a second bootstrapper becomes + harmless. That means per-instrument state on user-supplied objects and a much + wider change; the supported model is one bootstrapper per application. +- Letting the second `bootstrap()` warn and return the app unchanged. It would + hand back an app whose instruments were never applied and whose teardown is + not wired — a quieter trap than the one being fixed. +- Detecting two bootstrappers built on two *different* `AppConfig` objects that + happen to share route paths. That is ordinary route-collision territory and + belongs to the framework. + +## Testing + +`just test -k "second_bootstrapper"`, extending the existing +`test_second__bootstrapper_on_same_*_warns_not_stacks` tests, which +today stop at construction and never call `bootstrap()`: + +- Litestar: the second `bootstrap()` raises `ConfigurationError` naming the + bootstrapper class, and the first application still works (its health route + responds). +- FastAPI: same, and the app's route count is unchanged by the failed second + bootstrap — the assertion that pins the silent-duplication half of the defect. +- FastStream and FastMCP: same raise, since they share the seam. +- `FreeBootstrapper`: two bootstrappers still work, since it has no attach + target. This pins that the guard did not overreach. + +Then `just lint-ci` and the full `just test`. + +## Risk + +**A caller today relies on double-bootstrapping.** Unlikely and already broken: +on Litestar it raises, on FastAPI it produces shadowed duplicate routes. The +release note should still call it out, since the FastAPI case currently +"works". + +**The flag survives on a bootstrapper that could otherwise be reused.** Once +`_attach_skipped` is set, that bootstrapper is permanently unusable. That is +intended — the application it was given is owned by someone else, and nothing +about that changes later. + +**Promotion:** `architecture/bootstrappers.md` records that the attach marker +now gates instrument application as well as teardown attachment, and what the +second bootstrapper gets.