Skip to content

fix: install the DI middleware on every broker, on startup - #52

Merged
lesnik512 merged 1 commit into
mainfrom
fix/di-middleware-on-every-broker
Sep 15, 2026
Merged

lesnik512 merged 1 commit into
mainfrom
fix/di-middleware-on-every-broker

Conversation

@lesnik512

Copy link
Copy Markdown
Member

Closes #42

What

setup_di installed the DI middleware on app.broker only. FastStream 0.7 apps hold a list of brokers (FastStream(*brokers), app.add_broker), and app.broker is just brokers[0], so every other broker ran without DI. The failure was silent at setup: the first message to a FromDI subscriber on another broker died with AttributeError: 'NoneType' object has no attribute 'resolve_dependency', because ContextRepo.get returns None for the never-set request container key.

How

setup_di no longer calls add_middleware itself. It registers an on_startup hook that walks app.brokers and adds the middleware factory to each broker that does not already carry it.

  • Startup is the one moment when the broker list is complete and nothing has been consumed, so a broker added with app.add_broker after setup_di is covered without documenting an ordering rule (the issue's options 1 and 2 both miss that case; option 3 refuses a shape FastStream supports).
  • It is safe because FastStream builds a subscriber's middleware stack per message from config.broker_middlewares, so a middleware added in a startup hook applies to subscribers registered earlier.
  • The membership check reads broker.config.broker_middlewares, the same sequence FastStream builds the stack from, so a stop/start cycle does not add a second copy.
  • The if not app.broker guard is unchanged. Relaxing it for brokers created inside a user's own on_startup hook is possible but depends on hook ordering, so it is left for its own issue.

Recorded in docs/adr/0002-install-middleware-on-startup.md, including the revisit trigger.

Tests

  • test_di_resolves_on_every_broker, parametrized over both brokers at construction and the second broker added after setup_di. Both cases failed on main.
  • test_middleware_is_installed_once_per_broker_across_restarts guards the idempotency check across two TestApp cycles.
  • just lint-ci clean, just test-ci at 100% coverage.

Follow-ups not in this PR

  • Raise a RuntimeError naming setup_di from Dependency.__call__ when the request container is missing, instead of the current AttributeError on None.
  • Relax the no-broker guard, see above.

FastStream 0.7 apps hold a list of brokers and app.broker is only the
first, so setup_di left brokers 2..N without DI. The first message to a
FromDI subscriber on one of them failed with a None request container and
nothing at setup time pointing at the cause.

setup_di now registers an on_startup hook that adds the middleware to each
broker in app.brokers not yet carrying it. Startup is when the broker list
is complete, so a broker added via app.add_broker after setup_di is covered
too, and the membership check keeps a restarted app from stacking a second
copy. Rationale in docs/adr/0002-install-middleware-on-startup.md.

Closes #42
@lesnik512
lesnik512 merged commit c146c95 into main Sep 15, 2026
8 checks passed
@lesnik512
lesnik512 deleted the fix/di-middleware-on-every-broker branch September 15, 2026 17:00
lesnik512 added a commit that referenced this pull request Sep 15, 2026
setup_di refused an app with no broker at call time, which blocked the
FastStream shape where the broker is built inside an on_startup hook and
attached with app.add_broker. Since #52 the broker list is read at
startup, so the guard was no longer needed for correctness.

The guard moves into the startup install hook: it raises when the broker
list is still empty when it runs, naming both remedies (pass a broker to
FastStream or app.add_broker; register a broker-adding hook before
setup_di, since hooks run in registration order). The FromDI message-time
error names the other order. ADR-0002 records the change.

Closes #56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

setup_di wires DI into app.broker only, so a multi-broker app silently loses DI on brokers 2..N

1 participant