Skip to content

fix(core): Sample errors after beforeSend while preserving session updates #22671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
s1gr1d merged 8 commits into from
Jul 29, 2026
Merged

fix(core): Sample errors after beforeSend while preserving session updates #22671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
342bd89
fix(core): Update session status before event is dropped or `beforeSe…
s1gr1d Jul 27, 2026
b002e45
fix(core): Update session status before event is dropped or `beforeSe…
s1gr1d Jul 27, 2026
db044d0
events dropped via beforeSend do not update a session
s1gr1d Jul 27, 2026
a7cb866
remove unused variable
s1gr1d Jul 27, 2026
f711e80
fix(core): Sample error events after beforeSend
s1gr1d Jul 28, 2026
ae2228e
fix integration test
s1gr1d Jul 29, 2026
6cc6285
Merge branch 'develop' into sig/fix-session-error-rate
s1gr1d Jul 29, 2026
b653230
add test
s1gr1d Jul 29, 2026
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
6 changes: 0 additions & 6 deletions dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
});
9 changes: 4 additions & 5 deletions dev-packages/browser-integration-tests/suites/public-api/init/stringSampleRate/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
});
10 changes: 10 additions & 0 deletions dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
});
7 changes: 7 additions & 0 deletions dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/subject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'));
});
10 changes: 10 additions & 0 deletions dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8" />
</head>
<body>
<button id="throw-error">Throw Error</button>
<button id="capture-exception">Capture Exception</button>
</body>
</html>
53 changes: 53 additions & 0 deletions dev-packages/browser-integration-tests/suites/sessions/sampled-session-update/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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');
},
);
17 changes: 7 additions & 10 deletions packages/core/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -1449,15 +1448,6 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
// 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)
Expand Down Expand Up @@ -1492,6 +1482,13 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
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})`,
);
}
Comment on lines +1488 to +1490

@sentry sentry Bot Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Sentry.lastEventId() is now set for events that are dropped by sampleRate, breaking user feedback integrations like showReportDialog() which rely on a sent event.
Severity: MEDIUM

Suggested Fix

Move the setLastEventId call to after the sampleRate check within the _prepareEvent function. This ensures Sentry.lastEventId() is only set for events that are actually sent, restoring the expected behavior for features that rely on it.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/core/src/client.ts#L1483-L1485

Potential issue: The `setLastEventId` call in `_prepareEvent` now executes before the
`sampleRate` check. Consequently, `Sentry.lastEventId()` stores the ID for all error
events, including those that are subsequently dropped due to sampling and never sent to
Sentry. This behavior breaks integrations like `showReportDialog()`, which depend on
`lastEventId()` to reference an event that was successfully transmitted in order to
associate user feedback with it.

@s1gr1d s1gr1d Jul 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a known limitation. I made it more obvious: b653230

Comment on lines +1487 to +1490

@sentry sentry Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The sampleRate check is now performed after event processing, causing unnecessary work for events that are ultimately sampled out and discarded.
Severity: MEDIUM

Suggested Fix

This change was intentional to support session tracking. A potential improvement could be a two-phase processing system: handle session-relevant data, make the sampling decision, and only then execute the full, expensive processing pipeline for events that will be sent, if the spec allows.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/core/src/client.ts#L1487-L1490

Potential issue: The order of operations for event capturing has been changed to `event
processors → beforeSend → session update → sampleRate → send`. This means that for any
event that is ultimately dropped due to sampling, the SDK still performs all the
processing work upfront. This includes merging scope data, running integrations,
executing potentially expensive user-defined `event processors` and `beforeSend`
callbacks, and data normalization. This introduces a performance regression,
particularly impactful for applications with low `sampleRate` values, where the majority
of events are processed only to be discarded.


if (isTransaction) {
const spanCountBefore = processedEvent.sdkProcessingMetadata?.spanCountBeforeProcessing || 0;
const spanCountAfter = processedEvent.spans ? processedEvent.spans.length : 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/exports.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Loading
Loading