diff --git a/packages/core/src/integrations/http/client-subscriptions.ts b/packages/core/src/integrations/http/client-subscriptions.ts index 554445eb8053..574141e8a260 100644 --- a/packages/core/src/integrations/http/client-subscriptions.ts +++ b/packages/core/src/integrations/http/client-subscriptions.ts @@ -184,9 +184,8 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): if (breadcrumbs) { addBreadcrumbs(request, response); } - const aborted = 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,6 +195,9 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions): response.on('end', () => finishWithResponse()); response.on(errorMonitor, finishWithResponse); + // `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 74cf76467586..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'; @@ -126,6 +127,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 +143,14 @@ describe('getHttpClientSubscriptions', () => { function makeRequest( path: string, - abortAt: 'beforeResponse' | 'afterResponse' | 'destroyWithError', + abortAt: + | 'beforeResponse' + | 'afterResponse' + | 'destroyWithError' + | 'destroyResponse' + | 'destroyResponseWithError' + | 'never', + { listenForResponseErrors = true }: { listenForResponseErrors?: boolean } = {}, ): Promise { const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 })); setCurrentClient(client); @@ -154,15 +166,22 @@ 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')); - } else { + } else if (abortAt === 'destroyResponse') { + response.destroy(); + } else if (abortAt === 'destroyResponseWithError') { + response.destroy(new Error('boom')); + } else if (abortAt !== 'never') { controller.abort(); } }); @@ -190,5 +209,28 @@ 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'); + }); + + 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'); + }); }); });