From e4c2c574f77470131abe0e0c222dff28c4cb2dfe Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:06:48 +0200 Subject: [PATCH 1/3] fix(core): Do not count dropped errors in request sessions --- .../filtered-errors-session-aggregate/test.ts | 33 ++++++++++++++ .../suites/sessions/instrument.mjs | 2 + .../suites/sessions/server.mjs | 9 ++++ packages/core/src/client.ts | 16 +++++-- .../http/record-request-session.ts | 36 +++++++++++++--- packages/core/src/scope.ts | 2 + packages/core/src/server-runtime-client.ts | 43 ++++++++++++------- .../test/lib/server-runtime-client.test.ts | 38 ++++++++++++++-- 8 files changed, 152 insertions(+), 27 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/sessions/filtered-errors-session-aggregate/test.ts diff --git a/dev-packages/node-integration-tests/suites/sessions/filtered-errors-session-aggregate/test.ts b/dev-packages/node-integration-tests/suites/sessions/filtered-errors-session-aggregate/test.ts new file mode 100644 index 000000000000..31afbac41a7f --- /dev/null +++ b/dev-packages/node-integration-tests/suites/sessions/filtered-errors-session-aggregate/test.ts @@ -0,0 +1,33 @@ +import { afterAll, expect, test } from 'vitest'; +import { join } from 'path'; +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('does not count errors dropped by ignoreErrors or beforeSend', async () => { + const runner = createRunner(__dirname, '..', 'server.mjs') + .withInstrument(join(__dirname, '..', 'instrument.mjs')) + .ignore('span', 'event') + .unignore('sessions') + .expect({ + sessions: agg => { + // Sessions are bucketed by minute; tolerate splits across a minute boundary by summing. + const totals = agg.aggregates.reduce( + (acc, b) => ({ + exited: acc.exited + (b.exited ?? 0), + errored: acc.errored + (b.errored ?? 0), + crashed: acc.crashed + (b.crashed ?? 0), + }), + { exited: 0, errored: 0, crashed: 0 }, + ); + expect(totals).toEqual({ exited: 2, errored: 0, crashed: 0 }); + }, + }) + .start(); + + runner.makeRequest('get', '/test/error_ignored', { expectError: true }); + runner.makeRequest('get', '/test/error_dropped'); + await runner.completed(); +}); diff --git a/dev-packages/node-integration-tests/suites/sessions/instrument.mjs b/dev-packages/node-integration-tests/suites/sessions/instrument.mjs index e727f2c5068a..3ce114b92fd3 100644 --- a/dev-packages/node-integration-tests/suites/sessions/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/sessions/instrument.mjs @@ -5,6 +5,8 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', transport: loggingTransport, + ignoreErrors: ['Ignored!'], + beforeSend: event => (event.exception?.values?.[0]?.value === 'Dropped!' ? null : event), integrations: [ Sentry.httpIntegration({ // Flush after 2 seconds (to avoid waiting for the default 60s) diff --git a/dev-packages/node-integration-tests/suites/sessions/server.mjs b/dev-packages/node-integration-tests/suites/sessions/server.mjs index f2f8eb5bc05e..00823d7d4fe2 100644 --- a/dev-packages/node-integration-tests/suites/sessions/server.mjs +++ b/dev-packages/node-integration-tests/suites/sessions/server.mjs @@ -32,4 +32,13 @@ app.get('/test/error_handled', (_req, res) => { res.send('Crash!'); }); +app.get('/test/error_ignored', () => { + throw new Error('Ignored!'); +}); + +app.get('/test/error_dropped', (_req, res) => { + Sentry.captureException(new Error('Dropped!')); + res.send('Dropped!'); +}); + startExpressServerAndSendPortToRunner(app); diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index c61b3975d41b..dacb66099e52 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -1362,6 +1362,17 @@ export abstract class Client { afterSetupIntegrations(this, integrations); } + /** + * Updates the sessions an error event counts towards. Runs after event processors and `beforeSend` kept the + * event, but before `sampleRate` is applied: dropped errors must not affect sessions, sampled-out errors must. + */ + protected _updateSessionsFromEvent(event: Event, currentScope: Scope, isolationScope: Scope): void { + const session = currentScope.getSession() || isolationScope.getSession(); + if (session) { + this._updateSessionFromEvent(session, event); + } + } + /** Updates existing session based on the provided event */ protected _updateSessionFromEvent(session: Session, event: Event): void { // initially, set `unhandled` based on the event level and update from exceptions if there are any later on @@ -1602,9 +1613,8 @@ export abstract class Client { throw _makeDoNotSendEventError(`${dropMessage}, will not send event.`); } - const session = currentScope.getSession() || isolationScope.getSession(); - if (isError && session) { - this._updateSessionFromEvent(session, processedEvent); + if (isError) { + this._updateSessionsFromEvent(processedEvent, currentScope, isolationScope); } if (isError && typeof parsedSampleRate === 'number' && safeMathRandom() > parsedSampleRate) { diff --git a/packages/core/src/integrations/http/record-request-session.ts b/packages/core/src/integrations/http/record-request-session.ts index fa608118a5a2..a7017dfe68d7 100644 --- a/packages/core/src/integrations/http/record-request-session.ts +++ b/packages/core/src/integrations/http/record-request-session.ts @@ -65,11 +65,7 @@ export function startRequestSession( client.emit('startRequestSession'); let ended = false; - return () => { - if (ended) { - return; - } - ended = true; + const record = (flushNow: boolean): void => { const requestSession = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession; if (client && requestSession) { @@ -115,7 +111,37 @@ export function startRequestSession( flushPendingClientAggregates(); }, sessionFlushingDelayMS); safeUnref(timeout); + + if (flushNow) { + flushPendingClientAggregates(); + } } } }; + + return () => { + if (ended) { + return; + } + ended = true; + + // The event pipeline decides whether an error counts towards the session, and it can still run after the + // response ended (e.g. async event processors), so the status is final only once those errors settle. + const pendingErrors = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession?.pendingErrors; + if (!pendingErrors?.size) { + record(false); + return; + } + + // `client.flush()` sends the aggregate before it waits for event processing. A session that settles during + // that wait is sent right away, so that the transport still has it when that flush drains it. + let flushedWhileSettling = false; + const unregisterClientFlushHook = client.on('flush', () => { + flushedWhileSettling = true; + }); + void Promise.allSettled(pendingErrors).then(() => { + unregisterClientFlushHook(); + record(flushedWhileSettling); + }); + }; } diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 1a2a1761854a..78af12729907 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -57,6 +57,8 @@ export interface SdkProcessingMetadata { [key: string]: unknown; requestSession?: { status: 'ok' | 'errored' | 'crashed'; + /** Error events of the request that are still in the event pipeline. */ + pendingErrors?: Set>; }; normalizedRequest?: RequestEventData; dynamicSamplingContext?: Partial; diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index 159c4aeb3b93..0fb289e32274 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -1,6 +1,5 @@ import { createCheckInEnvelope } from './checkin'; import { Client } from './client'; -import { getIsolationScope } from './currentScopes'; import { DEBUG_BUILD } from './debug-build'; import { spanStreamingIntegration, @@ -87,22 +86,33 @@ export class ServerRuntimeClient< /** * @inheritDoc */ - public captureException(exception: unknown, hint?: EventHint, scope?: Scope): string { - setCurrentRequestSessionErroredOrCrashed(hint); - return super.captureException(exception, hint, scope); + protected override _processEvent( + event: Event, + hint: EventHint, + currentScope: Scope, + isolationScope: Scope, + ): PromiseLike { + const processedEvent = super._processEvent(event, hint, currentScope, isolationScope); + + const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession; + if (requestSession && !event.type) { + const pendingErrors = (requestSession.pendingErrors ||= new Set()); + const settle = (): void => { + pendingErrors.delete(processedEvent); + }; + pendingErrors.add(processedEvent); + processedEvent.then(settle, settle); + } + + return processedEvent; } /** * @inheritDoc */ - public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string { - // If the event is of type Exception, then a request session should be captured - const isException = !event.type && event.exception?.values && event.exception.values.length > 0; - if (isException) { - setCurrentRequestSessionErroredOrCrashed(hint); - } - - return super.captureEvent(event, hint, scope); + protected override _updateSessionsFromEvent(event: Event, currentScope: Scope, isolationScope: Scope): void { + super._updateSessionsFromEvent(event, currentScope, isolationScope); + setRequestSessionErroredOrCrashed(isolationScope, event); } /** @@ -254,13 +264,14 @@ export class ServerRuntimeClient< } } -function setCurrentRequestSessionErroredOrCrashed(eventHint?: EventHint): void { - const requestSession = getIsolationScope().getScopeData().sdkProcessingMetadata.requestSession; - if (requestSession) { +function setRequestSessionErroredOrCrashed(isolationScope: Scope, event: Event): void { + const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession; + const exceptions = event.exception?.values; + if (requestSession && exceptions?.length) { // We mutate instead of doing `setSdkProcessingMetadata` because the http integration stores away a particular // isolationScope. If that isolation scope is forked, setting the processing metadata here will not mutate the // original isolation scope that the http integration stored away. - const isHandledException = eventHint?.mechanism?.handled ?? true; + const isHandledException = !exceptions.some(exception => exception.mechanism?.handled === false); // A request session can go from "errored" -> "crashed" but not "crashed" -> "errored". // Crashed (unhandled exception) is worse than errored (handled exception). if (isHandledException && requestSession.status !== 'crashed') { diff --git a/packages/core/test/lib/server-runtime-client.test.ts b/packages/core/test/lib/server-runtime-client.test.ts index 12ae4d625503..df7554d8b31f 100644 --- a/packages/core/test/lib/server-runtime-client.test.ts +++ b/packages/core/test/lib/server-runtime-client.test.ts @@ -1,9 +1,11 @@ -import { describe, expect, it, test, vi } from 'vitest'; -import { applySdkMetadata, createTransport, Scope } from '../../src'; +import { beforeEach, describe, expect, it, test, vi } from 'vitest'; +import { applySdkMetadata, createTransport, getIsolationScope, Scope } from '../../src'; +import { startRequestSession } from '../../src/integrations/http/record-request-session'; import { _INTERNAL_captureMetric, _INTERNAL_getMetricBuffer } from '../../src/metrics/internal'; import type { ServerRuntimeClientOptions } from '../../src/server-runtime-client'; import { ServerRuntimeClient } from '../../src/server-runtime-client'; -import type { Event, EventHint } from '../../src/types/event'; +import type { ErrorEvent, Event, EventHint } from '../../src/types/event'; +import { resetGlobals } from '../testutils'; const PUBLIC_DSN = 'https://username@domain/123'; @@ -183,6 +185,36 @@ describe('ServerRuntimeClient', () => { }); }); + describe('request sessions', () => { + beforeEach(() => { + resetGlobals(); + }); + + it.each([ + ['errored for a handled exception', {}, undefined, { errored: 1 }], + ['crashed for an unhandled exception', {}, { mechanism: { type: 'test', handled: false } }, { crashed: 1 }], + ['exited for an exception that `beforeSend` drops', { beforeSend: () => null }, undefined, { exited: 1 }], + ['errored for an exception that `sampleRate` drops', { sampleRate: 0 }, undefined, { errored: 1 }], + [ + 'errored for an exception that is processed after the request ended', + { beforeSend: (event: ErrorEvent) => new Promise(resolve => setTimeout(() => resolve(event), 10)) }, + undefined, + { errored: 1 }, + ], + ] as const)('records the session as %s', async (_, options, hint, counts) => { + client = new ServerRuntimeClient(getDefaultClientOptions({ dsn: PUBLIC_DSN, ...options })); + const sendSessionSpy = vi.spyOn(client, 'sendSession'); + const endRequestSession = startRequestSession(client, { requestIsolationScope: getIsolationScope() }); + client.captureException(new Error('foo'), hint); + endRequestSession(); + await client.flush(); + + expect(sendSessionSpy).toHaveBeenCalledExactlyOnceWith({ + aggregates: [{ started: expect.any(String), exited: 0, errored: 0, crashed: 0, ...counts }], + }); + }); + }); + describe('captureMessage', () => { it('sends a message event with level info', () => { const options = getDefaultClientOptions({ dsn: PUBLIC_DSN }); From a27f147caa7efadb45a4abb476a092ce97055ae1 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:32:08 +0200 Subject: [PATCH 2/3] test(bun): Exclude filtered errors session suite on Bun The scenario sets `sessionFlushingDelayMS` on `httpIntegration`, which has no effect with `@sentry/bun`, so the session aggregate arrives after the test timeout. --- dev-packages/bun-integration-tests/node-suites/excludes.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-packages/bun-integration-tests/node-suites/excludes.ts b/dev-packages/bun-integration-tests/node-suites/excludes.ts index 65edcd311712..b71c830dcaa3 100644 --- a/dev-packages/bun-integration-tests/node-suites/excludes.ts +++ b/dev-packages/bun-integration-tests/node-suites/excludes.ts @@ -166,6 +166,7 @@ const FETCH_INTEGRATION_DIFFERS = [ // that sets `emit` back to the `emit` it had before its first request gets no spans after that. const HTTP_SERVER_OPTIONS_IGNORED = [ 'suites/sessions/exited-session-aggregate/test.ts', + 'suites/sessions/filtered-errors-session-aggregate/test.ts', 'suites/sessions/server-without-process-session/test.ts', 'suites/tracing/httpIntegration/test.ts', ]; From d60f4440996c227b5f6c4431a97cee924c8c91f8 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:25:33 +0200 Subject: [PATCH 3/3] fix(core): Send request sessions that settle during a flush A session that settled while `client.flush()` waited for event processing was only sent when it opened a new aggregate. If another request opened an aggregate during that wait, the session joined it and that flush did not send it. Add an `afterFlushProcessing` client hook, which `flush()` emits after event processing and before the transport flush. Request session aggregates now also send on this hook. --- packages/core/src/client.ts | 12 +++++++++ .../http/record-request-session.ts | 25 +++++++------------ .../test/lib/server-runtime-client.test.ts | 21 ++++++++++++++++ 3 files changed, 42 insertions(+), 16 deletions(-) diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index dacb66099e52..e1fba71c0343 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -487,6 +487,7 @@ export abstract class Client { } const clientFinished = await this._isClientDoneProcessing(timeout); + this.emit('afterFlushProcessing'); const transportFlushed = await transport.flush(timeout); return clientFinished && transportFlushed; @@ -912,6 +913,12 @@ export abstract class Client { */ public on(hook: 'flush', callback: () => void): () => void; + /** + * A hook that is called during a flush, after the client is done processing events and before the transport is flushed. + * @returns {() => void} A function that, when executed, removes the registered callback. + */ + public on(hook: 'afterFlushProcessing', callback: () => void): () => void; + /** * A hook that is called when spans of a single trace should be flushed eagerly, * ahead of the trace's regular flush point. Only runtimes with a span streaming @@ -1227,6 +1234,11 @@ export abstract class Client { */ public emit(hook: 'flush'): void; + /** + * Emit a hook event during a flush, after the client is done processing events. + */ + public emit(hook: 'afterFlushProcessing'): void; + /** * Fire a hook event indicating that spans of a single trace should be flushed eagerly. */ diff --git a/packages/core/src/integrations/http/record-request-session.ts b/packages/core/src/integrations/http/record-request-session.ts index a7017dfe68d7..806ac6c9cef4 100644 --- a/packages/core/src/integrations/http/record-request-session.ts +++ b/packages/core/src/integrations/http/record-request-session.ts @@ -65,7 +65,7 @@ export function startRequestSession( client.emit('startRequestSession'); let ended = false; - const record = (flushNow: boolean): void => { + const record = (): void => { const requestSession = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession; if (client && requestSession) { @@ -89,6 +89,7 @@ export function startRequestSession( const flushPendingClientAggregates = (): void => { clearTimeout(timeout); unregisterClientFlushHook(); + unregisterAfterFlushProcessingHook(); clientToRequestSessionAggregatesMap.delete(client); const aggregatePayload: AggregationCounts[] = Object.entries(newClientAggregate).map( @@ -106,15 +107,16 @@ export function startRequestSession( DEBUG_BUILD && debug.log('Sending request session aggregate due to client flush'); flushPendingClientAggregates(); }); + // A session can settle while `client.flush()` waits for event processing, after `flush` was emitted. + const unregisterAfterFlushProcessingHook = client.on('afterFlushProcessing', () => { + DEBUG_BUILD && debug.log('Sending request session aggregate due to client flush'); + flushPendingClientAggregates(); + }); const timeout = setTimeout(() => { DEBUG_BUILD && debug.log('Sending request session aggregate due to flushing schedule'); flushPendingClientAggregates(); }, sessionFlushingDelayMS); safeUnref(timeout); - - if (flushNow) { - flushPendingClientAggregates(); - } } } }; @@ -129,19 +131,10 @@ export function startRequestSession( // response ended (e.g. async event processors), so the status is final only once those errors settle. const pendingErrors = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession?.pendingErrors; if (!pendingErrors?.size) { - record(false); + record(); return; } - // `client.flush()` sends the aggregate before it waits for event processing. A session that settles during - // that wait is sent right away, so that the transport still has it when that flush drains it. - let flushedWhileSettling = false; - const unregisterClientFlushHook = client.on('flush', () => { - flushedWhileSettling = true; - }); - void Promise.allSettled(pendingErrors).then(() => { - unregisterClientFlushHook(); - record(flushedWhileSettling); - }); + void Promise.allSettled(pendingErrors).then(record); }; } diff --git a/packages/core/test/lib/server-runtime-client.test.ts b/packages/core/test/lib/server-runtime-client.test.ts index df7554d8b31f..95d1da74c3fe 100644 --- a/packages/core/test/lib/server-runtime-client.test.ts +++ b/packages/core/test/lib/server-runtime-client.test.ts @@ -213,6 +213,27 @@ describe('ServerRuntimeClient', () => { aggregates: [{ started: expect.any(String), exited: 0, errored: 0, crashed: 0, ...counts }], }); }); + + it('sends a session that settles during a flush with an aggregate opened during that flush', async () => { + client = new ServerRuntimeClient( + getDefaultClientOptions({ + dsn: PUBLIC_DSN, + beforeSend: event => new Promise(resolve => setTimeout(() => resolve(event), 10)), + }), + ); + const sendSessionSpy = vi.spyOn(client, 'sendSession'); + const endErroredRequestSession = startRequestSession(client, { requestIsolationScope: getIsolationScope() }); + client.captureException(new Error('foo')); + endErroredRequestSession(); + + const flushed = client.flush(); + startRequestSession(client, { requestIsolationScope: new Scope() })(); + await flushed; + + expect(sendSessionSpy).toHaveBeenCalledExactlyOnceWith({ + aggregates: [{ started: expect.any(String), exited: 1, errored: 1, crashed: 0 }], + }); + }); }); describe('captureMessage', () => {