From 8f3490d9ee1fdb229a71bd4df118708fe708132b Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 26 Jan 2026 06:13:25 +0000 Subject: [PATCH] fix: MockAgent delayed response with AbortSignal (#4693) When a MockAgent had a delayed response and the request was aborted via AbortSignal, the delayed response callback would still fire and attempt to call handler methods after onError had been called. This violated state invariants expected by handlers like DecoratorHandler (used by decompress interceptor). The fix: - Tracks abort state in mockDispatch - Clears pending setTimeout timer when abort is called - Checks abort flag in handleReply before invoking handler callbacks - Properly calls handler.onConnect to register abort callback Fixes #4693 --- lib/mock/mock-utils.js | 37 +++++++++- test/mock-delayed-abort.js | 137 +++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 test/mock-delayed-abort.js diff --git a/lib/mock/mock-utils.js b/lib/mock/mock-utils.js index e1e3f040643..291a85753be 100644 --- a/lib/mock/mock-utils.js +++ b/lib/mock/mock-utils.js @@ -312,9 +312,33 @@ function mockDispatch (opts, handler) { return true } + // Track whether the request has been aborted + let aborted = false + let timer = null + + function abort (err) { + if (aborted) { + return + } + aborted = true + + // Clear the pending delayed response if any + if (timer !== null) { + clearTimeout(timer) + timer = null + } + + // Notify the handler of the abort + handler.onError(err) + } + + // Call onConnect to allow the handler to register the abort callback + handler.onConnect?.(abort, null) + // Handle the request with a delay if necessary if (typeof delay === 'number' && delay > 0) { - setTimeout(() => { + timer = setTimeout(() => { + timer = null handleReply(this[kDispatches]) }, delay) } else { @@ -322,6 +346,11 @@ function mockDispatch (opts, handler) { } function handleReply (mockDispatches, _data = data) { + // Don't send response if the request was aborted + if (aborted) { + return + } + // fetch's HeadersList is a 1D string array const optsHeaders = Array.isArray(opts.headers) ? buildHeadersFromArray(opts.headers) @@ -340,11 +369,15 @@ function mockDispatch (opts, handler) { return body.then((newData) => handleReply(mockDispatches, newData)) } + // Check again if aborted after async body resolution + if (aborted) { + return + } + const responseData = getResponseData(body) const responseHeaders = generateKeyValues(headers) const responseTrailers = generateKeyValues(trailers) - handler.onConnect?.(err => handler.onError(err), null) handler.onHeaders?.(statusCode, responseHeaders, resume, getStatusText(statusCode)) handler.onData?.(Buffer.from(responseData)) handler.onComplete?.(responseTrailers) diff --git a/test/mock-delayed-abort.js b/test/mock-delayed-abort.js new file mode 100644 index 00000000000..abdf086ffe8 --- /dev/null +++ b/test/mock-delayed-abort.js @@ -0,0 +1,137 @@ +'use strict' + +const { test } = require('node:test') +const { MockAgent, interceptors } = require('..') +const DecoratorHandler = require('../lib/handler/decorator-handler') +const { tspl } = require('@matteo.collina/tspl') + +test('MockAgent with delayed response and AbortSignal should not cause uncaught errors', async (t) => { + const p = tspl(t, { plan: 2 }) + + const agent = new MockAgent() + t.after(() => agent.close()) + + const mockPool = agent.get('https://example.com') + mockPool.intercept({ path: '/test', method: 'GET' }) + .reply(200, { success: true }, { headers: { 'content-type': 'application/json' } }) + .delay(100) + + const ac = new AbortController() + + // Abort the request after 10ms + setTimeout(() => { + ac.abort(new Error('Request aborted')) + }, 10) + + try { + await agent.request({ + origin: 'https://example.com', + path: '/test', + method: 'GET', + signal: ac.signal + }) + p.fail('Should have thrown an error') + } catch (err) { + p.ok(err.message === 'Request aborted' || err.name === 'AbortError', 'Error should be related to abort') + } + + // Wait for the delayed response to fire - should not cause any uncaught errors + await new Promise(resolve => setTimeout(resolve, 150)) + + p.ok(true, 'No uncaught errors after delayed response') +}) + +test('MockAgent with delayed response and composed interceptor (decompress) should not cause uncaught errors', async (t) => { + const p = tspl(t, { plan: 2 }) + + // The decompress interceptor has assertions that fail if onResponseStart is called after onError + const agent = new MockAgent().compose(interceptors.decompress()) + t.after(() => agent.close()) + + const mockPool = agent.get('https://example.com') + mockPool.intercept({ path: '/test', method: 'GET' }) + .reply(200, { success: true }, { headers: { 'content-type': 'application/json' } }) + .delay(100) + + const ac = new AbortController() + + // Abort the request after 10ms + setTimeout(() => { + ac.abort(new Error('Request aborted')) + }, 10) + + try { + await agent.request({ + origin: 'https://example.com', + path: '/test', + method: 'GET', + signal: ac.signal + }) + p.fail('Should have thrown an error') + } catch (err) { + p.ok(err.message === 'Request aborted' || err.name === 'AbortError', 'Error should be related to abort') + } + + // Wait for the delayed response to fire - should not cause any uncaught errors + await new Promise(resolve => setTimeout(resolve, 150)) + + p.ok(true, 'No uncaught errors after delayed response') +}) + +test('MockAgent with delayed response and DecoratorHandler should not call onResponseStart after onError', async (t) => { + const p = tspl(t, { plan: 2 }) + + class TestDecoratorHandler extends DecoratorHandler { + #onErrorCalled = false + + onResponseStart (controller, statusCode, headers, statusMessage) { + if (this.#onErrorCalled) { + p.fail('onResponseStart should not be called after onError') + } + return super.onResponseStart(controller, statusCode, headers, statusMessage) + } + + onResponseError (controller, err) { + this.#onErrorCalled = true + return super.onResponseError(controller, err) + } + } + + const agent = new MockAgent() + t.after(() => agent.close()) + + const mockPool = agent.get('https://example.com') + mockPool.intercept({ path: '/test', method: 'GET' }) + .reply(200, { success: true }, { headers: { 'content-type': 'application/json' } }) + .delay(100) + + const ac = new AbortController() + + // Abort the request after 10ms + setTimeout(() => { + ac.abort(new Error('Request aborted')) + }, 10) + + const originalDispatch = agent.dispatch.bind(agent) + agent.dispatch = (opts, handler) => { + const decoratedHandler = new TestDecoratorHandler(handler) + return originalDispatch(opts, decoratedHandler) + } + + try { + await agent.request({ + origin: 'https://example.com', + path: '/test', + method: 'GET', + signal: ac.signal + }) + p.fail('Should have thrown an error') + } catch (err) { + p.ok(err.message === 'Request aborted' || err.name === 'AbortError', 'Error should be related to abort') + } + + // Wait for the delayed response to fire + await new Promise(resolve => setTimeout(resolve, 150)) + + p.ok(true, 'Decorator handler invariants maintained') +})