Skip to content

Commit 8b3ae38

Browse files
committed
fix(browser): Defer browser session envelope until browser is idle
1 parent b534db4 commit 8b3ae38

5 files changed

Lines changed: 46 additions & 11 deletions

File tree

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
Sentry.setUser({
2-
id: '1337',
3-
email: 'user@name.com',
4-
username: 'user1337',
1+
document.getElementById('set-user').addEventListener('click', () => {
2+
Sentry.setUser({
3+
id: '1337',
4+
email: 'user@name.com',
5+
username: 'user1337',
6+
});
57
});

dev-packages/browser-integration-tests/suites/sessions/user/template.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,6 @@
55
</head>
66
<body>
77
<a id="navigate" href="#foo">Navigate</a>
8+
<button id="set-user">Set user</button>
89
</body>
910
</html>

dev-packages/browser-integration-tests/suites/sessions/user/test.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@ import { expect } from '@playwright/test';
22
import { sentryTest } from '../../../utils/fixtures';
33
import { waitForSession } from '../../../utils/helpers';
44

5-
sentryTest('updates the session when setting the user', async ({ getLocalTestUrl, page }) => {
5+
sentryTest('updates the session when the user is set after the initial session', async ({ getLocalTestUrl, page }) => {
66
const initialSessionPromise = waitForSession(page, s => !!s.init && s.status === 'ok');
7-
const updatedSessionPromise = waitForSession(page, s => !s.init && s.status === 'ok');
87

98
const url = await getLocalTestUrl({ testDir: __dirname });
109
await page.goto(url);
1110

11+
// The initial session envelope is deferred (sent once the browser is idle). At this point
12+
// no user has been set yet, so it carries no `did`.
1213
const initialSession = await initialSessionPromise;
13-
const updatedSession = await updatedSessionPromise;
1414

1515
expect(initialSession).toEqual({
1616
attrs: {
@@ -26,6 +26,12 @@ sentryTest('updates the session when setting the user', async ({ getLocalTestUrl
2626
timestamp: expect.any(String),
2727
});
2828

29+
// Setting the user _after_ the initial session was sent must still be captured as a
30+
// dedicated session update carrying the `did`.
31+
const updatedSessionPromise = waitForSession(page, s => !s.init && s.status === 'ok');
32+
await page.locator('#set-user').click();
33+
const updatedSession = await updatedSessionPromise;
34+
2935
expect(updatedSession).toEqual({
3036
...initialSession,
3137
init: false,
@@ -42,6 +48,10 @@ sentryTest('includes the user id in the exited session', async ({ getLocalTestUr
4248

4349
const initialSession = await initialSessionPromise;
4450

51+
// Set the user after the initial session was sent, then navigate so the (now exited)
52+
// initial session carries the `did`.
53+
await page.locator('#set-user').click();
54+
4555
const exitedInitialSessionPromise = waitForSession(page, s => !s.init && s.status === 'exited');
4656

4757
await page.locator('#navigate').click();
@@ -79,6 +89,10 @@ sentryTest('includes the user id in the subsequent session', async ({ getLocalTe
7989
timestamp: expect.any(String),
8090
});
8191

92+
// Set the user after the initial session was sent, then navigate so the subsequent
93+
// session inherits the `did`.
94+
await page.locator('#set-user').click();
95+
8296
const secondSessionPromise = waitForSession(page, s => !!s.init && s.status === 'ok' && s.sid !== initialSession.sid);
8397

8498
await page.locator('#navigate').click();

packages/browser-utils/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ export { extractNetworkProtocol } from './metrics/utils';
2525

2626
export { trackClsAsSpan, trackInpAsSpan, trackLcpAsSpan } from './metrics/webVitalSpans';
2727

28+
export { whenIdleOrHidden } from './metrics/web-vitals/lib/whenIdleOrHidden';
29+
2830
export { addClickKeypressInstrumentationHandler } from './instrument/dom';
2931

3032
export { addHistoryInstrumentationHandler } from './instrument/history';

packages/browser/src/integrations/browsersession.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { captureSession, debug, defineIntegration, getIsolationScope, startSession } from '@sentry/core/browser';
2-
import { addHistoryInstrumentationHandler } from '@sentry/browser-utils';
2+
import { addHistoryInstrumentationHandler, whenIdleOrHidden } from '@sentry/browser-utils';
33
import { DEBUG_BUILD } from '../debug-build';
44
import { WINDOW } from '../helpers';
55

@@ -41,7 +41,17 @@ export const browserSessionIntegration = defineIntegration((options: BrowserSess
4141
// Automatically captured sessions are akin to page views, and thus we
4242
// discard their duration.
4343
startSession({ ignoreDuration: true });
44-
captureSession();
44+
45+
// Sending the session envelope synchronously in `init()` runs the full send
46+
// pipeline during page load, competing with critical resources for the network and
47+
// adding overhead that measurably hurts LCP. We defer the initial send until the
48+
// browser is idle; `whenIdleOrHidden` flushes it on page-hide so we don't lose short
49+
// (page-view-like) sessions.
50+
let initialSessionSent = false;
51+
whenIdleOrHidden(() => {
52+
captureSession();
53+
initialSessionSent = true;
54+
});
4555

4656
// User data can be set at any time, for example async after Sentry.init has run and the initial session
4757
// envelope was already sent, but still on the initial page.
@@ -58,9 +68,15 @@ export const browserSessionIntegration = defineIntegration((options: BrowserSess
5868
const maybeNewUser = scope.getUser();
5969
// sessions only care about user id and ip address, so we only need to capture the session if the user has changed
6070
if (previousUser?.id !== maybeNewUser?.id || previousUser?.ip_address !== maybeNewUser?.ip_address) {
61-
// the scope class already writes the user to its session, so we only need to capture the session here
62-
captureSession();
6371
previousUser = maybeNewUser;
72+
// Only emit a dedicated update envelope for user data that arrives _after_ the
73+
// deferred initial session was sent. User data set during page load is already
74+
// reflected in that session (the scope writes it onto the session), so capturing
75+
// here would send a redundant envelope - and do so during page load, which is
76+
// exactly the overhead we're deferring away from.
77+
if (initialSessionSent) {
78+
captureSession();
79+
}
6480
}
6581
});
6682

0 commit comments

Comments
 (0)