Skip to content

fix: reject a buffered response when the request was aborted in flight - #231

Open
bengeorge wants to merge 1 commit into
margelo:mainfrom
bengeorge:fix/abort-after-buffered-response
Open

fix: reject a buffered response when the request was aborted in flight#231
bengeorge wants to merge 1 commit into
margelo:mainfrom
bengeorge:fix/abort-after-buffered-response

Conversation

@bengeorge

@bengeorge bengeorge commented Sep 8, 2026

Copy link
Copy Markdown

Closes #232.

Problem

fetch() can resolve as a normal success after its AbortSignal has fired. Cronet's UrlRequest.cancel() and URLSessionTask.cancel() are best-effort and are documented no-ops once the request has completed, so when the cancel loses that race nitroFetchRaw hands back a response the caller has already abandoned — which then goes on to parse a body it discarded. nitroStreamFetch already checks signal?.aborted in several places; only the buffered path is missing it.

Full diagnosis and the device measurement are in #232. In short: on a Fire TV stick, 4 of 4 superseded search requests completed and were JSON-parsed with signal.aborted === true; with this change, 0 of 4.

Fix

  const res: NitroResponseNative = await client.request(req);
+ if (signal?.aborted) {
+   throw createAbortError();
+ }

Placed before NetworkInspector._recordEnd so an aborted request falls into the existing catch and is recorded as aborted, rather than as a 200 the caller never received.

The existing abort mid-flight cancels a slow request harness case passes without this change — aborting 100 ms into /delay/20 lets the native cancel win comfortably — so it never covered the window where the response is already buffered. Added a case alongside it that blocks the JS thread to hold that window open.

Testing

  • bun test — 10 pass, 2 existing todos, 0 fail
  • bun typecheck — clean
  • eslint "**/*.{js,ts,tsx}" — 0 errors, the 2 existing no-shadow warnings in the Expo plugin files. Run under node; bun lint hits an unrelated structuredClone is not defined in @eslint/config-array on macOS that CI on Linux does not
  • tsc --noEmit in example/ — the new harness case adds no errors; the 4 in that file are identical to those on main
  • The new harness case itself is not runtest:harness needs the example app built onto a device, which I have not done

@vercel

vercel Bot commented Sep 8, 2026

Copy link
Copy Markdown

@bengeorge is attempting to deploy a commit to the Margelo Team on Vercel.

A member of the Team first needs to authorize it.

@bengeorge
bengeorge force-pushed the fix/abort-after-buffered-response branch from c5b123e to 59008ce Compare September 8, 2026 06:19
@bengeorge
bengeorge force-pushed the fix/abort-after-buffered-response branch from 59008ce to 08a8d4b Compare September 8, 2026 06:24
@bengeorge
bengeorge marked this pull request as ready for review September 8, 2026 12:05
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.

AbortSignal is ignored when it fires after the response is buffered (non-streaming fetch)

1 participant