diff --git a/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/init.js b/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/init.js index 0144febaeb38..cebc5f71b52b 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/init.js +++ b/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/init.js @@ -2,14 +2,8 @@ import * as Sentry from '@sentry/browser'; window.Sentry = Sentry; -window._errorCount = 0; - Sentry.init({ traceLifecycle: 'static', dsn: 'https://public@dsn.ingest.sentry.io/1337', sampleRate: '0', - beforeSend() { - window._errorCount++; - return null; - }, }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/test.ts b/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/test.ts index 3b74fd7f7860..cd95d0098f74 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/test.ts @@ -1,15 +1,14 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; +import { countEnvelopes } from '../../../../utils/helpers'; -sentryTest('parses a string sample rate', async ({ getLocalTestUrl, page }) => { +sentryTest('drops error events when sampleRate is the string "0"', async ({ getLocalTestUrl, page }) => { const url = await getLocalTestUrl({ testDir: __dirname }); + const errorCountPromise = countEnvelopes(page, { envelopeType: 'event', timeout: 2000 }); await page.goto(url); - await page.waitForFunction('window._testDone'); await page.evaluate('window.Sentry.getClient().flush()'); - const count = await page.evaluate('window._errorCount'); - - expect(count).toStrictEqual(0); + expect(await errorCountPromise).toBe(0); }); diff --git a/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/init.js b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/init.js new file mode 100644 index 000000000000..f5c6bdef5e5f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + traceLifecycle: 'static', + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '0.1', + sampleRate: 0, +}); diff --git a/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/subject.js b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/subject.js new file mode 100644 index 000000000000..49160a6dd1fa --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/subject.js @@ -0,0 +1,7 @@ +document.getElementById('throw-error').addEventListener('click', () => { + throw new Error('unhandled crash'); +}); + +document.getElementById('capture-exception').addEventListener('click', () => { + Sentry.captureException(new Error('handled capture')); +}); diff --git a/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/template.html b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/template.html new file mode 100644 index 000000000000..d0cb96c31a41 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/template.html @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/test.ts b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/test.ts new file mode 100644 index 000000000000..1b1e63162dab --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/test.ts @@ -0,0 +1,53 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../utils/fixtures'; +import { countEnvelopes, waitForSession } from '../../../utils/helpers'; + +sentryTest( + 'marks session as unhandled when unhandled error is sampled out by sampleRate', + async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadSessionPromise = waitForSession(page, s => !!s.init && s.status === 'ok'); + await page.goto(url); + const pageloadSession = await pageloadSessionPromise; + + const updatedSessionPromise = waitForSession(page, s => !s.init && s.status !== 'ok'); + const errorCountPromise = countEnvelopes(page, { envelopeType: 'event', timeout: 2000 }); + await page.locator('#throw-error').click(); + const updatedSession = await updatedSessionPromise; + const errorCount = await errorCountPromise; + + // The error event is not sent — it was sampled out + expect(errorCount).toBe(0); + + // But the session update is still sent, reflecting the crash + expect(updatedSession.sid).toBe(pageloadSession.sid); + expect(updatedSession.errors).toBe(1); + expect(updatedSession.status).toBe('unhandled'); + }, +); + +sentryTest( + 'marks session as errored when handled exception is sampled out by sampleRate', + async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + const pageloadSessionPromise = waitForSession(page, s => !!s.init && s.status === 'ok'); + await page.goto(url); + const pageloadSession = await pageloadSessionPromise; + + const updatedSessionPromise = waitForSession(page, s => !s.init); + const errorCountPromise = countEnvelopes(page, { envelopeType: 'event', timeout: 2000 }); + await page.locator('#capture-exception').click(); + const updatedSession = await updatedSessionPromise; + const errorCount = await errorCountPromise; + + // The error event is not sent — it was sampled out + expect(errorCount).toBe(0); + + // But the session update is still sent, recording the error + expect(updatedSession.sid).toBe(pageloadSession.sid); + expect(updatedSession.errors).toBe(1); + expect(updatedSession.status).toBe('ok'); + }, +); diff --git a/packages/core/src/client.ts b/packages/core/src/client.ts index 2b3338bfe7f2..058c93cf67c0 100644 --- a/packages/core/src/client.ts +++ b/packages/core/src/client.ts @@ -52,7 +52,6 @@ import { makePromiseBuffer, type PromiseBuffer, SENTRY_BUFFER_FULL_ERROR } from import { safeMathRandom } from './utils/randomSafeContext'; import { reparentChildSpans, shouldIgnoreSpan } from './utils/should-ignore-span'; import { showSpanDropWarning } from './utils/spanUtils'; -import { rejectedSyncPromise } from './utils/syncpromise'; import { safeUnref } from './utils/timer'; import { convertSpanJsonToTransactionEvent, convertTransactionEventToSpanJson } from './utils/transactionEvent'; import { resolveDataCollectionOptions } from './utils/data-collection/resolveDataCollectionOptions'; @@ -1449,15 +1448,6 @@ export abstract class Client { // 0.0 === 0% events are sent // Sampling for transaction happens somewhere else const parsedSampleRate = typeof sampleRate === 'undefined' ? undefined : parseSampleRate(sampleRate); - if (isError && typeof parsedSampleRate === 'number' && safeMathRandom() > parsedSampleRate) { - this.recordDroppedEvent('sample_rate', 'error'); - return rejectedSyncPromise( - _makeDoNotSendEventError( - `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`, - ), - ); - } - const dataCategory = getDataCategoryByType(event.type); return this._prepareEvent(event, hint, currentScope, isolationScope) @@ -1492,6 +1482,13 @@ export abstract class Client { this._updateSessionFromEvent(session, processedEvent); } + if (isError && typeof parsedSampleRate === 'number' && safeMathRandom() > parsedSampleRate) { + this.recordDroppedEvent('sample_rate', 'error'); + throw _makeDoNotSendEventError( + `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`, + ); + } + if (isTransaction) { const spanCountBefore = processedEvent.sdkProcessingMetadata?.spanCountBeforeProcessing || 0; const spanCountAfter = processedEvent.spans ? processedEvent.spans.length : 0; diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index 95b0234434bf..e2322766287b 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -174,7 +174,7 @@ export function setConversationId(conversationId: string | null | undefined): vo * isolation scope. If you call this function after handling a certain error and another error * is captured in between, the last one is returned instead of the one you might expect. * Also, ids of events that were never sent to Sentry (for example because - * they were dropped in `beforeSend`) could be returned. + * they were dropped by sampling or `beforeSend`) could be returned. * * @returns The last event id of the isolation scope. */ diff --git a/packages/core/test/lib/client.test.ts b/packages/core/test/lib/client.test.ts index ddc992476533..5846b48b8185 100644 --- a/packages/core/test/lib/client.test.ts +++ b/packages/core/test/lib/client.test.ts @@ -321,6 +321,29 @@ describe('Client', () => { expect(eventId).toEqual(lastEventId()); }); + test('sets lastEventId when an error is sampled out', () => { + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, sampleRate: 0 }); + const client = new TestClient(options); + + const eventId = client.captureException(new Error('sampled-out exception')); + + expect(eventId).toEqual(lastEventId()); + expect(TestClient.instance!.event).toBeUndefined(); + }); + + test('(known limitation) replaces lastEventId with a sampled-out error ID', () => { + // After a successfully sent error, a subsequent sampled-out error replaces lastEventId() even though that new ID has no corresponding event in Sentry. + // The `setLastEventId` call in `_prepareEvent` now executes before the `sampleRate` check + const client = new TestClient(getDefaultTestClientOptions({ dsn: PUBLIC_DSN })); + + client.captureException(new Error('sent exception'), { event_id: 'sent-event-id' }); + client.getOptions().sampleRate = 0; + client.captureException(new Error('sampled-out exception'), { event_id: 'sampled-out-event-id' }); + + expect(TestClient.instance!.event?.event_id).toBe('sent-event-id'); + expect(lastEventId()).toBe('sampled-out-event-id'); + }); + test('allows for providing explicit scope', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); @@ -2574,6 +2597,215 @@ describe('Client', () => { }); }); + describe('session update filtering', () => { + describe('sampleRate drop updates session', () => { + test('marks session as crashed for sampled-out unhandled error', () => { + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, sampleRate: 0 }); + const client = new TestClient(options); + setCurrentClient(client); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'unhandled crash', mechanism: { type: 'generic', handled: false } }], + }, + }, + { mechanism: { handled: false } }, + ); + + expect(TestClient.instance!.event).toBeUndefined(); + expect(client.session?.errors).toBe(1); + expect(client.session?.status).toBe('crashed'); + }); + + test('marks session as errored for sampled-out handled error', () => { + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, sampleRate: 0 }); + const client = new TestClient(options); + setCurrentClient(client); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'handled capture', mechanism: { type: 'generic', handled: true } }], + }, + }, + {}, + ); + + expect(TestClient.instance!.event).toBeUndefined(); + expect(client.session?.errors).toBe(1); + expect(client.session?.status).toBe('ok'); + }); + }); + + describe('beforeSend drop does not update session', () => { + test('does not update session when beforeSend returns null for unhandled error', () => { + const beforeSend = vi.fn(() => null); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); + const client = new TestClient(options); + setCurrentClient(client); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'unhandled crash', mechanism: { type: 'generic', handled: false } }], + }, + }, + { mechanism: { handled: false } }, + ); + + expect(beforeSend).toHaveBeenCalledOnce(); + expect(TestClient.instance!.event).toBeUndefined(); + expect(client.session).toBeUndefined(); + expect(session.errors).toBe(0); + expect(session.status).toBe('ok'); + }); + + test('does not update session when beforeSend returns null for handled error', () => { + const beforeSend = vi.fn(() => null); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); + const client = new TestClient(options); + setCurrentClient(client); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'handled capture', mechanism: { type: 'generic', handled: true } }], + }, + }, + {}, + ); + + expect(beforeSend).toHaveBeenCalledOnce(); + expect(TestClient.instance!.event).toBeUndefined(); + expect(client.session).toBeUndefined(); + expect(session.errors).toBe(0); + expect(session.status).toBe('ok'); + }); + }); + + describe('event processor drop does not update session', () => { + test('does not update session when event processor returns null for unhandled error', () => { + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); + const client = new TestClient(options); + setCurrentClient(client); + + client.addEventProcessor(() => null); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'unhandled crash', mechanism: { type: 'generic', handled: false } }], + }, + }, + { mechanism: { handled: false } }, + ); + + expect(client.session).toBeUndefined(); + expect(session.errors).toBe(0); + expect(session.status).toBe('ok'); + }); + }); + + describe('error that passes through beforeSend updates session', () => { + test('updates session when beforeSend passes unhandled error through', () => { + const beforeSend = vi.fn(event => event); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend }); + const client = new TestClient(options); + setCurrentClient(client); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'unhandled crash', mechanism: { type: 'generic', handled: false } }], + }, + }, + { mechanism: { handled: false } }, + ); + + expect(beforeSend).toHaveBeenCalledOnce(); + expect(TestClient.instance!.event).toBeDefined(); + expect(client.session?.errors).toBe(1); + expect(client.session?.status).toBe('crashed'); + }); + }); + + describe('sampleRate runs after beforeSend', () => { + test('does not update session when beforeSend drops an error that would be sampled out', () => { + const beforeSend = vi.fn(() => null); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, sampleRate: 0, beforeSend }); + const client = new TestClient(options); + setCurrentClient(client); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'filtered crash', mechanism: { type: 'generic', handled: false } }], + }, + }, + { mechanism: { handled: false } }, + ); + + expect(beforeSend).toHaveBeenCalledOnce(); + expect(TestClient.instance!.event).toBeUndefined(); + expect(client.session).toBeUndefined(); + expect(session.errors).toBe(0); + expect(session.status).toBe('ok'); + }); + + test('uses the event returned by beforeSend to update a sampled-out session', () => { + const beforeSend = vi.fn((event: ErrorEvent) => { + const exception = event.exception?.values?.[0]; + if (exception) { + exception.mechanism = { type: 'generic', handled: true }; + } + return event; + }); + const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, sampleRate: 0, beforeSend }); + const client = new TestClient(options); + setCurrentClient(client); + + const session = makeSession(); + getCurrentScope().setSession(session); + + client.captureEvent( + { + exception: { + values: [{ type: 'Error', value: 'reclassified crash', mechanism: { type: 'generic', handled: false } }], + }, + }, + { mechanism: { handled: false } }, + ); + + expect(beforeSend).toHaveBeenCalledOnce(); + expect(TestClient.instance!.event).toBeUndefined(); + expect(client.session?.errors).toBe(1); + expect(client.session?.status).toBe('ok'); + }); + }); + }); + describe('recordDroppedEvent()/_clearOutcomes()', () => { test('records and returns outcomes', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });