Merge pull request #8208 from getsentry/prepare-release/7.53.1

Author: Luca Forstner

CHANGELOG.md

@@ -2,9 +2,16 @@
 
 ## Unreleased
 
-
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+## 7.53.1
+
+- chore(deps): bump socket.io-parser from 4.2.1 to 4.2.3 (#8196)
+- chore(svelte): Bump magic-string to 0.30.0 (#8197)
+- fix(core): Fix racecondition that modifies in-flight sessions (#8203)
+- fix(node): Catch `os.uptime()` throwing because of EPERM (#8206)
+- fix(replay): Fix buffered replays creating replay w/o error occuring (#8168)
+
 ## 7.53.0
 
 - feat(replay): Add `beforeAddRecordingEvent` Replay option (#8124)

packages/core/src/envelope.ts

@@ -48,7 +48,7 @@ export function createSessionEnvelope(
   };
 
   const envelopeItem: SessionItem =
-    'aggregates' in session ? [{ type: 'sessions' }, session] : [{ type: 'session' }, session];
+    'aggregates' in session ? [{ type: 'sessions' }, session] : [{ type: 'session' }, session.toJSON()];
 
   return createEnvelope<SessionEnvelope>(envelopeHeaders, [envelopeItem]);
 }

packages/node/src/integrations/context.ts

@@ -195,10 +195,17 @@ function getAppContext(): AppContext {
 export function getDeviceContext(deviceOpt: DeviceContextOptions | true): DeviceContext {
   const device: DeviceContext = {};
 
+  // Sometimes os.uptime() throws due to lacking permissions: https://github.com/getsentry/sentry-javascript/issues/8202
+  let uptime;
+  try {
+    uptime = os.uptime && os.uptime();
+  } catch (e) {
+    // noop
+  }
+
   // os.uptime or its return value seem to be undefined in certain environments (e.g. Azure functions).
   // Hence, we only set boot time, if we get a valid uptime value.
   // @see https://github.com/getsentry/sentry-javascript/issues/5856
-  const uptime = os.uptime && os.uptime();
   if (typeof uptime === 'number') {
     device.boot_time = new Date(Date.now() - uptime * 1000).toISOString();
   }

packages/replay/jest.setup.ts

@@ -128,6 +128,24 @@ function checkCallForSentReplay(
   };
 }
 
+/**
+* Only want calls that send replay events, i.e. ignore error events
+*/
+function getReplayCalls(calls: any[][][]): any[][][] {
+  return calls.map(call => {
+    const arg = call[0];
+      if (arg.length !== 2) {
+        return [];
+      }
+
+      if (!arg[1][0].find(({type}: {type: string}) => ['replay_event', 'replay_recording'].includes(type))) {
+        return [];
+      }
+
+      return [ arg ];
+  }).filter(Boolean);
+}
+
 /**
  * Checks all calls to `fetch` and ensures a replay was uploaded by
  * checking the `fetch()` request's body.
@@ -143,7 +161,9 @@ const toHaveSentReplay = function (
 
   const expectedKeysLength = expected ? ('sample' in expected ? Object.keys(expected.sample) : Object.keys(expected)).length : 0;
 
-  for (const currentCall of calls) {
+  const replayCalls = getReplayCalls(calls)
+
+  for (const currentCall of replayCalls) {
     result = checkCallForSentReplay.call(this, currentCall[0], expected);
     if (result.pass) {
       break;
@@ -193,7 +213,9 @@ const toHaveLastSentReplay = function (
   expected?: SentReplayExpected | { sample: SentReplayExpected; inverse: boolean },
 ) {
   const { calls } = (getCurrentHub().getClient()?.getTransport()?.send as MockTransport).mock;
-  const lastCall = calls[calls.length - 1]?.[0];
+  const replayCalls = getReplayCalls(calls)
+
+  const lastCall = replayCalls[calls.length - 1]?.[0];
 
   const { results, call, pass } = checkCallForSentReplay.call(this, lastCall, expected);
 

packages/replay/src/replay.ts

@@ -327,7 +327,9 @@ export class ReplayContainer implements ReplayContainerInterface {
       this._debouncedFlush.cancel();
       // See comment above re: `_isEnabled`, we "force" a flush, ignoring the
       // `_isEnabled` state of the plugin since it was disabled above.
-      await this._flush({ force: true });
+      if (this.recordingMode === 'session') {
+        await this._flush({ force: true });
+      }
 
       // After flush, destroy event buffer
       this.eventBuffer && this.eventBuffer.destroy();

packages/replay/test/integration/errorSampleRate-delayFlush.test.ts

@@ -229,6 +229,21 @@ describe('Integration | errorSampleRate with delayed flush', () => {
     });
   });
 
+  // This tests a regression where we were calling flush indiscriminantly in `stop()`
+  it('does not upload a replay event if error is not sampled', async () => {
+    // We are trying to replicate the case where error rate is 0 and session
+    // rate is > 0, we can't set them both to 0 otherwise
+    // `_loadAndCheckSession` is not called when initializing the plugin.
+    replay.stop();
+    replay['_options']['errorSampleRate'] = 0;
+    replay['_loadAndCheckSession']();
+
+    jest.runAllTimers();
+    await new Promise(process.nextTick);
+    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
+    expect(replay).not.toHaveLastSentReplay();
+  });
+
   it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_EXPIRE_DURATION]ms', async () => {
     Object.defineProperty(document, 'visibilityState', {
       configurable: true,
@@ -664,7 +679,7 @@ describe('Integration | errorSampleRate with delayed flush', () => {
     jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
     await new Promise(process.nextTick);
 
-    expect(replay).toHaveLastSentReplay();
+    expect(replay).not.toHaveLastSentReplay();
 
     // Wait a bit, shortly before session expires
     jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000);

packages/replay/test/integration/errorSampleRate.test.ts

@@ -234,6 +234,21 @@ describe('Integration | errorSampleRate', () => {
     });
   });
 
+  // This tests a regression where we were calling flush indiscriminantly in `stop()`
+  it('does not upload a replay event if error is not sampled', async () => {
+    // We are trying to replicate the case where error rate is 0 and session
+    // rate is > 0, we can't set them both to 0 otherwise
+    // `_loadAndCheckSession` is not called when initializing the plugin.
+    replay.stop();
+    replay['_options']['errorSampleRate'] = 0;
+    replay['_loadAndCheckSession']();
+
+    jest.runAllTimers();
+    await new Promise(process.nextTick);
+    expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
+    expect(replay).not.toHaveLastSentReplay();
+  });
+
   it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_EXPIRE_DURATION]ms', async () => {
     Object.defineProperty(document, 'visibilityState', {
       configurable: true,
@@ -668,8 +683,7 @@ describe('Integration | errorSampleRate', () => {
 
     jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
     await new Promise(process.nextTick);
-
-    expect(replay).toHaveLastSentReplay();
+    expect(replay).not.toHaveLastSentReplay();
 
     // Wait a bit, shortly before session expires
     jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000);

packages/svelte/package.json

@@ -19,7 +19,7 @@
     "@sentry/browser": "7.53.0",
     "@sentry/types": "7.53.0",
     "@sentry/utils": "7.53.0",
-    "magic-string": "^0.26.2",
+    "magic-string": "^0.30.0",
     "tslib": "^1.9.3"
   },
   "peerDependencies": {

packages/sveltekit/package.json

@@ -28,6 +28,7 @@
     "@sentry/types": "7.53.0",
     "@sentry/utils": "7.53.0",
     "@sentry/vite-plugin": "^0.6.0",
+    "magicast": "0.2.6",
     "sorcery": "0.11.0"
   },
   "devDependencies": {

packages/sveltekit/src/vite/autoInstrument.ts

@@ -1,5 +1,7 @@
 /* eslint-disable @sentry-internal/sdk/no-optional-chaining */
+import type { ExportNamedDeclaration } from '@babel/types';
 import * as fs from 'fs';
+import { parseModule } from 'magicast';
 import * as path from 'path';
 import type { Plugin } from 'vite';
 
@@ -89,24 +91,50 @@ export function makeAutoInstrumentationPlugin(options: AutoInstrumentPluginOptio
  */
 export async function canWrapLoad(id: string, debug: boolean): Promise<boolean> {
   const code = (await fs.promises.readFile(id, 'utf8')).toString();
+  const mod = parseModule(code);
 
-  const codeWithoutComments = code.replace(/(\/\/.*| ?\/\*[^]*?\*\/)(,?)$/gm, '');
-
-  const hasSentryContent = codeWithoutComments.includes('@sentry/sveltekit');
-  if (hasSentryContent) {
+  const program = mod.$ast.type === 'Program' && mod.$ast;
+  if (!program) {
     // eslint-disable-next-line no-console
-    debug && console.log(`Skipping wrapping ${id} because it already contains Sentry code`);
+    debug && console.log(`Skipping wrapping ${id} because it doesn't contain valid JavaScript or TypeScript`);
+    return false;
   }
 
-  const hasLoadDeclaration = /((const|let|var|function)\s+load\s*(=|\(|:))|as\s+load\s*(,|})/gm.test(
-    codeWithoutComments,
-  );
+  const hasLoadDeclaration = program.body
+    .filter((statement): statement is ExportNamedDeclaration => statement.type === 'ExportNamedDeclaration')
+    .find(exportDecl => {
+      // find `export const load = ...`
+      if (exportDecl.declaration && exportDecl.declaration.type === 'VariableDeclaration') {
+        const variableDeclarations = exportDecl.declaration.declarations;
+        return variableDeclarations.find(decl => decl.id.type === 'Identifier' && decl.id.name === 'load');
+      }
+
+      // find `export function load = ...`
+      if (exportDecl.declaration && exportDecl.declaration.type === 'FunctionDeclaration') {
+        const functionId = exportDecl.declaration.id;
+        return functionId?.name === 'load';
+      }
+
+      // find `export { load, somethingElse as load, somethingElse as "load" }`
+      if (exportDecl.specifiers) {
+        return exportDecl.specifiers.find(specifier => {
+          return (
+            (specifier.exported.type === 'Identifier' && specifier.exported.name === 'load') ||
+            (specifier.exported.type === 'StringLiteral' && specifier.exported.value === 'load')
+          );
+        });
+      }
+
+      return false;
+    });
+
   if (!hasLoadDeclaration) {
     // eslint-disable-next-line no-console
     debug && console.log(`Skipping wrapping ${id} because it doesn't declare a \`load\` function`);
+    return false;
   }
 
-  return !hasSentryContent && hasLoadDeclaration;
+  return true;
 }
 
 /**

packages/sveltekit/test/vite/autoInstrument.test.ts

@@ -121,6 +121,7 @@ describe('canWrapLoad', () => {
     ['export variable declaration - function pointer', 'export const load=   loadPageData'],
     ['export variable declaration - factory function call', 'export const load    =loadPageData()'],
     ['export variable declaration - inline function', 'export const load = () => { return { props: { msg: "hi" } } }'],
+    ['export variable declaration - inline function let', 'export let load = () => {}'],
     [
       'export variable declaration - inline async function',
       'export const load = async () => { return { props: { msg: "hi" } } }',
@@ -139,14 +140,14 @@ describe('canWrapLoad', () => {
       'variable declaration (let)',
       `import {something} from 'somewhere';
       let load = async () => {};
-      export prerender = true;
+      export const prerender = true;
       export { load}`,
     ],
     [
       'variable declaration (var)',
       `import {something} from 'somewhere';
       var    load=async () => {};
-      export prerender = true;
+      export const prerender = true;
       export { load}`,
     ],
 
@@ -176,13 +177,18 @@ describe('canWrapLoad', () => {
        async function somethingElse(){};
        export { somethingElse as  load, foo }`,
     ],
-
+    [
+      'function declaration with different string literal name',
+      `import { foo } from 'somewhere';
+       async function somethingElse(){};
+       export { somethingElse as "load", foo }`,
+    ],
     [
       'export variable declaration - inline function with assigned type',
       `import type { LayoutLoad } from './$types';
        export const load :  LayoutLoad = async () => { return { props: { msg: "hi" } } }`,
     ],
-  ])('returns `true` if a load declaration  (%s) exists and no Sentry code was found', async (_, code) => {
+  ])('returns `true` if a load declaration  (%s) exists', async (_, code) => {
     fileContent = code;
     expect(await canWrapLoad('+page.ts', false)).toEqual(true);
   });
@@ -203,14 +209,4 @@ describe('canWrapLoad', () => {
     fileContent = code;
     expect(await canWrapLoad('+page.ts', false)).toEqual(true);
   });
-
-  it('returns `false` if Sentry code was found', async () => {
-    fileContent = 'import * as Sentry from "@sentry/sveltekit";';
-    expect(await canWrapLoad('+page.ts', false)).toEqual(false);
-  });
-
-  it('returns `false` if Sentry code was found', async () => {
-    fileContent = 'import * as Sentry from "@sentry/sveltekit";';
-    expect(await canWrapLoad('+page.ts', false)).toEqual(false);
-  });
 });

packages/types/src/envelope.ts

@@ -4,7 +4,7 @@ import type { DsnComponents } from './dsn';
 import type { Event } from './event';
 import type { ReplayEvent, ReplayRecordingData } from './replay';
 import type { SdkInfo } from './sdkinfo';
-import type { Session, SessionAggregates } from './session';
+import type { SerializedSession, Session, SessionAggregates } from './session';
 import type { Transaction } from './transaction';
 import type { UserFeedback } from './user';
 
@@ -76,7 +76,8 @@ export type EventItem = BaseEnvelopeItem<EventItemHeaders, Event>;
 export type AttachmentItem = BaseEnvelopeItem<AttachmentItemHeaders, string | Uint8Array>;
 export type UserFeedbackItem = BaseEnvelopeItem<UserFeedbackItemHeaders, UserFeedback>;
 export type SessionItem =
-  | BaseEnvelopeItem<SessionItemHeaders, Session>
+  // TODO(v8): Only allow serialized session here (as opposed to Session or SerializedSesison)
+  | BaseEnvelopeItem<SessionItemHeaders, Session | SerializedSession>
   | BaseEnvelopeItem<SessionAggregatesItemHeaders, SessionAggregates>;
 export type ClientReportItem = BaseEnvelopeItem<ClientReportItemHeaders, ClientReport>;
 export type CheckInItem = BaseEnvelopeItem<CheckInItemHeaders, SerializedCheckIn>;