Skip to content

fix: normalise non-native AbortSignal for Node 24+ compatibility#205

Merged
bkaney merged 7 commits into
mainfrom
fix/non-native-abort-signal-node24
May 16, 2026
Merged

fix: normalise non-native AbortSignal for Node 24+ compatibility#205
bkaney merged 7 commits into
mainfrom
fix/non-native-abort-signal-node24

Conversation

@bkaney
Copy link
Copy Markdown
Member

@bkaney bkaney commented May 16, 2026

Problem

Closes #204

On Node 24+, undici's Request constructor enforces a strict instanceof AbortSignal check. Polyfill libraries like node-abort-controller create their own AbortSignal class that is not the native one, causing a TypeError at request-build time:

TypeError: RequestInit: Expected signal ("AbortSignal {...}") to be an instance of AbortSignal.

The crash site was http-client.ts where the incoming signal was assigned directly to RequestInit and forwarded to new Request(url, requestInit):

if (options.signal) requestInit.signal = options.signal; // ← non-native signal
return new Request(url, requestInit);                     // ← undici rejects on Node 24+

Fix

Introduce a normalizeSignal() helper that bridges any foreign signal to a native AbortController/AbortSignal pair before it reaches new Request():

function normalizeSignal(signal: AbortSignal): AbortSignal {
  if (signal instanceof AbortSignal) return signal; // native → no-op

  const controller = new AbortController();
  if (signal.aborted) {
    controller.abort(signal.reason);          // already aborted → propagate immediately
  } else {
    signal.addEventListener('abort', () => controller.abort(signal.reason), { once: true });
  }
  return controller.signal;
}

Properties of the fix:

  • ✅ Zero-cost for the happy path — native signals pass through unchanged
  • ✅ Correctly propagates an already-aborted signal
  • ✅ Correctly forwards a live abort event from the foreign signal
  • ✅ No new dependencies — uses only built-in globals available on all supported Node versions

Tests

Two regression tests added under describe('AbortSignal compatibility') in test/client.test.ts using a FakeAbortSignal (a custom EventTarget subclass that mirrors node-abort-controller's pattern):

  1. Non-native signal on a live request — verifies the request completes successfully
  2. Already-aborted non-native signal — verifies the request is correctly rejected

…ructor

On Node 24+, undici enforces a strict instanceof AbortSignal check inside
the Request constructor. Polyfill libraries such as node-abort-controller
create their own AbortSignal class that fails this check, causing:

  TypeError: RequestInit: Expected signal ("AbortSignal {...}") to be an
  instance of AbortSignal.

Introduce normalizeSignal() in http-client.ts that bridges any foreign
signal to a native AbortController/AbortSignal pair, preserving both the
already-aborted and live-cancel cases. Native signals pass through as a
no-op via an instanceof guard.

Fixes #204
bkaney added 6 commits May 16, 2026 19:20
- package.json: simplify check script to 'biome check' so Biome uses its
  own files.includes config rather than a shell glob that fails to expand
- biome.json: reformat to Biome's expected indent style (spaces)
- test/test-utils.ts: sort named imports (dirname before join)
- test/setup.ts: sort named imports (afterAll, afterEach, beforeAll)
- test/smart.test.ts: move src import before local import
- test/utils.test.ts: sort named imports (describe, expect, it)
- test/capability-tool.test.ts: sort imports; reformat long lines
- test/client.test.ts: sort imports; reformat long lines; remove
  unnecessary = undefined initialiser
- test/reference-resolver.test.ts: sort imports
- test/request-signer.test.ts: sort imports; reformat long lines
MSW's CookieStore.getCookieStoreIndex() guards localStorage access with
`typeof localStorage === 'undefined'`, but Node 22+ exposes localStorage
as a global Web Storage API even without a backing file. The guard passes,
the access fires, and Node emits a warning per test worker:

  Warning: `--localstorage-file` was provided without a valid path

Pass `--localstorage-file` pointing at a temp file via vitest's top-level
`execArgv` option so Node's web storage implementation has a valid backing
file and the warning is suppressed.

Also bumps msw from 2.13.6 -> 2.14.6 (warning not yet fixed upstream).
…rage

## Bug fix
- client: update() now throws 'update requires either id or searchParams'
  when called without either parameter, preventing a silent PUT to
  'ResourceType/undefined'

## Simplification
- client: replace manual FetchQueue/Promise constructor pattern in
  smartAuthMetadata() with Promise.any() + per-request AbortControllers;
  removes ~25 lines of custom racing logic and the FetchQueue abstraction
- src/fetch-queue.ts deleted (was only used by smartAuthMetadata)

## Structural
- http-client: move agentCache from module-level singleton to instance
  property; eliminates hidden shared state between HttpClient instances,
  improving test isolation and predictability

## Test coverage (+32 tests, 143 → 175)
- utils: direct tests for validResourceType (null, undefined, empty,
  whitespace, leading slash, URL with colon) and createQueryString
  (undefined, empty object, scalar, multi-param, array, mixed)
- http-client: new test/http-client.test.ts covering expandUrl edge cases
  (absolute passthrough, trailing-slash combinations, empty path)
- client: resourceType validation tested on every method that accepts one
  (vread, create, update, delete, patch, operation, search,
  compartmentSearch, history, compartment resourceType)
- client: update() mutual-exclusion guard tests (neither / both)
- client: smartAuthMetadata all-endpoints-fail rejection path
- smart: authFromWellKnown registerUrl (registration_endpoint) parsing
…ignals

With strict narrowing, typing normalizeSignal's parameter as AbortSignal
caused the instanceof AbortSignal false-branch to collapse to never —
a value typed as AbortSignal that fails that check is impossible.

Introduce SignalLike: a minimal structural interface (aborted, reason,
addEventListener) that both the native AbortSignal and polyfills such as
node-abort-controller satisfy. Using SignalLike as the parameter type
means the false branch retains all properties we need.

Changes:
- types.ts: add SignalLike interface; narrow RequestOptions to omit
  'signal' from RequestInit and redeclare it as signal?: SignalLike
- http-client.ts: normalizeSignal(signal: SignalLike)
- index.ts: export SignalLike as part of the public API
@bkaney bkaney merged commit 1aef07d into main May 16, 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.

node-abort-controller errors on node 24

1 participant