Skip to content

Handle client-cancelled shape requests as disconnects instead of 500s - #4791

Merged
alco merged 2 commits into
mainfrom
client-disconnect-handling
Sep 1, 2026
Merged

Handle client-cancelled shape requests as disconnects instead of 500s#4791
alco merged 2 commits into
mainfrom
client-disconnect-handling

Conversation

@alco

@alco alco commented Aug 31, 2026

Copy link
Copy Markdown
Member

Fixes #4790

Two complementary changes:

  1. Abort the long poll the moment the cancel arrives. wait_for_live_change/2 now also matches Bandit's {:bandit, {:rst_stream, _}} notification and completes the request immediately — status 499, empty body, change-listener cleanup — releasing the process and admission permit at cancel time. The message format is Bandit-private (Bandit.HTTP2.Stream.deliver_rst_stream/2); the receive clause carries a comment pinning that. HTTP/1 has no equivalent early signal and is covered by the second change only.

  2. Classify send-time transport failures as client disconnects. A %Bandit.TransportError{error: :closed} caught around the plug pipeline is now accounted for as status 499 (nginx's "client closed request" convention) with a shape_req.client_disconnected span attribute — no record_exception, no unreadable error body — and is reraised for Bandit to close the stream through its native quiet path (log_client_closures: false by default). This mirrors how the chunked streaming path already treats {:error, "closed"} as a quiet halt.

The [:electric, :plug, :serve_shape] telemetry event still fires for these requests (with status 499), so cancellation volume stays measurable while dropping out of the 5xx error signal.

🤖 Generated with Claude Code

https://claude.ai/code/session_01C4Hiytggpfq8UqvFnshh5r

alco and others added 2 commits August 31, 2026 19:30
Client-cancelled live long-polls were the single dominant source of error
spans on /v1/shape: when a client resets the HTTP/2 stream mid-poll, Bandit
only surfaces the reset lazily — as a Bandit.TransportError raised from
send_resp once the long-poll timeout fires — and the generic exception path
then mapped it to a 500 nobody ever received, while the handler process and
its admission permit stayed occupied for the full timeout after the client
had already gone.

Two complementary changes:

- Abort the long poll early: the wait-for-changes receive now also matches
  Bandit's rst_stream notification message, so a cancelled request completes
  (status 499, empty body, listener cleanup) the moment the client resets
  the stream, releasing the process and admission permit immediately.

- Classify send-time transport failures: a Bandit.TransportError with
  error: :closed caught around the plug pipeline is now accounted for as a
  client disconnect — status 499 and a `shape_req.client_disconnected` span
  attribute, with no recorded exception and no unreadable error body — and
  is reraised for Bandit to close the stream quietly, instead of being
  reported as a 500 with a recorded exception.

Fixes #4790

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C4Hiytggpfq8UqvFnshh5r
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
2332 1 2331 2371
View the top 2 failed test(s) by shortest run time
Elixir.Electric.Shapes.FilterTest::test optimisations where clause in the form `array_field @> const_array` is optimised
Stack Traces | 0.0478s run time
11) test optimisations where clause in the form `array_field @> const_array` is optimised (Electric.Shapes.FilterTest)
     .../electric/shapes/filter_test.exs:894
     Assertion with < failed
     code:  assert add_reductions < max_reductions
     left:  7980
     right: 6500
     stacktrace:
       (elixir 1.20.2) lib/enum.ex:983: Enum."-each/2-lists^foreach/1-0-"/2
       .../electric/shapes/filter_test.exs:922: (test)
test/runtime-dsl.test.ts > F: coordination orchestration > F1: dispatcher routes to the requested specialist type and records the child
Stack Traces | 0.137s run time
AssertionError: expected false to be true // Object.is equality

- Expected
+ Received

- true
+ false

 ❯ test/runtime-dsl.test.ts:3936:7

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@alco
alco merged commit 2e9e9b4 into main Sep 1, 2026
57 of 63 checks passed
@alco
alco deleted the client-disconnect-handling branch September 1, 2026 09:46
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

This PR has been released! 🚀

The following packages include changes from this PR:

  • @core/sync-service@1.8.0

Thanks for contributing to Electric!

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.

Client-cancelled live long-polls are misreported as 500s and hold resources for the full 20s timeout

1 participant