diff --git a/PRE_RELEASE_CHANGELOG.md b/PRE_RELEASE_CHANGELOG.md index eaaf648a..197ba6d6 100644 --- a/PRE_RELEASE_CHANGELOG.md +++ b/PRE_RELEASE_CHANGELOG.md @@ -8,6 +8,7 @@ ## Bug Fixes +- **Error Serialization:** Made `AxiosError.cause` non-enumerable, matching native `Error` semantics and preventing structured loggers from recursing into circular network internals. (**#10913**, closes **#7205**) - **URL Validation:** Reject malformed `http:` and `https:` URLs that omit `//` before adapter URL normalization, returning `ERR_INVALID_URL` instead of silently normalizing invalid input. (**#10900**, closes **#7315**) - **Types:** Add the missing readonly `name: 'CanceledError'` declaration to CommonJS `CanceledError` typings to match the ESM declarations. (**#10922**) - **Config Merge:** Added `transitional.validateStatusUndefinedResolves` (default `true`) so applications can opt into treating explicit `validateStatus: undefined` like an omitted option by setting it to `false`. `validateStatus: null` still accepts every response status. (**#10899**, closes **#6688**) diff --git a/lib/adapters/fetch.js b/lib/adapters/fetch.js index 9b031015..1a2bfd5c 100644 --- a/lib/adapters/fetch.js +++ b/lib/adapters/fetch.js @@ -557,7 +557,17 @@ const factory = (env) => { const canceledError = composedSignal.reason; canceledError.config = config; request && (canceledError.request = request); - err !== canceledError && (canceledError.cause = err); + if (err !== canceledError) { + // Non-enumerable to match native Error `cause` semantics so loggers + // don't recurse into circular fetch internals (see #7205). + Object.defineProperty(canceledError, 'cause', { + __proto__: null, + value: err, + writable: true, + enumerable: false, + configurable: true, + }); + } throw canceledError; } @@ -579,18 +589,23 @@ const factory = (env) => { } if (err && err.name === 'TypeError' && /Load failed|fetch/i.test(err.message)) { - throw Object.assign( - new AxiosError( - 'Network Error', - AxiosError.ERR_NETWORK, - config, - request, - err && err.response - ), - { - cause: err.cause || err, - } + const networkError = new AxiosError( + 'Network Error', + AxiosError.ERR_NETWORK, + config, + request, + err && err.response ); + // Non-enumerable to match native Error `cause` semantics so loggers + // don't recurse into circular fetch internals (see #7205). + Object.defineProperty(networkError, 'cause', { + __proto__: null, + value: err.cause || err, + writable: true, + enumerable: false, + configurable: true, + }); + throw networkError; } throw AxiosError.from(err, err && err.code, config, request, err && err.response); diff --git a/lib/core/AxiosError.js b/lib/core/AxiosError.js index d4924854..c61eda0e 100644 --- a/lib/core/AxiosError.js +++ b/lib/core/AxiosError.js @@ -75,7 +75,19 @@ function redactConfig(config, redactKeys) { class AxiosError extends Error { static from(error, code, config, request, response, customProps) { const axiosError = new AxiosError(error.message, code || error.code, config, request, response); - axiosError.cause = error; + // Match native `Error` `cause` semantics: non-enumerable. The wrapped + // error often carries circular internals (sockets, requests, agents), so + // an enumerable `cause` makes structured loggers (pino/winston) and any + // own-property walk throw "Converting circular structure to JSON". + // Regression from #6982; see #7205. `__proto__: null` mirrors the + // `message` descriptor below (prototype-pollution-safe descriptor). + Object.defineProperty(axiosError, 'cause', { + __proto__: null, + value: error, + writable: true, + enumerable: false, + configurable: true, + }); axiosError.name = error.name; // Preserve status from the original error if not already set from response diff --git a/tests/unit/adapters/fetch.test.js b/tests/unit/adapters/fetch.test.js index 53a9845f..557a7bbb 100644 --- a/tests/unit/adapters/fetch.test.js +++ b/tests/unit/adapters/fetch.test.js @@ -844,6 +844,45 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => ); }); + it('sets a non-enumerable cause on canceled fetch errors so loggers do not throw (#7205)', async () => { + const underlying = new Error('abort internals'); + const socket = { name: 'Socket' }; + socket.self = socket; + underlying.socket = socket; + + const abortingFetch = (url, init) => { + const signal = getFetchSignal(url, init); + + return new Promise((_resolve, reject) => { + const onAbort = () => { + signal.removeEventListener('abort', onAbort); + reject(underlying); + }; + + if (signal.aborted) return onAbort(); + signal.addEventListener('abort', onAbort); + }); + }; + + const controller = new AbortController(); + + const request = fetchAxios.get('/', { + signal: controller.signal, + env: { fetch: abortingFetch }, + }); + + controller.abort(); + + const err = await request.catch((e) => e); + + assert.strictEqual(err.name, 'CanceledError'); + assert.strictEqual(err.code, 'ERR_CANCELED'); + assert.strictEqual(err.cause, underlying); + assert.strictEqual(Object.getOwnPropertyDescriptor(err, 'cause').enumerable, false); + assert.ok(!Object.keys(err).includes('cause')); + assert.doesNotThrow(() => JSON.stringify(Object.fromEntries(Object.entries(err)))); + }); + // Timing-sensitive: a 50ms abort race observed by a fake fetch can flake // under CI runner load even though the production code is fine. Retry as // a backstop. @@ -945,9 +984,32 @@ describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => } catch (err) { assert.strictEqual(String(err), 'AxiosError: Network Error'); assert.strictEqual(err.cause && err.cause.code, 'ENOTFOUND'); + // `cause` must be non-enumerable so own-property serialization is safe (#7205). + assert.strictEqual(Object.getOwnPropertyDescriptor(err, 'cause').enumerable, false); } }); + it('sets a non-enumerable cause on network errors so loggers do not throw (#7205)', async () => { + // Underlying error carrying a circular reference, like a Node socket. + const underlying = new Error('connect ECONNREFUSED'); + underlying.code = 'ECONNREFUSED'; + const socket = { name: 'Socket' }; + socket.self = socket; // circular + underlying.socket = socket; + + const failingFetch = () => + Promise.reject(Object.assign(new TypeError('fetch failed'), { cause: underlying })); + + const err = await fetchAxios.get('/', { env: { fetch: failingFetch } }).catch((e) => e); + + assert.strictEqual(err.code, 'ERR_NETWORK'); + assert.strictEqual(err.cause, underlying); // still accessible for debugging + assert.strictEqual(Object.getOwnPropertyDescriptor(err, 'cause').enumerable, false); + assert.ok(!Object.keys(err).includes('cause')); + // pino/winston-style own-property walk must not throw on the circular cause. + assert.doesNotThrow(() => JSON.stringify(Object.fromEntries(Object.entries(err)))); + }); + it('should get response headers', async () => { const server = await startHTTPServer( (req, res) => { diff --git a/tests/unit/core/AxiosError.test.js b/tests/unit/core/AxiosError.test.js index 63de026d..53380dbf 100644 --- a/tests/unit/core/AxiosError.test.js +++ b/tests/unit/core/AxiosError.test.js @@ -76,6 +76,55 @@ describe('core::AxiosError', () => { }); }); + describe('cause serialization (regression #7205)', () => { + // A wrapped low-level error carrying a circular reference, like a Node + // socket/request held by network errors. + const makeCircularCause = () => { + const cause = new Error('socket hang up'); + cause.code = 'ECONNRESET'; + const socket = { name: 'Socket' }; + socket.self = socket; // circular + cause.socket = socket; + return cause; + }; + + it('sets `cause` as a non-enumerable own property (native Error parity)', () => { + const axiosError = AxiosError.from(new Error('boom'), 'ERR_NETWORK', { url: '/x' }); + const descriptor = Object.getOwnPropertyDescriptor(axiosError, 'cause'); + + expect(descriptor).toBeDefined(); + expect(descriptor.enumerable).toBe(false); + expect(Object.keys(axiosError)).not.toContain('cause'); + expect('cause' in axiosError).toBe(true); // still discoverable via `in` + }); + + it('keeps `cause` fully accessible for debugging', () => { + const original = makeCircularCause(); + const axiosError = AxiosError.from(original, 'ERR_NETWORK', { url: '/x' }); + + expect(axiosError.cause).toBe(original); + expect(axiosError.cause.code).toBe('ECONNRESET'); + }); + + it('does not break structured loggers / own-property serialization', () => { + const axiosError = AxiosError.from(makeCircularCause(), 'ERR_NETWORK', { url: '/x' }); + + // pino/winston-style: enumerate own enumerable props and serialize. + const loggerWalk = () => + JSON.stringify(Object.fromEntries(Object.entries(axiosError))); + + expect(loggerWalk).not.toThrow(); + expect(() => JSON.stringify(axiosError)).not.toThrow(); + expect(() => JSON.stringify({ wrapped: axiosError })).not.toThrow(); + }); + + it('omits `cause` from toJSON() output', () => { + const axiosError = AxiosError.from(makeCircularCause(), 'ERR_NETWORK', { url: '/x' }); + + expect(axiosError.toJSON()).not.toHaveProperty('cause'); + }); + }); + it('is recognized as a native error by Node util/types', () => { expect(isNativeError(new AxiosError('My Axios Error'))).toBe(true); });