diff --git a/lib/web/fetch/body.js b/lib/web/fetch/body.js index 3d2d848656a..5d172496527 100644 --- a/lib/web/fetch/body.js +++ b/lib/web/fetch/body.js @@ -422,18 +422,14 @@ function consumeBody (object, convertBytesToJSValue, instance, getInternalState) return Promise.reject(e) } - const state = getInternalState(object) + object = getInternalState(object) // 1. If object is unusable, then return a promise rejected // with a TypeError. - if (bodyUnusable(state)) { + if (bodyUnusable(object)) { return Promise.reject(new TypeError('Body is unusable: Body has already been read')) } - if (state.aborted) { - return Promise.reject(new DOMException('The operation was aborted.', 'AbortError')) - } - // 2. Let promise be a new promise. const promise = createDeferredPromise() @@ -454,14 +450,14 @@ function consumeBody (object, convertBytesToJSValue, instance, getInternalState) // 5. If object’s body is null, then run successSteps with an // empty byte sequence. - if (state.body == null) { + if (object.body == null) { successSteps(Buffer.allocUnsafe(0)) return promise.promise } // 6. Otherwise, fully read object’s body given successSteps, // errorSteps, and object’s relevant global object. - fullyReadBody(state.body, successSteps, errorSteps) + fullyReadBody(object.body, successSteps, errorSteps) // 7. Return promise. return promise.promise diff --git a/lib/web/fetch/index.js b/lib/web/fetch/index.js index 8224e1b4ff7..47d0a1fde09 100644 --- a/lib/web/fetch/index.js +++ b/lib/web/fetch/index.js @@ -157,7 +157,7 @@ function fetch (input, init = undefined) { if (requestObject.signal.aborted) { // 1. Abort the fetch() call with p, request, null, and // requestObject’s signal’s abort reason. - abortFetch(p, request, null, requestObject.signal.reason) + abortFetch(p, request, null, requestObject.signal.reason, null) // 2. Return p. return p.promise @@ -200,7 +200,7 @@ function fetch (input, init = undefined) { // 4. Abort the fetch() call with p, request, responseObject, // and requestObject’s signal’s abort reason. - abortFetch(p, request, realResponse, requestObject.signal.reason) + abortFetch(p, request, realResponse, requestObject.signal.reason, controller.controller) } ) @@ -227,7 +227,7 @@ function fetch (input, init = undefined) { // 2. Abort the fetch() call with p, request, responseObject, and // deserializedError. - abortFetch(p, request, responseObject, controller.serializedAbortReason) + abortFetch(p, request, responseObject, controller.serializedAbortReason, controller.controller) return } @@ -327,7 +327,7 @@ function finalizeAndReportTiming (response, initiatorType = 'other') { const markResourceTiming = performance.markResourceTiming // https://fetch.spec.whatwg.org/#abort-fetch -function abortFetch (p, request, responseObject, error) { +function abortFetch (p, request, responseObject, error, controller /* undici-specific */) { // 1. Reject promise with error. if (p) { // We might have already resolved the promise at this stage @@ -357,13 +357,7 @@ function abortFetch (p, request, responseObject, error) { // 5. If response’s body is not null and is readable, then error response’s // body with error. if (response.body?.stream != null && isReadable(response.body.stream)) { - response.body.stream.cancel(error).catch((err) => { - if (err.code === 'ERR_INVALID_STATE') { - // Node bug? - return - } - throw err - }) + controller.error(error) } } diff --git a/test/fetch/issue-4799.js b/test/fetch/issue-4799.js new file mode 100644 index 00000000000..3e59df0030d --- /dev/null +++ b/test/fetch/issue-4799.js @@ -0,0 +1,109 @@ +'use strict' + +const { test } = require('node:test') +const { fetch } = require('../..') +const { createServer } = require('node:http') +const { once } = require('node:events') +const { closeServerAsPromise } = require('../utils/node-http') + +test('response clone + abort should return AbortError, not TypeError', async (t) => { + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + res.statusCode = 200 + res.setHeader('Content-Type', 'application/json') + res.end(JSON.stringify({ message: 'hello' })) + }) + + t.after(closeServerAsPromise(server)) + server.listen(0) + await once(server, 'listening') + + const controller = new AbortController() + const response = await fetch(`http://localhost:${server.address().port}`, { + signal: controller.signal + }) + + // Clone the response before aborting + const clonedResponse = response.clone() + + // Abort after cloning + controller.abort() + + // Both original and cloned response should reject with AbortError + await t.test('original response should reject with AbortError', async () => { + await t.assert.rejects( + response.text(), + { + name: 'AbortError', + code: DOMException.ABORT_ERR + } + ) + }) + + await t.test('cloned response should reject with AbortError', async () => { + await t.assert.rejects( + clonedResponse.text(), + { + name: 'AbortError', + code: DOMException.ABORT_ERR + } + ) + }) +}) + +test('response without clone + abort should still return AbortError', async (t) => { + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + res.statusCode = 200 + res.setHeader('Content-Type', 'application/json') + res.end(JSON.stringify({ message: 'hello' })) + }) + + t.after(closeServerAsPromise(server)) + server.listen(0) + await once(server, 'listening') + + const controller = new AbortController() + const response = await fetch(`http://localhost:${server.address().port}`, { + signal: controller.signal + }) + + // Abort without cloning + controller.abort() + + await t.assert.rejects( + response.text(), + { + name: 'AbortError', + code: DOMException.ABORT_ERR + } + ) +}) + +test('response bodyUsed after clone and abort', async (t) => { + const server = createServer({ joinDuplicateHeaders: true }, (req, res) => { + res.statusCode = 200 + res.setHeader('Content-Type', 'application/json') + res.end(JSON.stringify({ message: 'hello' })) + }) + + t.after(closeServerAsPromise(server)) + server.listen(0) + await once(server, 'listening') + + const controller = new AbortController() + const response = await fetch(`http://localhost:${server.address().port}`, { + signal: controller.signal + }) + + t.assert.strictEqual(response.bodyUsed, false) + + const clonedResponse = response.clone() + + t.assert.strictEqual(response.bodyUsed, false) + t.assert.strictEqual(clonedResponse.bodyUsed, false) + + controller.abort() + + // Erroring a stream (see: https://fetch.spec.whatwg.org/#abort-fetch step 5) does not disturb it. + t.assert.strictEqual(response.bodyUsed, false) + t.assert.strictEqual(clonedResponse.bodyUsed, false) +})