Skip to content

fix(peer): send the cancel after an in-flight request message - #119

Merged
dinwwwh merged 5 commits into
mainfrom
claude/blissful-fermi-ftb4ay
Sep 29, 2026
Merged

dinwwwh merged 5 commits into
mainfrom
claude/blissful-fermi-ftb4ay

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Summary

When a ClientPeer request was aborted while its request message was still being sent, the cancel message could reach the server first. For example, the README transport port.postMessage(await encodePeerMessage(m)) awaits blob.arrayBuffer() for File/FormData bodies. ServerPeer ignores a cancel for an id it has not seen, so the handler then ran with signal.aborted === false, and a streaming response kept going until the peer closed.

Changes

  • ClientPeer tracks requestSent, which is set once send(request) resolves.
  • abortById sends the cancel only if the request message has been sent. Otherwise transmitRequest checks request.signal?.aborted after the request send and awaits the cancel there, so it always follows the request.
  • A request aborted before its message is sent (for example while its body is still being encoded) no longer sends a cancel at all.

Trade-off

On transports whose send only resolves after the server has handled the request, the cancel is no longer sent while that handling runs.

Tests

  • New integration test: aborting while file.arrayBuffer() is being read during encoding still aborts the server handler.
  • New unit tests: the cancel is sent after the in-flight request message, and no cancel is sent when the request message fails to send.
  • Two existing tests changed from expecting only a cancel message to expecting no messages, and one now waits for the cancel instead of asserting it immediately.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TSY5GvoAft9apocjwoSQrk

When a client request was aborted while its request message was still
being sent (e.g. `encodePeerMessage` awaiting `blob.arrayBuffer()`), the
cancel message could reach the server first. The server ignores cancels
for ids it has not seen, so the handler then ran un-aborted and a
streaming response kept going until the peer closed.

The client now sends the cancel again once the in-flight request message
is sent. The immediate cancel is kept for transports whose `send` only
resolves after the server handled the message, where it is the only way
to abort the running handler. A duplicate cancel is a no-op on the server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSY5GvoAft9apocjwoSQrk
- Track a single `cancelSent` flag instead of two: `transmitRequest` only
  finds the state gone right after the request send resolves, so the
  in-flight window is already implied there.
- Fire the resent cancel without awaiting it, so releasing the untransmitted
  request body is not delayed by another `send`.
- Tests: give `connect()` `awaitRemote` and `beforeClientSend` options
  instead of copying the wire setup, run the client abort propagation test
  for both transport styles, share the abort-while-sending setup in the
  client tests, and fold a duplicate test into the existing one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSY5GvoAft9apocjwoSQrk
Instead of cancelling right away and resending afterwards, track whether
the request message has been sent. `abortById` only sends the cancel once
it has; otherwise `transmitRequest` checks the signal after the request
send and awaits the cancel there. This sends a single cancel, always after
the request, and none at all when the request is aborted before it is sent.

Transports whose `send` only resolves after the server has handled the
request no longer get a cancel while that handling runs, so the abort test
for that transport style is dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSY5GvoAft9apocjwoSQrk
- Explain why the cancel is held back once, on `requestSent`, instead of
  at both call sites.
- Client tests: drive the abort-while-sending helper from the `send` mock
  and the released body instead of polling, and fold the failed-cancel
  test into the existing abort-during-send body test.
- Integration tests: restore `connect()` and its wiring from main, and
  hold back the real `file.arrayBuffer()` read instead of a custom hook.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSY5GvoAft9apocjwoSQrk
… catch

Drop the `.catch(() => {})` on the cancel sent after the request message.
A failed delivery is already handled by the surrounding try/catch, which
closes the request and cancels the unsent body with that error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TSY5GvoAft9apocjwoSQrk
@pkg-pr-new

pkg-pr-new Bot commented Sep 29, 2026

Copy link
Copy Markdown
@standard-server/aws-lambda

npm i https://pkg.pr.new/@standard-server/aws-lambda@119

@standard-server/core

npm i https://pkg.pr.new/@standard-server/core@119

@standard-server/fastify

npm i https://pkg.pr.new/@standard-server/fastify@119

@standard-server/fetch

npm i https://pkg.pr.new/@standard-server/fetch@119

@standard-server/node

npm i https://pkg.pr.new/@standard-server/node@119

@standard-server/peer

npm i https://pkg.pr.new/@standard-server/peer@119

@standard-server/shared

npm i https://pkg.pr.new/@standard-server/shared@119

commit: caa86c4

@dinwwwh dinwwwh changed the title Fix abort cancel message ordering during request transmission fix(peer): send the cancel after an in-flight request message Sep 29, 2026
@codspeed

codspeed Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 108 skipped benchmarks1


Comparing claude/blissful-fermi-ftb4ay (caa86c4) with main (f414f41)

Open in CodSpeed

Footnotes

  1. 108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@pullfrog pullfrog Bot 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.

✅ No new issues found.

Reviewed changes — verified the client-side cancel ordering fix end to end and traced the abort racing paths; the full suite (1244 tests), eslint, and tsc -b pass.

  • Defer cancel until the request message is sent — ClientPeerRequestStateInternal.requestSent gates abortById's cancel send (client.ts:279), with transmitRequest emitting the held-back cancel once the in-flight request send resolves (client.ts:99-105).
  • No cancel when the request was never sent — abort during atomic-body encode now sends nothing (previously sent a cancel that the server dropped), and a rejected request send intentionally sends no cancel.
  • Tests — new unit helper/tests for abort during an in-flight send (cancel after delivery / none on send failure), plus an integration test aborting mid-encode that only passes with the corrected ordering; existing expectations updated accordingly.

The reasoning is sound: the early set of requestSent and the post-send check share one synchronous block, so no double-cancel window exists, and the server's closeById already no-ops for unknown ids, which is exactly why the old overtaking cancel was lost. I traced the stream-transmit abort path and the peer-close path as unaffected.

Pullfrog  | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

@dinwwwh
dinwwwh merged commit 05f5ba9 into main Sep 29, 2026
16 of 17 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.

2 participants