From e6a234c7472bbceb121ecc0dd0eacd5f854532d0 Mon Sep 17 00:00:00 2001 From: 4ktluffy Date: Mon, 19 Oct 2026 10:57:12 +0300 Subject: [PATCH 1/2] fix(core): End node:http client spans when the response is destroyed --- .../integrations/http/client-subscriptions.ts | 8 +++-- .../http/client-subscriptions.test.ts | 35 +++++++++++++++++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/core/src/integrations/http/client-subscriptions.ts b/packages/core/src/integrations/http/client-subscriptions.ts index 554445eb8053..f2d696f263b0 100644 --- a/packages/core/src/integrations/http/client-subscriptions.ts +++ b/packages/core/src/integrations/http/client-subscriptions.ts @@ -175,7 +175,7 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): options.outgoingResponseHook?.(span, response); let finished = false; - function finishWithResponse(error?: unknown): void { + function finishWithResponse(error?: unknown, destroyedByCaller = false): void { if (!finished) { finished = true; if (error) { @@ -184,7 +184,7 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): if (breadcrumbs) { addBreadcrumbs(request, response); } - const aborted = response.aborted && !response.complete; + const aborted = !destroyedByCaller && response.aborted && !response.complete; const status: SpanStatus = error || typeof response.statusCode !== 'number' || aborted ? { code: SPAN_STATUS_ERROR } @@ -196,6 +196,10 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): response.on('end', () => finishWithResponse()); response.on(errorMonitor, finishWithResponse); + // A response destroyed by the caller without an error (e.g. `res.destroy()`) emits neither + // 'end' nor 'error', only 'close'. Like a caller abort, this is not a failure of the request, + // so end the span with the status of the response. After 'end' or an error this is a no-op. + response.on('close', () => finishWithResponse(undefined, true)); }); }; 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 74cf76467586..4dee157a3ac4 100644 --- a/packages/core/test/lib/integrations/http/client-subscriptions.test.ts +++ b/packages/core/test/lib/integrations/http/client-subscriptions.test.ts @@ -126,6 +126,10 @@ describe('getHttpClientSubscriptions', () => { 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'); + if (req.url === '/server-abort') { + // The server goes away before the body is complete. + setTimeout(() => res.socket?.destroy(), 10); + } }); await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)); port = (server.address() as AddressInfo).port; @@ -138,7 +142,13 @@ describe('getHttpClientSubscriptions', () => { function makeRequest( path: string, - abortAt: 'beforeResponse' | 'afterResponse' | 'destroyWithError', + abortAt: + | 'beforeResponse' + | 'afterResponse' + | 'destroyWithError' + | 'destroyResponse' + | 'destroyResponseWithError' + | 'never', ): Promise { const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 })); setCurrentClient(client); @@ -162,7 +172,11 @@ describe('getHttpClientSubscriptions', () => { response.once('data', () => { if (abortAt === 'destroyWithError') { request.destroy(new Error('boom')); - } else { + } else if (abortAt === 'destroyResponse') { + response.destroy(); + } else if (abortAt === 'destroyResponseWithError') { + response.destroy(new Error('boom')); + } else if (abortAt !== 'never') { controller.abort(); } }); @@ -190,5 +204,22 @@ describe('getHttpClientSubscriptions', () => { expect(span.status).toBe('error'); }); + + it('ends the span with the response status when the caller destroys the response mid-body', async () => { + expect((await makeRequest('/', 'destroyResponse')).status).toBe('ok'); + expect((await makeRequest('/not-found', 'destroyResponse')).status).toBe('error'); + }); + + it('ends the span with an error status when the response is destroyed with an error mid-body', async () => { + const span = await makeRequest('/', 'destroyResponseWithError'); + + expect(span.status).toBe('error'); + }); + + it('sets an error status when the server closes the connection mid-body', async () => { + const span = await makeRequest('/server-abort', 'never'); + + expect(span.status).toBe('error'); + }); }); }); From 2d88ecfd3bbceeb5fe8ef17069c87d20ac33cd88 Mon Sep 17 00:00:00 2001 From: 4ktluffy Date: Mon, 19 Oct 2026 17:09:37 +0300 Subject: [PATCH 2/2] fix(core): Use response.errored to tell a caller destroy from a connection reset --- .../src/integrations/http/client-subscriptions.ts | 12 +++++------- packages/core/src/integrations/http/types.ts | 1 + .../http/client-subscriptions.test.ts | 15 +++++++++++++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/packages/core/src/integrations/http/client-subscriptions.ts b/packages/core/src/integrations/http/client-subscriptions.ts index f2d696f263b0..574141e8a260 100644 --- a/packages/core/src/integrations/http/client-subscriptions.ts +++ b/packages/core/src/integrations/http/client-subscriptions.ts @@ -175,7 +175,7 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): options.outgoingResponseHook?.(span, response); let finished = false; - function finishWithResponse(error?: unknown, destroyedByCaller = false): void { + function finishWithResponse(error?: unknown): void { if (!finished) { finished = true; if (error) { @@ -184,9 +184,8 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): if (breadcrumbs) { addBreadcrumbs(request, response); } - const aborted = !destroyedByCaller && response.aborted && !response.complete; const status: SpanStatus = - error || typeof response.statusCode !== 'number' || aborted + error || typeof response.statusCode !== 'number' ? { code: SPAN_STATUS_ERROR } : getSpanStatusFromHttpCode(response.statusCode); options.applyCustomAttributesOnSpan?.(span, request, response); @@ -196,10 +195,9 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): response.on('end', () => finishWithResponse()); response.on(errorMonitor, finishWithResponse); - // A response destroyed by the caller without an error (e.g. `res.destroy()`) emits neither - // 'end' nor 'error', only 'close'. Like a caller abort, this is not a failure of the request, - // so end the span with the status of the response. After 'end' or an error this is a no-op. - response.on('close', () => finishWithResponse(undefined, true)); + // `res.destroy()` emits only `close`. `errored` separates a caller destroy from a connection + // error that was not emitted because nothing listens for `error`. + response.on('close', () => finishWithResponse(response.errored)); }); }; diff --git a/packages/core/src/integrations/http/types.ts b/packages/core/src/integrations/http/types.ts index 70c76fbf9256..7ec38a86fd4a 100644 --- a/packages/core/src/integrations/http/types.ts +++ b/packages/core/src/integrations/http/types.ts @@ -93,6 +93,7 @@ export interface HttpIncomingMessage { socket?: HttpSocket; aborted?: boolean; complete?: boolean; + errored?: Error | null; resume(): void; on(event: 'end', listener: () => void): this; on(event: string | symbol, listener: (...args: unknown[]) => void): this; 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 4dee157a3ac4..24962c79ae0d 100644 --- a/packages/core/test/lib/integrations/http/client-subscriptions.test.ts +++ b/packages/core/test/lib/integrations/http/client-subscriptions.test.ts @@ -1,4 +1,5 @@ import * as http from 'node:http'; +import { errorMonitor } from 'node:events'; 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'; @@ -149,6 +150,7 @@ describe('getHttpClientSubscriptions', () => { | 'destroyResponse' | 'destroyResponseWithError' | 'never', + { listenForResponseErrors = true }: { listenForResponseErrors?: boolean } = {}, ): Promise { const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 })); setCurrentClient(client); @@ -164,11 +166,14 @@ describe('getHttpClientSubscriptions', () => { 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); + // Like @sentry/node, pass the real `errorMonitor`, so Sentry itself does not add an `error` listener. + getHttpClientSubscriptions({ errorMonitor })[HTTP_ON_CLIENT_REQUEST]({ request }, HTTP_ON_CLIENT_REQUEST); }); request.on('error', () => {}); request.on('response', response => { - response.on('error', () => {}); + if (listenForResponseErrors) { + response.on('error', () => {}); + } response.once('data', () => { if (abortAt === 'destroyWithError') { request.destroy(new Error('boom')); @@ -221,5 +226,11 @@ describe('getHttpClientSubscriptions', () => { expect(span.status).toBe('error'); }); + + it('sets an error status when the connection resets mid-body and nothing listens for response errors', async () => { + const span = await makeRequest('/server-abort', 'never', { listenForResponseErrors: false }); + + expect(span.status).toBe('error'); + }); }); });