Skip to content

fix: support nested routers in FastAPI 0.140 - #286

Open
LilScottyPippen wants to merge 1 commit into
laurentS:masterfrom
LilScottyPippen:fix-fastapi-new-router-support
Open

fix: support nested routers in FastAPI 0.140#286
LilScottyPippen wants to merge 1 commit into
laurentS:masterfrom
LilScottyPippen:fix-fastapi-new-router-support

Conversation

@LilScottyPippen

Copy link
Copy Markdown

Fix default_limits not applied with FastAPI >= 0.140

FastAPI changed route internals and introduced _IncludedRouter.
SlowAPI middleware couldn't resolve endpoint handlers anymore,
causing default_limits and application_limits to silently skip.

This patch adds recursive route resolution for new router structures.

@Mukller Mukller left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Triage note for the cluster of PRs fixing included-router rate limiting (#285, #286, #282): I verified all three functionally on FastAPI 0.141.1 with a uniform probe (default_limits only, SlowAPIMiddleware, decorator-free endpoint — the path where only _find_route_handler can enforce):

app.include_router(sub)            -> second request status
app.include_router(sub, prefix="/sub")

master : 200,200 BYPASSED   | 200,200 BYPASSED     (bug confirmed)
#285   : 200,429 ENFORCED   | 200,429 ENFORCED
#286   : 200,429 ENFORCED   | 200,429 ENFORCED   <- this PR
#282   : 200,429 ENFORCED   | 200,200 BYPASSED    <- prefixed case still leaks

Findings specific to this PR (as posted on #285):

  • Your _effective_candidates + .routes recursion works against real 0.141 internals (_IncludedRouter exposes both).
  • One semantic difference vs the other two candidates worth knowing about: #286 returns on the first FULL match, while #285 keeps the original last-match-wins scan (handler = ...; continue) — see note on #285; on apps where several routes could match the same scope this may differ from pre-0.137 slowapi behavior. On apps where several routes could match the same scope, behavior may differ from pre-0.137 slowapi — might be worth an explicit decision/test.
  • Your suite passes locally for me (61 passed, incl. the new default-limits integration test).

Version coverage question for maintainers: #285 keys off effective_route_contexts (≥0.137 lazy inclusion), #286 keys off _effective_candidates (the _IncludedRouter attr present in ≥0.140 builds I inspected), and #282 tries .routes/original_router.routes. All three attributes coexist on 0.141.1 — so the practical choice is about which older FastAPI lines you want to keep supporting, since the attribute landscape changed between 0.137–0.140.

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.

2 participants