fix(core): Take user from current scope when starting a session (#10153)

Lms24 · web-flow · commit dd72e4d3c3cb · 2024-01-15T11:51:42.000+01:00
When creating a session, we previously only read the user from the
isolation scope. This was problematic because the user could be set in
`initialScope` which will be applied to the _current_ scope on SDK init.
This would happen before the session is started when using auto session
tracking.

This PR now takes the user from the current scope if it's set and falls
back to the isolation scope should the user be set there. Added an
integration test to cover this regression.
diff --git a/dev-packages/browser-integration-tests/suites/sessions/initial-scope/init.js b/dev-packages/browser-integration-tests/suites/sessions/initial-scope/init.js
@@ -0,0 +1,16 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  release: '0.1',
+  initialScope: {
+    user: {
+      id: '1337',
+      email: 'user@name.com',
+      username: 'user1337',
+    },
+  },
+  debug: true,
+});
diff --git a/dev-packages/browser-integration-tests/suites/sessions/initial-scope/template.html b/dev-packages/browser-integration-tests/suites/sessions/initial-scope/template.html
@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <a id='navigate' href="foo">Navigate</button>
+  </body>
+</html>
diff --git a/dev-packages/browser-integration-tests/suites/sessions/initial-scope/test.ts b/dev-packages/browser-integration-tests/suites/sessions/initial-scope/test.ts
@@ -0,0 +1,41 @@
+import type { Route } from '@playwright/test';
+import { expect } from '@playwright/test';
+import type { SessionContext } from '@sentry/types';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { getFirstSentryEnvelopeRequest } from '../../../utils/helpers';
+
+sentryTest('should start a new session on pageload.', async ({ getLocalTestPath, page }) => {
+  const url = await getLocalTestPath({ testDir: __dirname });
+  const session = await getFirstSentryEnvelopeRequest<SessionContext>(page, url);
+
+  expect(session).toBeDefined();
+  expect(session.init).toBe(true);
+  expect(session.errors).toBe(0);
+  expect(session.status).toBe('ok');
+  expect(session.did).toBe('1337');
+});
+
+sentryTest('should start a new session with navigation.', async ({ getLocalTestPath, page, browserName }) => {
+  // Navigations get CORS error on Firefox and WebKit as we're using `file://` protocol.
+  if (browserName !== 'chromium') {
+    sentryTest.skip();
+  }
+
+  const url = await getLocalTestPath({ testDir: __dirname });
+  await page.route('**/foo', (route: Route) => route.fulfill({ path: `${__dirname}/dist/index.html` }));
+
+  const initSession = await getFirstSentryEnvelopeRequest<SessionContext>(page, url);
+
+  await page.click('#navigate');
+
+  const newSession = await getFirstSentryEnvelopeRequest<SessionContext>(page, url);
+
+  expect(newSession).toBeDefined();
+  expect(newSession.init).toBe(true);
+  expect(newSession.errors).toBe(0);
+  expect(newSession.status).toBe('ok');
+  expect(newSession.sid).toBeDefined();
+  expect(initSession.sid).not.toBe(newSession.sid);
+  expect(newSession.did).toBe('1337');
+});
diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts
@@ -404,7 +404,7 @@ export function startSession(context?: SessionContext): Session {
   const session = makeSession({
     release,
     environment,
-    user: isolationScope.getUser(),
+    user: currentScope.getUser() || isolationScope.getUser(),
     ...(userAgent && { userAgent }),
     ...context,
   });
@@ -434,7 +434,7 @@ export function endSession(): void {
   const isolationScope = getIsolationScope();
   const currentScope = getCurrentScope();
 
-  const session = isolationScope.getSession();
+  const session = currentScope.getSession() || isolationScope.getSession();
   if (session) {
     closeSession(session);
   }
