From 87328461a4c3d03db6fa1a0d38a5e9ba5ce9e66b Mon Sep 17 00:00:00 2001 From: steveluscher Date: Sat, 24 Feb 2024 00:00:19 +0000 Subject: [PATCH] Fix crash when `fetch` is aborted with `null` as the `AbortSignal's` `reason` --- index-fetch.js | 2 +- index.js | 2 +- test/fetch/issue-2242.js | 60 ++++++++++++++++++++++++++++++---------- 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/index-fetch.js b/index-fetch.js index b8b3f3c7cac..dc6c0d05e3e 100644 --- a/index-fetch.js +++ b/index-fetch.js @@ -4,7 +4,7 @@ const fetchImpl = require('./lib/web/fetch').fetch module.exports.fetch = function fetch (resource, init = undefined) { return fetchImpl(resource, init).catch((err) => { - if (typeof err === 'object') { + if (err && typeof err === 'object') { Error.captureStackTrace(err, this) } throw err diff --git a/index.js b/index.js index dd52f2e46a1..8997f348989 100644 --- a/index.js +++ b/index.js @@ -101,7 +101,7 @@ module.exports.fetch = async function fetch (init, options = undefined) { try { return await fetchImpl(init, options) } catch (err) { - if (typeof err === 'object') { + if (err && typeof err === 'object') { Error.captureStackTrace(err, this) } diff --git a/test/fetch/issue-2242.js b/test/fetch/issue-2242.js index ca12cc61c09..1c0fe1f4303 100644 --- a/test/fetch/issue-2242.js +++ b/test/fetch/issue-2242.js @@ -1,24 +1,54 @@ 'use strict' -const { test } = require('node:test') +const { beforeEach, describe, it } = require('node:test') const assert = require('node:assert') const { fetch } = require('../..') const nodeFetch = require('../../index-fetch') -test('fetch with signal already aborted', async () => { - await assert.rejects( - fetch('http://localhost', { - signal: AbortSignal.abort('Already aborted') - }), - /Already aborted/ +describe('Issue #2242', () => { + ['Already aborted', null, false, true, 123, Symbol('Some reason')].forEach( + (reason) => + describe(`when an already-aborted signal's reason is \`${String( + reason + )}\``, () => { + let signal + beforeEach(() => { + signal = AbortSignal.abort(reason) + }) + it('rejects with that reason ', async () => { + await assert.rejects(fetch('http://localhost', { signal }), (err) => { + assert.strictEqual(err, reason) + return true + }) + }) + it('rejects with that reason (from index-fetch)', async () => { + await assert.rejects( + nodeFetch.fetch('http://localhost', { signal }), + (err) => { + assert.strictEqual(err, reason) + return true + } + ) + }) + }) ) -}) -test('fetch with signal already aborted (from index-fetch)', async () => { - await assert.rejects( - nodeFetch.fetch('http://localhost', { - signal: AbortSignal.abort('Already aborted') - }), - /Already aborted/ - ) + describe("when an already-aborted signal's reason is `undefined`", () => { + let signal + beforeEach(() => { + signal = AbortSignal.abort(undefined) + }) + it('rejects with an `AbortError`', async () => { + await assert.rejects( + fetch('http://localhost', { signal }), + new DOMException('This operation was aborted', 'AbortError') + ) + }) + it('rejects with an `AbortError` (from index-fetch)', async () => { + await assert.rejects( + nodeFetch.fetch('http://localhost', { signal }), + new DOMException('This operation was aborted', 'AbortError') + ) + }) + }) })