Skip to content

Commit 7acce0f

Browse files
Lms24c298lee
Lms24
authored and
c298lee
committed
feat(core): Deprecate session APIs on hub and add global replacements (#10054)
Deprecates and adds top-level replacements for * `hub.startSession` -> `Sentry.startSession` * `hub.endSession` -> `Sentry.endSession` * `hub.captureException` -> `Sentry.captureSession` These APIs were mostly used in browser and node `startSessionTracking` functions which were called on `Sentry.init`. I updated these usages with the global replacements and it seems integration tests are happy with it. For now, we not only put the session onto the isolation scope but also onto the current scope to avoid changing event processing and stuff like the ANR worker, as well as avoid "soft-breaking" users who use `hub.(get|set)Session`.
1 parent 4fa15d6 commit 7acce0f

File tree

16 files changed

+408
-94
lines changed

16 files changed

+408
-94
lines changed

‎MIGRATION.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,12 @@ Sentry.init({
5252

5353
In v8, the Hub class will be removed. The following methods are therefore deprecated:
5454

55-
- `hub.shouldSendDefaultPii()`: Access Sentry client option via `Sentry.getClient().getOptions().sendDefaultPii` instead
55+
* `hub.startTransaction()`: See [Deprecation of `startTransaction`](#deprecate-starttransaction)
56+
* `hub.lastEventId()`: See [Deprecation of `lastEventId`](#deprecate-sentrylasteventid-and-hublasteventid)
57+
* `hub.startSession()`: Use top-level `Sentry.startSession()` instead
58+
* `hub.endSession()`: Use top-level `Sentry.endSession()` instead
59+
* `hub.captureSession()`: Use top-level `Sentry.captureSession()` instead
60+
* `hub.shouldSendDefaultPii()`: Access Sentry client option via `Sentry.getClient().getOptions().sendDefaultPii` instead
5661

5762
## Deprecated fields on `Span` and `Transaction`
5863

‎dev-packages/browser-integration-tests/suites/sessions/v7-hub-start-session/init.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://[email protected]/1337',
7+
release: '0.1',
8+
// intentionally disabling this, we want to leverage the deprecated hub API
9+
autoSessionTracking: false,
10+
});
11+
12+
// simulate old startSessionTracking behavior
13+
Sentry.getCurrentHub().startSession({ ignoreDuration: true });
14+
Sentry.getCurrentHub().captureSession();

‎dev-packages/browser-integration-tests/suites/sessions/v7-hub-start-session/template.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<a id='navigate' href="foo">Navigate</button>
8+
</body>
9+
</html>

‎dev-packages/browser-integration-tests/suites/sessions/v7-hub-start-session/test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import type { Route } from '@playwright/test';
2+
import { expect } from '@playwright/test';
3+
import type { SessionContext } from '@sentry/types';
4+
5+
import { sentryTest } from '../../../utils/fixtures';
6+
import { getFirstSentryEnvelopeRequest } from '../../../utils/helpers';
7+
8+
sentryTest('should start a new session on pageload.', async ({ getLocalTestPath, page }) => {
9+
const url = await getLocalTestPath({ testDir: __dirname });
10+
const session = await getFirstSentryEnvelopeRequest<SessionContext>(page, url);
11+
12+
expect(session).toBeDefined();
13+
expect(session.init).toBe(true);
14+
expect(session.errors).toBe(0);
15+
expect(session.status).toBe('ok');
16+
});
17+
18+
sentryTest('should start a new session with navigation.', async ({ getLocalTestPath, page, browserName }) => {
19+
// Navigations get CORS error on Firefox and WebKit as we're using `file://` protocol.
20+
if (browserName !== 'chromium') {
21+
sentryTest.skip();
22+
}
23+
24+
const url = await getLocalTestPath({ testDir: __dirname });
25+
await page.route('**/foo', (route: Route) => route.fulfill({ path: `${__dirname}/dist/index.html` }));
26+
27+
const initSession = await getFirstSentryEnvelopeRequest<SessionContext>(page, url);
28+
29+
await page.click('#navigate');
30+
31+
const newSession = await getFirstSentryEnvelopeRequest<SessionContext>(page, url);
32+
33+
expect(newSession).toBeDefined();
34+
expect(newSession.init).toBe(true);
35+
expect(newSession.errors).toBe(0);
36+
expect(newSession.status).toBe('ok');
37+
expect(newSession.sid).toBeDefined();
38+
expect(initSession.sid).not.toBe(newSession.sid);
39+
});

‎packages/browser/src/sdk.ts

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import type { Hub } from '@sentry/core';
22
import {
33
Integrations as CoreIntegrations,
4+
captureSession,
45
getClient,
56
getCurrentHub,
67
getIntegrationsToSetup,
78
getReportDialogEndpoint,
89
initAndBind,
10+
startSession,
911
} from '@sentry/core';
1012
import type { UserFeedback } from '@sentry/types';
1113
import {
@@ -250,11 +252,6 @@ export function wrap(fn: (...args: any) => any): any {
250252
return internalWrap(fn)();
251253
}
252254

253-
function startSessionOnHub(hub: Hub): void {
254-
hub.startSession({ ignoreDuration: true });
255-
hub.captureSession();
256-
}
257-
258255
/**
259256
* Enable automatic Session Tracking for the initial page load.
260257
*/
@@ -264,29 +261,19 @@ function startSessionTracking(): void {
264261
return;
265262
}
266263

267-
const hub = getCurrentHub();
268-
269-
// The only way for this to be false is for there to be a version mismatch between @sentry/browser (>= 6.0.0) and
270-
// @sentry/hub (< 5.27.0). In the simple case, there won't ever be such a mismatch, because the two packages are
271-
// pinned at the same version in package.json, but there are edge cases where it's possible. See
272-
// https://github.com/getsentry/sentry-javascript/issues/3207 and
273-
// https://github.com/getsentry/sentry-javascript/issues/3234 and
274-
// https://github.com/getsentry/sentry-javascript/issues/3278.
275-
if (!hub.captureSession) {
276-
return;
277-
}
278-
279264
// The session duration for browser sessions does not track a meaningful
280265
// concept that can be used as a metric.
281266
// Automatically captured sessions are akin to page views, and thus we
282267
// discard their duration.
283-
startSessionOnHub(hub);
268+
startSession({ ignoreDuration: true });
269+
captureSession();
284270

285271
// We want to create a session for every navigation as well
286272
addHistoryInstrumentationHandler(({ from, to }) => {
287273
// Don't create an additional session for the initial route or if the location did not change
288274
if (from !== undefined && from !== to) {
289-
startSessionOnHub(getCurrentHub());
275+
startSession({ ignoreDuration: true });
276+
captureSession();
290277
}
291278
});
292279
}

‎packages/core/src/exports.ts

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,21 @@ import type {
1212
FinishedCheckIn,
1313
MonitorConfig,
1414
Primitive,
15+
Session,
16+
SessionContext,
1517
Severity,
1618
SeverityLevel,
1719
TransactionContext,
1820
User,
1921
} from '@sentry/types';
20-
import { isThenable, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
22+
import { GLOBAL_OBJ, isThenable, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
2123

24+
import { DEFAULT_ENVIRONMENT } from './constants';
2225
import { DEBUG_BUILD } from './debug-build';
2326
import type { Hub } from './hub';
24-
import { getCurrentHub } from './hub';
27+
import { getCurrentHub, getIsolationScope } from './hub';
2528
import type { Scope } from './scope';
29+
import { closeSession, makeSession, updateSession } from './session';
2630
import type { ExclusiveEventHintOrCaptureContext } from './utils/prepareEvent';
2731
import { parseEventHintOrCaptureContext } from './utils/prepareEvent';
2832

@@ -322,3 +326,99 @@ export function getClient<C extends Client>(): C | undefined {
322326
export function getCurrentScope(): Scope {
323327
return getCurrentHub().getScope();
324328
}
329+
330+
/**
331+
* Start a session on the current isolation scope.
332+
*
333+
* @param context (optional) additional properties to be applied to the returned session object
334+
*
335+
* @returns the new active session
336+
*/
337+
export function startSession(context?: SessionContext): Session {
338+
const client = getClient();
339+
const isolationScope = getIsolationScope();
340+
const currentScope = getCurrentScope();
341+
342+
const { release, environment = DEFAULT_ENVIRONMENT } = (client && client.getOptions()) || {};
343+
344+
// Will fetch userAgent if called from browser sdk
345+
const { userAgent } = GLOBAL_OBJ.navigator || {};
346+
347+
const session = makeSession({
348+
release,
349+
environment,
350+
user: isolationScope.getUser(),
351+
...(userAgent && { userAgent }),
352+
...context,
353+
});
354+
355+
// End existing session if there's one
356+
const currentSession = isolationScope.getSession();
357+
if (currentSession && currentSession.status === 'ok') {
358+
updateSession(currentSession, { status: 'exited' });
359+
}
360+
361+
endSession();
362+
363+
// Afterwards we set the new session on the scope
364+
isolationScope.setSession(session);
365+
366+
// TODO (v8): Remove this and only use the isolation scope(?).
367+
// For v7 though, we can't "soft-break" people using getCurrentHub().getScope().setSession()
368+
currentScope.setSession(session);
369+
370+
return session;
371+
}
372+
373+
/**
374+
* End the session on the current isolation scope.
375+
*/
376+
export function endSession(): void {
377+
const isolationScope = getIsolationScope();
378+
const currentScope = getCurrentScope();
379+
380+
const session = isolationScope.getSession();
381+
if (session) {
382+
closeSession(session);
383+
}
384+
_sendSessionUpdate();
385+
386+
// the session is over; take it off of the scope
387+
isolationScope.setSession();
388+
389+
// TODO (v8): Remove this and only use the isolation scope(?).
390+
// For v7 though, we can't "soft-break" people using getCurrentHub().getScope().setSession()
391+
currentScope.setSession();
392+
}
393+
394+
/**
395+
* Sends the current Session on the scope
396+
*/
397+
function _sendSessionUpdate(): void {
398+
const isolationScope = getIsolationScope();
399+
const currentScope = getCurrentScope();
400+
const client = getClient();
401+
// TODO (v8): Remove currentScope and only use the isolation scope(?).
402+
// For v7 though, we can't "soft-break" people using getCurrentHub().getScope().setSession()
403+
const session = currentScope.getSession() || isolationScope.getSession();
404+
if (session && client && client.captureSession) {
405+
client.captureSession(session);
406+
}
407+
}
408+
409+
/**
410+
* Sends the current session on the scope to Sentry
411+
*
412+
* @param end If set the session will be marked as exited and removed from the scope.
413+
* Defaults to `false`.
414+
*/
415+
export function captureSession(end: boolean = false): void {
416+
// both send the update and pull the session from the scope
417+
if (end) {
418+
endSession();
419+
return;
420+
}
421+
422+
// only send the update
423+
_sendSessionUpdate();
424+
}

‎packages/core/src/hub.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,10 +487,13 @@ Sentry.init({...});
487487

488488
/**
489489
* @inheritDoc
490+
*
491+
* @deprecated Use top level `captureSession` instead.
490492
*/
491493
public captureSession(endSession: boolean = false): void {
492494
// both send the update and pull the session from the scope
493495
if (endSession) {
496+
// eslint-disable-next-line deprecation/deprecation
494497
return this.endSession();
495498
}
496499

@@ -500,6 +503,7 @@ Sentry.init({...});
500503

501504
/**
502505
* @inheritDoc
506+
* @deprecated Use top level `endSession` instead.
503507
*/
504508
public endSession(): void {
505509
const layer = this.getStackTop();
@@ -516,6 +520,7 @@ Sentry.init({...});
516520

517521
/**
518522
* @inheritDoc
523+
* @deprecated Use top level `startSession` instead.
519524
*/
520525
public startSession(context?: SessionContext): Session {
521526
const { scope, client } = this.getStackTop();
@@ -537,6 +542,7 @@ Sentry.init({...});
537542
if (currentSession && currentSession.status === 'ok') {
538543
updateSession(currentSession, { status: 'exited' });
539544
}
545+
// eslint-disable-next-line deprecation/deprecation
540546
this.endSession();
541547

542548
// Afterwards we set the new session on the scope

‎packages/core/src/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ export {
3030
withScope,
3131
getClient,
3232
getCurrentScope,
33+
startSession,
34+
endSession,
35+
captureSession,
3336
} from './exports';
3437
export {
3538
getCurrentHub,

‎packages/core/src/session.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import type { SerializedSession, Session, SessionContext, SessionStatus } from '@sentry/types';
22
import { dropUndefinedKeys, timestampInSeconds, uuid4 } from '@sentry/utils';
3-
43
/**
54
* Creates a new `Session` object by setting certain default parameters. If optional @param context
65
* is passed, the passed properties are applied to the session object.

0 commit comments

Comments
 (0)