Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,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',
];
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
28 changes: 25 additions & 3 deletions packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
}

const clientFinished = await this._isClientDoneProcessing(timeout);
this.emit('afterFlushProcessing');
const transportFlushed = await transport.flush(timeout);

return clientFinished && transportFlushed;
Expand Down Expand Up @@ -912,6 +913,12 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
*/
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
Expand Down Expand Up @@ -1227,6 +1234,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
*/
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.
*/
Expand Down Expand Up @@ -1362,6 +1374,17 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
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
Expand Down Expand Up @@ -1602,9 +1625,8 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
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) {
Expand Down
29 changes: 24 additions & 5 deletions packages/core/src/integrations/http/record-request-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,7 @@ export function startRequestSession(
client.emit('startRequestSession');

let ended = false;
return () => {
if (ended) {
return;
}
ended = true;
const record = (): void => {
const requestSession = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession;

if (client && requestSession) {
Expand All @@ -93,6 +89,7 @@ export function startRequestSession(
const flushPendingClientAggregates = (): void => {
clearTimeout(timeout);
unregisterClientFlushHook();
unregisterAfterFlushProcessingHook();
clientToRequestSessionAggregatesMap.delete(client);

const aggregatePayload: AggregationCounts[] = Object.entries(newClientAggregate).map(
Expand All @@ -110,6 +107,11 @@ 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();
Expand All @@ -118,4 +120,21 @@ export function startRequestSession(
}
}
};

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();
return;
}

void Promise.allSettled(pendingErrors).then(record);
};
}
2 changes: 2 additions & 0 deletions packages/core/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<PromiseLike<unknown>>;
};
normalizedRequest?: RequestEventData;
dynamicSamplingContext?: Partial<DynamicSamplingContext>;
Expand Down
43 changes: 27 additions & 16 deletions packages/core/src/server-runtime-client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { createCheckInEnvelope } from './checkin';
import { Client } from './client';
import { getIsolationScope } from './currentScopes';
import { DEBUG_BUILD } from './debug-build';
import {
spanStreamingIntegration,
Expand Down Expand Up @@ -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<Event> {
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);
}

/**
Expand Down Expand Up @@ -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') {
Expand Down
59 changes: 56 additions & 3 deletions packages/core/test/lib/server-runtime-client.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -183,6 +185,57 @@ 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<ErrorEvent>(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 }],
});
});

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<ErrorEvent>(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', () => {
it('sends a message event with level info', () => {
const options = getDefaultClientOptions({ dsn: PUBLIC_DSN });
Expand Down
Loading