diff --git a/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts b/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts index 2a364ad31727..83d4a1cd7fdc 100644 --- a/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts +++ b/packages/node-core/src/integrations/node-fetch/undici-instrumentation.ts @@ -397,9 +397,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); @@ -407,6 +408,19 @@ 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' || + // `DOMException.code` is deprecated, but undici (>= 6.3.0) reports aborts as a `DOMException` with `ABORT_ERR` + // eslint-disable-next-line typescript/no-deprecated + (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-core/test/integrations/undici.test.ts b/packages/node-core/test/integrations/undici.test.ts new file mode 100644 index 000000000000..82c733a03e65 --- /dev/null +++ b/packages/node-core/test/integrations/undici.test.ts @@ -0,0 +1,84 @@ +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(() => ({ + span: {}, + startInactiveSpan: vi.fn(), + getClient: vi.fn(), +})); + +vi.mock('@sentry/core', async () => { + const actual = (await vi.importActual('@sentry/core')) as Record; + return { + ...actual, + getClient, + startInactiveSpan: startInactiveSpan.mockReturnValue(span), + }; +}); + +vi.mock('../../src/utils/outgoingFetchRequest', () => ({ + addFetchRequestBreadcrumb: vi.fn(), + addTracePropagationHeadersToFetchRequest: vi.fn(), +})); + +describe('instrumentUndici', () => { + beforeAll(async () => { + const { instrumentUndici } = await import('../../src/integrations/node-fetch/undici-instrumentation'); + instrumentUndici({ spans: true }); + }); + + beforeEach(() => { + startInactiveSpan.mockClear(); + getClient.mockReturnValue(undefined); + }); + + 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); + }); + }); +});