diff --git a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts index 4b9f9f82324c..9d1617bbac9e 100644 --- a/packages/node/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node/src/integrations/node-fetch/undici-instrumentation.ts @@ -414,9 +414,10 @@ function onDone({ request }: RequestTrailersMessage): void { // This is the event we get when something is wrong in the request like // - invalid options when calling `fetch` global API or any undici method for request // - connectivity errors such as unreachable host -// - requests aborted through an `AbortController.signal` +// - requests aborted through an `AbortController.signal`, or a response body cancelled by the caller // NOTE: server errors are considered valid responses and it's the lib consumer -// who should deal with that. +// who should deal with that. Aborts initiated by the caller are not errors either, so they end the +// span without an error status. function onError({ request, error }: RequestErrorMessage): void { const span = spanFromReq.get(request); @@ -424,6 +425,17 @@ function onError({ request, error }: RequestErrorMessage): void { return; } + // An intentional cancellation (`AbortController.abort()` or cancelling the response body) is not + // a failure of the request, so don't flip the span (which may already carry a 2xx status) to error. + const isAbort = + error.name === 'AbortError' || + (typeof DOMException !== 'undefined' && error instanceof DOMException && error.code === DOMException.ABORT_ERR); + if (isAbort) { + span.end(); + spanFromReq.delete(request); + return; + } + // NOTE: in `undici@6.3.0` when request aborted the error type changes from // a custom error (`RequestAbortedError`) to a built-in `DOMException` carrying // some differences: diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index 807b16ac5468..a387cec9b6f1 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -12,8 +12,10 @@ import { URL_SCHEME, SENTRY_ORIGIN, } from '@sentry/conventions/attributes'; +import { SPAN_STATUS_ERROR } from '@sentry/core'; import { channel } from 'node:diagnostics_channel'; import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Mock } from 'vitest'; import type { UndiciRequest } from '../../src/integrations/node-fetch/types'; const { span, startInactiveSpan, getClient } = vi.hoisted(() => ({ @@ -109,4 +111,51 @@ describe('instrumentUndici', () => { expect.objectContaining({ name: 'QUERY api.example.com', onlyIfParent: false }), ); }); + + describe('request errors', () => { + function startRequest(): { request: UndiciRequest; testSpan: { setStatus: Mock; setAttributes: Mock; end: Mock } } { + const testSpan = { setStatus: vi.fn(), setAttributes: vi.fn(), end: vi.fn() }; + startInactiveSpan.mockReturnValueOnce(testSpan); + const request = { + method: 'GET', + origin: 'https://api.example.com', + path: '/stream', + headers: [], + } as unknown as UndiciRequest; + channel('undici:request:create').publish({ request }); + channel('undici:request:headers').publish({ + request, + response: { statusCode: 200, statusText: 'OK', headers: [] }, + }); + return { request, testSpan }; + } + + it.each([ + ['an AbortError', () => Object.assign(new Error('aborted'), { name: 'AbortError' })], + ['a DOMException named AbortError', () => new DOMException('This operation was aborted', 'AbortError')], + [ + 'a DOMException with only the ABORT_ERR code', + () => Object.defineProperty(new DOMException('aborted', 'Other'), 'code', { value: DOMException.ABORT_ERR }), + ], + ])('ends the span without an error status when the request is aborted (%s)', (_name, makeError) => { + const { request, testSpan } = startRequest(); + + channel('undici:request:error').publish({ request, error: makeError() }); + + expect(testSpan.setStatus).not.toHaveBeenCalledWith(expect.objectContaining({ code: SPAN_STATUS_ERROR })); + expect(testSpan.end).toHaveBeenCalledTimes(1); + }); + + it.each([ + ['a TypeError', () => new TypeError('fetch failed')], + ['ECONNRESET', () => Object.assign(new Error('read ECONNRESET'), { code: 'ECONNRESET' })], + ])('still sets an error status for a non-abort error (%s)', (_name, makeError) => { + const { request, testSpan } = startRequest(); + + channel('undici:request:error').publish({ request, error: makeError() }); + + expect(testSpan.setStatus).toHaveBeenCalledWith({ code: SPAN_STATUS_ERROR, message: expect.any(String) }); + expect(testSpan.end).toHaveBeenCalledTimes(1); + }); + }); });