Skip to content

feat(fastapi): CancelOnDisconnectRoute - cancel the handler (and its query) when the client disconnects - #58

Merged
aersam merged 7 commits into
mainfrom
cancel-on-disconnect-route
Oct 10, 2026
Merged

aersam merged 7 commits into
mainfrom
cancel-on-disconnect-route

Conversation

@aersam

@aersam aersam commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Granian (our server) does not cancel a request handler when the client disconnects (verified over a real socket). Only PostgresJsonResponse watches for the disconnect; fetch_all/raw-cursor/Meili based endpoints run to completion, so frontend AbortController aborts (debounced search, grid filter changes) don't free the database.

Change

  • New opt-in pgdevkit.fastapi.CancelOnDisconnectRoute (APIRouter(route_class=...)): cancels the handler when the client disconnects (in the request's own task, via an asyncio.timeout cancel scope as in PostgresJsonResponse); psycopg/pgdevkit then cancel the running statement. The handler's Request gets a receive() fed by a small bounded reader queue, so the body is not buffered. The gone client gets a 499 (an HTTPException, so yield dependencies roll back, and BaseHTTPMiddleware stays happy).
  • Opt-in only (writes must never be cancelled; many read endpoints are POSTs).
  • Tests (14 in-process + real Granian socket with a BaseHTTPMiddleware in the stack): disconnect cancels query, plain routes unaffected, big/unread bodies, yield dependencies see a failure, context-var dependencies work, exception handlers can read the body, outer cancellation during cleanup is not swallowed, finished-handler-wins race. Mutation-checked.
  • README + pgdevkit skill document it, incl. caveats: everything in the route is cancelled (dependencies, background tasks), shared single-flight work must be awaited via asyncio.shield.
  • Version bump 0.16.0 (auto-release on merge).

Reviewed with /bms-code-review --full (correctness, security, backend, docs); findings addressed. Known, not fixed here (separate issue, pre-existing in json_response.py): with BaseHTTPMiddleware in front, an aborted PostgresJsonResponse before its first byte logs RuntimeError: No response returned; the disconnect watcher there can call reschedule on a finished timeout.

🤖 Generated with Claude Code

aersam and others added 7 commits October 9, 2026 21:08
…lient leaves (0.16.0)

ASGI servers such as Granian don't cancel a request handler on disconnect, so
fetch_all-based endpoints ran to completion after a frontend abort. The opt-in
route class runs the handler as a task and cancels it on http.disconnect;
psycopg/pgdevkit then cancel the statement on the server.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…gone before the handler starts

request.body() raised ClientDisconnect there, which surfaced as a 500 with a traceback.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ield dependencies roll back, keep outer cancellation

Review findings on CancelOnDisconnectRoute:
- request.body() buffered the whole body before dependencies ran (memory pinning
  by unauthenticated large uploads). The handler now gets a Request fed by a
  bounded reader queue that also spots the disconnect.
- A yield dependency saw a clean exit after a cancelled handler; the route now
  raises HTTPException(499), so transactions roll back.
- _cancel swallowed an outer cancellation arriving during the handler's cleanup.
- A failing receive() was reported as a disconnect (499).
- Docs: dependencies/BackgroundTasks are cancelled too, SKILL description and
  checklist, cross references from the fetch_all/cancel docs.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…an asyncio timeout scope

The separate handler task broke yield dependencies that set/reset context
variables (ValueError: Token created in a different Context, a 500 on every
request) and gave exception handlers a Request whose body the reader had already
taken (they hung on request.body()). Cancelling via asyncio.timeout(None) +
reschedule, as PostgresJsonResponse does, keeps one task and one Request object
and removes the cleanup-wait helper.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
… front with an empty 499

BaseHTTPMiddleware fails with 'RuntimeError: No response returned.' (traceback in
the log) when the client disconnects before the first byte and nothing was sent.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@aersam
aersam marked this pull request as ready for review October 10, 2026 05:18
@aersam
aersam merged commit 38c01a1 into main Oct 10, 2026
2 checks passed
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.

1 participant