Skip to content

fix: populate FastAPI dependency injection panel with real resolutions - #64

Merged
JacobCoffee merged 5 commits into
mainfrom
fix/fastapi-dependencies-panel
Oct 11, 2026
Merged

JacobCoffee merged 5 commits into
mainfrom
fix/fastapi-dependencies-panel

Conversation

@JacobCoffee

Copy link
Copy Markdown
Owner

What was wrong

The Dependencies panel on the FastAPI integration always reported zero resolved dependencies, even for routes using real Depends() callables. Nothing in the codebase ever called the tracking hooks that fed the panel, and the one place that tried to populate the data ran in a finally block after the request context had already been cleared (and after the toolbar had already stored its request snapshot), so the write was always a no-op.

What changed

  • src/debug_toolbar/fastapi/middleware.py: wraps FastAPI's internal solve_dependencies so every Depends() resolution (name, type, cache hit/miss, duration) is recorded as it happens, and writes the data directly into the live request context instead of relying on a late, already-too-late population step.
  • Removed the dead population path that never actually ran.
  • Added tests/integration/test_fastapi_dependencies_panel.py: a regression test asserting a route using Depends() produces non-empty resolved_dependencies in the panel's stored stats.
  • Updated tests/unit/fastapi/test_middleware.py to cover the new live-sync behavior instead of the removed dead code path.

How verified

  • uv sync --all-extras && make ci passes in the worktree.
  • New integration test fails before the fix (empty resolved_dependencies) and passes after.
  • Manually verified in a browser against examples/fastapi_basic (port 8981): GET /users/123 now shows get_db, get_token, and get_current_user in the Dependencies panel, including a cache hit on the second get_db resolution.

The Dependencies panel always showed zero resolved dependencies, even
on routes using Depends(). Nothing ever called the tracking hooks, and
the fallback population step ran after the request context had already
been cleared and the toolbar had stored its (empty) snapshot.

Wrap FastAPI's internal solve_dependencies to record each Depends()
resolution (name, type, cache hit, duration) as it happens, and write
it straight into the live request context instead of deferring to a
step that runs too late.

Verified with a new integration test and manually against
examples/fastapi_basic, where routes using get_db/get_token/
get_current_user now show up in the Dependencies panel.
Copilot AI balanced review requested due to automatic review settings October 11, 2026 00:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://JacobCoffee.github.io/debug-toolbar/pr-preview/pr-64/

Built to branch gh-pages at 2026-10-11 01:30 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

…n sub-deps

The previous duration_ms only timed solving each dependency's OWN
sub-dependencies (the recursive solve_dependencies call), not the
actual invocation of the dependency itself, which FastAPI runs
afterwards in the parent's loop.

Time the real call instead, per dependency kind:
- sync functions/classes: wrap run_in_threadpool, the only place
  FastAPI invokes them from within solve_dependencies.
- generator dependencies (e.g. yield-based DB sessions): wrap
  _solve_generator, which performs the actual context-manager enter.
- cached resolutions: report duration_ms=0.0 directly, since FastAPI
  never re-invokes them.
- coroutine-function dependencies: FastAPI awaits them inline with no
  interceptable call site, so duration_ms is reported as None rather
  than measuring the wrong thing.

These internals changed shape across FastAPI releases (cache_key moved
from a Dependant property to a module-level _get_cache_key() helper,
and the is_*_callable attributes were later dropped entirely), so
instrumentation now probes for the exact attributes it needs and
no-ops safely when they are absent, instead of crashing requests.

Verified:
- New tests assert sync/generator dependencies report real elapsed
  time (>= a 50ms sleep) and coroutine dependencies report None.
- Manually checked fastapi==0.100.0 (declared floor) and fastapi==0.143.0
  (latest release): both run requests successfully with the panel
  gracefully empty, since neither matches the instrumented shape.
  fastapi==0.123.0 (this repo's pinned version) gets full, accurate
  per-dependency timing.
- make ci passes; verified in browser against examples/fastapi_basic.
Copilot AI balanced review requested due to automatic review settings October 11, 2026 00:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ss FastAPI versions

Per review: per-kind duration timing (run_in_threadpool / _solve_generator
patches) was too fragile and version-coupled, and still ended up a no-op on
latest FastAPI. Replaced with a single, defensive patch of solve_dependencies
only (both the fastapi.dependencies.utils and fastapi.routing bindings):

- Records each resolved sub-dependency's call name and cached flag.
  duration_ms is always None now: FastAPI invokes dependencies from
  different internal call sites across versions, and resolving a
  sub-dependant's own params (all this hook can time) isn't the same
  as running the dependency itself.
- Cache-key lookup now tries dependant.cache_key first (present as an
  instance attribute even on FastAPI 0.100.0), falling back to the
  module-level _get_cache_key() helper FastAPI introduced later.
- The original solve_dependencies call always runs and its result or
  exception passes through untouched; all of our own bookkeeping is
  wrapped in try/except so it can never fail a real request.
- Dropped the version-gate/capability-probing that skipped instrumentation
  outright; the defensive try/except plus the dual cache-key lookup is
  enough on its own.
- toolbar.js renders null values as an em dash instead of the literal
  text "null".

Verified end-to-end (names present, cached=True on second use of a shared
dependency) against:
- fastapi==0.100.0 (declared floor)
- fastapi==0.123.0 (this repo's pinned version)
- fastapi==0.143.0 (latest release)
all via throwaway venvs with debug_toolbar's src on sys.path, uv.lock
untouched. Replaced the duration-based tests with a cached-detection test.
make ci passes; re-verified in browser against examples/fastapi_basic
on port 8981.
Copilot AI balanced review requested due to automatic review settings October 11, 2026 00:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@JacobCoffee
JacobCoffee enabled auto-merge (squash) October 11, 2026 01:12
Copilot AI balanced review requested due to automatic review settings October 11, 2026 01:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI balanced review requested due to automatic review settings October 11, 2026 01:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@JacobCoffee
JacobCoffee merged commit 22c491a into main Oct 11, 2026
24 checks passed
@JacobCoffee
JacobCoffee deleted the fix/fastapi-dependencies-panel branch October 11, 2026 01:37
JacobCoffee added a commit that referenced this pull request Oct 11, 2026
PR #64 made the FastAPI dependency injection panel actually populate
resolved_dependencies for nested Depends() trees; assert on the real
dependency names (get_db, get_current_user, get_token) and counts
instead of only checking the panel key exists.

PR #67 made the async profiler's taskfactory backend report the real
caller-supplied task name (previously always 'Task-N') and made
blocking call detection actually fire (previously silent, since
asyncio logs slow callbacks via the asyncio logger rather than
call_exception_handler). Assert on the real task names and a real
detected blocking call instead of working around both bugs.
JacobCoffee added a commit that referenced this pull request Oct 11, 2026
* fix: cascade-delete posts when deleting a user in advanced-alchemy example

DELETE /api/users/{id} raised a 500 IntegrityError for any user with
posts, since the default relationship behaviour nulls out the foreign
key instead of deleting dependents, and Post.author_id is non-nullable.
Found while writing tests for the example app.

* test: add full test coverage for example apps

Add tests/examples/ covering every app under examples/: asgi_basic,
litestar_basic, litestar_advanced_alchemy, starlette_basic,
fastapi_basic, websocket_panel_example, graphql_panel_example,
mcp_server_example, and the standalone panel demo scripts (cache,
templates, settings, profiling, memory, flamegraph, async_profiler,
alerts).

Each app is exercised through its own framework's real test client:
documented routes are checked for status/content-type, toolbar HTML
injection and static assets, and the per-request panel data the
example is built to demonstrate (SQL N+1 detection, GraphQL resolver
tracking, websocket connection stats, async profiler stats, and so
on).

A shared tests/examples/conftest.py fixture (import_example) imports
each example fresh per test and evicts it afterwards, since several
examples build module-level toolbar/app singletons at import time.

Add make test-examples to run just this suite.

* ci: add weekly workflow to test examples against latest dependencies

Runs tests/examples against uv sync --all-extras --upgrade (ignoring
the lockfile pins) on the oldest and newest supported Python versions,
on a weekly schedule, workflow_dispatch, and PRs touching examples/ or
tests/examples/. Prints resolved versions of the key frameworks for
debugging. Not part of the required CI Success aggregator.

Confirmed this actually catches regressions: with mcp upgraded past
2.0, debug_toolbar.mcp.server fails to import because mcp 2.x renamed
FastMCP to MCPServer, which this workflow surfaces as a test failure
in tests/examples/test_mcp_server_example.py.

* test: exercise the real example scripts instead of replicating panel logic

The standalone panel demo tests previously had a second test per
script that reconstructed the same DebugToolbar/Panel/Config objects
the script's main() builds, independent of the script itself. That
let a broken script (e.g. a renamed import, wrong panel class, wrong
config key) pass the test suite as long as the underlying panel class
still worked when built directly.

Replace those with a single test per script that imports the module
and calls its real main() (or free functions, for cache_panel_demo),
capturing real printed output and asserting on specific computed
values (counts, ratios, byte totals, timings, redacted fields) that
would change if the panel actually broke.

Also widen examples-latest.yml's pull_request trigger to include
src/** and pyproject.toml, since a library change (not just an
examples/tests change) can break examples against upgraded
dependencies.

* test: rebase asgi_basic tests onto the merged PR #61 fix

PR #61 made the asgi_basic example inject the toolbar markup into HTML
responses, serve /_debug_toolbar/static/{toolbar.css,toolbar.js}, and
move its JSON endpoints to /_debug_toolbar/api/requests(/<id>). Flip
the tests that previously asserted the pre-fix absence of these
behaviors to assert the real, now-fixed behavior instead.

* test: assert real behavior now fixed by PR #64 and #67

PR #64 made the FastAPI dependency injection panel actually populate
resolved_dependencies for nested Depends() trees; assert on the real
dependency names (get_db, get_current_user, get_token) and counts
instead of only checking the panel key exists.

PR #67 made the async profiler's taskfactory backend report the real
caller-supplied task name (previously always 'Task-N') and made
blocking call detection actually fire (previously silent, since
asyncio logs slow callbacks via the asyncio logger rather than
call_exception_handler). Assert on the real task names and a real
detected blocking call instead of working around both bugs.

* test: drop comparison of shortened task ids in async profiler example test
@JacobCoffee JacobCoffee mentioned this pull request Oct 11, 2026
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