Skip to content
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.

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

meta(changelog): Update changelog for 7.53.1 #8208

Merged
merged 8 commits into from
May 24, 2023
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/node/src/integrations/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
26 changes: 24 additions & 2 deletions packages/replay/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 3 additions & 1 deletion packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
18 changes: 16 additions & 2 deletions packages/replay/test/integration/errorSampleRate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
1 change: 1 addition & 0 deletions packages/sveltekit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
46 changes: 37 additions & 9 deletions packages/sveltekit/src/vite/autoInstrument.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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;
}

/**
Expand Down
24 changes: 10 additions & 14 deletions packages/sveltekit/test/vite/autoInstrument.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" } } }',
Expand All @@ -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}`,
],

Expand Down Expand Up @@ -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);
});
Expand All @@ -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);
});
});
5 changes: 3 additions & 2 deletions packages/types/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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>;
Expand Down