Skip to content

feat(interceptors): add signal interceptor#5541

Open
Arman-Luthra wants to merge 1 commit into
nodejs:mainfrom
Arman-Luthra:feat/signal-interceptor
Open

feat(interceptors): add signal interceptor#5541
Arman-Luthra wants to merge 1 commit into
nodejs:mainfrom
Arman-Luthra:feat/signal-interceptor

Conversation

@Arman-Luthra

Copy link
Copy Markdown

This relates to...

Closes #3276

Rationale

#3276 asks for AbortSignal handling to be available as an interceptor. Following the direction given in the issue thread, this adds a completely new signal interceptor shaped like interceptors.responseError: when the signal dispatch option is aborted, the request is aborted and the abort listener is cleaned up.

This is mainly useful when calling dispatch() directly with a custom handler, where today every caller has to wire up (and remember to remove) their own abort listener the way the API-layer handlers in lib/api do. The existing API-layer signal handling is intentionally left untouched.

Changes

  • Add lib/interceptor/signal.js, exported as interceptors.signal
  • Supports both AbortSignal and EventEmitter-style signals (same contract as the signal option of the API methods, validated the same way)
  • Aborts with signal.reason when available, RequestAbortedError otherwise
  • A signal that is already aborted fails the dispatch before the request reaches the server, mirroring RequestHandler
  • The abort listener is registered once per dispatched request and removed on onResponseEnd, onResponseError and onRequestUpgrade, so long-lived signals do not accumulate listeners
  • The controller reference is refreshed on every onRequestStart so composing with retry() keeps aborting the active attempt
  • Docs (docs/docs/api/Interceptors.md, docs/docs/api/Dispatcher.md), types (types/interceptors.d.ts) and type tests included

Features

  • New interceptors.signal() interceptor

Bug Fixes

N/A

Breaking Changes and Deprecations

N/A

Testing

test/interceptors/signal.js exercises the interceptor through client.dispatch() with a bare handler rather than client.request(), since the API-layer handlers already implement their own signal handling and would mask what the interceptor does. Covered: pre-aborted signal (exact reason propagated, request never reaches the server, only onResponseError is delivered), mid-response abort, EventEmitter-style signals, listener removal after completion and after an error (asserted via getEventListeners/listenerCount), pass-through when no signal is provided, InvalidArgumentError for non-signal values, and compatibility with client.request().

npm run lint, npm run test:interceptors and the TypeScript checks pass locally (the pre-existing zstd decompress test is skipped locally as Node 22.14 lacks zlib.createZstdCompress).

Status

Add a new interceptor that aborts the dispatched request when the
signal dispatch option is aborted, per the direction in nodejs#3276.

Supports both AbortSignal and EventEmitter-style signals, aborts with
signal.reason when available (RequestAbortedError otherwise), and
removes the abort listener once the request completes, errors or is
upgraded. A signal that is already aborted fails the dispatch before
the request reaches the server.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.84211% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.46%. Comparing base (c7c7e7f) to head (c999117).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
lib/interceptor/signal.js 96.77% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5541      +/-   ##
==========================================
+ Coverage   93.45%   93.46%   +0.01%     
==========================================
  Files         110      111       +1     
  Lines       37376    37470      +94     
==========================================
+ Hits        34928    35020      +92     
- Misses       2448     2450       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

interceptors: move signal handling to interceptor

2 participants