From c5e8a18ce92c6101aac2ac36ae142b494575c18e Mon Sep 17 00:00:00 2001 From: Andrei Borza Date: Mon, 12 Oct 2026 11:11:33 +0200 Subject: [PATCH] fix(core): Don't mark http client spans as errors when the caller aborts Co-Authored-By: Claude Opus 5.5 --- .../integrations/http/client-subscriptions.ts | 16 +++- .../http/client-subscriptions.test.ts | 84 ++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/packages/core/src/integrations/http/client-subscriptions.ts b/packages/core/src/integrations/http/client-subscriptions.ts index e91da7a55ad0..554445eb8053 100644 --- a/packages/core/src/integrations/http/client-subscriptions.ts +++ b/packages/core/src/integrations/http/client-subscriptions.ts @@ -25,6 +25,7 @@ import { } from '../../tracing'; import { startInactiveSpan } from '../../tracing/trace'; import { debug } from '../../utils/debug-logger'; +import { isError } from '../../utils/is'; import { LRUMap } from '../../utils/lru'; import { getOutgoingRequestSpanData, setIncomingResponseSpanData } from './get-outgoing-span-data'; import { getRequestUrlFromClientRequest } from './get-request-url'; @@ -141,18 +142,31 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): const requestOnClose = () => endSpan({ code: SPAN_STATUS_UNSET }); request.on('close', requestOnClose); + let responseStatusCode: number | undefined; + request.on(errorMonitor, error => { DEBUG_BUILD && debug.log(LOG_PREFIX, 'outgoingRequest on request error()', error); if (breadcrumbs) { addBreadcrumbs(request, undefined); } - endSpan({ code: SPAN_STATUS_ERROR }); + // An abort by the caller (e.g. `AbortController.abort()`) is not a failure of the request, + // so keep the status of the response if one arrived already. + if (isError(error) && error.name === 'AbortError') { + endSpan( + typeof responseStatusCode === 'number' + ? getSpanStatusFromHttpCode(responseStatusCode) + : { code: SPAN_STATUS_UNSET }, + ); + } else { + endSpan({ code: SPAN_STATUS_ERROR }); + } }); request.prependListener('response', response => { // no longer need this, listen on response now. // do not end the span until the response finishes request.removeListener('close', requestOnClose); + responseStatusCode = response.statusCode; if (request.listenerCount('response') <= 1) { response.resume(); } diff --git a/packages/core/test/lib/integrations/http/client-subscriptions.test.ts b/packages/core/test/lib/integrations/http/client-subscriptions.test.ts index 62428d171605..74cf76467586 100644 --- a/packages/core/test/lib/integrations/http/client-subscriptions.test.ts +++ b/packages/core/test/lib/integrations/http/client-subscriptions.test.ts @@ -1,10 +1,17 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; +import * as http from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import * as breadcrumbModule from '../../../../src/integrations/http/add-outgoing-request-breadcrumb'; import { HTTP_ON_CLIENT_REQUEST } from '../../../../src/integrations/http/constants'; import { getHttpClientSubscriptions } from '../../../../src/integrations/http/client-subscriptions'; import type { HttpClientRequest, HttpIncomingMessage } from '../../../../src/integrations/http/types'; import { SUPPRESS_TRACING_KEY } from '../../../../src/tracing/constants'; import { getCurrentScope, withScope } from '../../../../src/currentScopes'; +import { setCurrentClient } from '../../../../src/sdk'; +import { startSpan } from '../../../../src/tracing/trace'; +import { spanToJSON } from '../../../../src/utils/spanUtils'; +import type { SpanJSON } from '../../../../src/types/span'; +import { getDefaultTestClientOptions, TestClient } from '../../../mocks/client'; function makeMockRequest(): HttpClientRequest & { _responseListeners: ((res: HttpIncomingMessage) => void)[]; @@ -109,4 +116,79 @@ describe('getHttpClientSubscriptions', () => { expect(spy).toHaveBeenCalledOnce(); }); }); + + describe('request errors', () => { + let server: http.Server; + let port: number; + + beforeAll(async () => { + server = http.createServer((req, res) => { + res.writeHead(req.url === '/not-found' ? 404 : 200); + // Keep the body open so that the request can be aborted after the response arrived. + res.write('x'); + }); + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); + port = (server.address() as AddressInfo).port; + }); + + afterAll(() => { + server.closeAllConnections(); + server.close(); + }); + + function makeRequest( + path: string, + abortAt: 'beforeResponse' | 'afterResponse' | 'destroyWithError', + ): Promise { + const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 })); + setCurrentClient(client); + client.init(); + + return new Promise(resolve => { + client.on('spanEnd', span => { + if (!spanToJSON(span).is_segment) { + resolve(spanToJSON(span)); + } + }); + + const controller = new AbortController(); + const request = http.request({ host: '127.0.0.1', port, path, signal: controller.signal }); + startSpan({ name: 'parent' }, () => { + getHttpClientSubscriptions({})[HTTP_ON_CLIENT_REQUEST]({ request }, HTTP_ON_CLIENT_REQUEST); + }); + request.on('error', () => {}); + request.on('response', response => { + response.on('error', () => {}); + response.once('data', () => { + if (abortAt === 'destroyWithError') { + request.destroy(new Error('boom')); + } else { + controller.abort(); + } + }); + }); + if (abortAt === 'beforeResponse') { + controller.abort(); + } + request.end(); + }); + } + + it('does not set an error status when the request is aborted before the response', async () => { + const span = await makeRequest('/', 'beforeResponse'); + + expect(span.status).toBe('ok'); + }); + + it('keeps the response status when the request is aborted after the response', async () => { + expect((await makeRequest('/', 'afterResponse')).status).toBe('ok'); + expect((await makeRequest('/not-found', 'afterResponse')).status).toBe('error'); + }); + + it('sets an error status when the request fails for another reason', async () => { + const span = await makeRequest('/', 'destroyWithError'); + + expect(span.status).toBe('error'); + }); + }); });