From b9a9390ddf1b9025d7579199977aa4dc2a701614 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 25 May 2023 09:30:04 +0200 Subject: [PATCH 01/19] ci: Increase playwright test timeout (#8209) --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3670646ec8a7..990361d6ae7c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -475,7 +475,7 @@ jobs: needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-20.04 - timeout-minutes: 18 + timeout-minutes: 25 strategy: fail-fast: false matrix: From 3f31b3d6533b29548a44a6487144e614a78320db Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 25 May 2023 08:48:48 -0400 Subject: [PATCH 02/19] feat(replay): Remove unused debug statements (#8167) These debug statements are not really providing value, removing them... --- .../src/coreHandlers/handleGlobalEvent.ts | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 26fb5f4a633a..84bd1afd7abc 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -1,4 +1,3 @@ -import { addBreadcrumb } from '@sentry/core'; import type { Event, EventHint } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -50,13 +49,6 @@ export function handleGlobalEventListener( event.tags = { ...event.tags, replayId: replay.getSessionId() }; } - if (__DEBUG_BUILD__ && replay.getOptions()._experiments.traceInternals && isErrorEvent(event)) { - const exc = getEventExceptionValues(event); - addInternalBreadcrumb({ - message: `Tagging event (${event.event_id}) - ${event.message} - ${exc.type}: ${exc.value}`, - }); - } - // In cases where a custom client is used that does not support the new hooks (yet), // we manually call this hook method here if (afterSendHandler) { @@ -67,22 +59,3 @@ export function handleGlobalEventListener( return event; }; } - -function addInternalBreadcrumb(arg: Parameters[0]): void { - const { category, level, message, ...rest } = arg; - - addBreadcrumb({ - category: category || 'console', - level: level || 'debug', - message: `[debug]: ${message}`, - ...rest, - }); -} - -function getEventExceptionValues(event: Event): { type: string; value: string } { - return { - type: 'Unknown', - value: 'n/a', - ...(event.exception && event.exception.values && event.exception.values[0]), - }; -} From 41fef4b10f3a644179b77985f00f8696c908539f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 25 May 2023 16:39:13 +0200 Subject: [PATCH 03/19] fix(sveltekit): Avoid capturing redirects and 4xx Http errors in request Handlers (#8215) Stop sending thrown redirects and 4xx Http errors to Sentry. This change is identical to #7731 which introduced these filters to `load` functions. --- packages/sveltekit/src/common/utils.ts | 9 +++++- packages/sveltekit/src/server/handle.ts | 9 ++++++ packages/sveltekit/src/server/load.ts | 8 ++--- packages/sveltekit/test/common/utils.test.ts | 20 ++++++++++++- packages/sveltekit/test/server/handle.test.ts | 30 ++++++++++++++++++- 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/packages/sveltekit/src/common/utils.ts b/packages/sveltekit/src/common/utils.ts index ecf762cee8c4..84b384861dff 100644 --- a/packages/sveltekit/src/common/utils.ts +++ b/packages/sveltekit/src/common/utils.ts @@ -1,4 +1,4 @@ -import type { Redirect } from '@sveltejs/kit'; +import type { HttpError, Redirect } from '@sveltejs/kit'; export type SentryWrappedFlag = { /** @@ -22,3 +22,10 @@ export function isRedirect(error: unknown): error is Redirect { 'status' in error && typeof error.status === 'number' && error.status >= 300 && error.status <= 308; return hasValidLocation && hasValidStatus; } + +/** + * Determines if a thrown "error" is a HttpError + */ +export function isHttpError(err: unknown): err is HttpError { + return typeof err === 'object' && err !== null && 'status' in err && 'body' in err; +} diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index 88e0615d13bb..4fa725a15a2e 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -5,6 +5,7 @@ import { captureException } from '@sentry/node'; import { addExceptionMechanism, dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; +import { isHttpError, isRedirect } from '../common/utils'; import { getTracePropagationData } from './utils'; function sendErrorToSentry(e: unknown): unknown { @@ -12,6 +13,14 @@ function sendErrorToSentry(e: unknown): unknown { // store a seen flag on it. const objectifiedErr = objectify(e); + // similarly to the `load` function, we don't want to capture 4xx errors or redirects + if ( + isRedirect(objectifiedErr) || + (isHttpError(objectifiedErr) && objectifiedErr.status < 500 && objectifiedErr.status >= 400) + ) { + return objectifiedErr; + } + captureException(objectifiedErr, scope => { scope.addEventProcessor(event => { addExceptionMechanism(event, { diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index ab9f9ebdafb3..c776dc639d3a 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -3,19 +3,15 @@ import { trace } from '@sentry/core'; import { captureException } from '@sentry/node'; import type { TransactionContext } from '@sentry/types'; import { addExceptionMechanism, addNonEnumerableProperty, objectify } from '@sentry/utils'; -import type { HttpError, LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; +import type { LoadEvent, ServerLoadEvent } from '@sveltejs/kit'; import type { SentryWrappedFlag } from '../common/utils'; -import { isRedirect } from '../common/utils'; +import { isHttpError, isRedirect } from '../common/utils'; import { getTracePropagationData } from './utils'; type PatchedLoadEvent = LoadEvent & SentryWrappedFlag; type PatchedServerLoadEvent = ServerLoadEvent & SentryWrappedFlag; -function isHttpError(err: unknown): err is HttpError { - return typeof err === 'object' && err !== null && 'status' in err && 'body' in err; -} - function sendErrorToSentry(e: unknown): unknown { // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can // store a seen flag on it. diff --git a/packages/sveltekit/test/common/utils.test.ts b/packages/sveltekit/test/common/utils.test.ts index 51af645cc961..5581fe60c5e4 100644 --- a/packages/sveltekit/test/common/utils.test.ts +++ b/packages/sveltekit/test/common/utils.test.ts @@ -1,4 +1,6 @@ -import { isRedirect } from '../../src/common/utils'; +import { redirect } from '@sveltejs/kit'; + +import { isHttpError, isRedirect } from '../../src/common/utils'; describe('isRedirect', () => { it.each([ @@ -23,3 +25,19 @@ describe('isRedirect', () => { expect(isRedirect(redirectObject)).toBe(false); }); }); + +describe('isHttpError', () => { + it.each([ + { status: 404, body: 'Not found' }, + { status: 500, body: 'Internal server error' }, + ])('returns `true` for valid HttpError objects (%s)', httpErrorObject => { + expect(isHttpError(httpErrorObject)).toBe(true); + }); + + it.each([new Error(), redirect(301, '/users/id'), 'string error', { status: 404 }, { body: 'Not found' }])( + 'returns `false` for other thrown objects (%s)', + httpErrorObject => { + expect(isHttpError(httpErrorObject)).toBe(false); + }, + ); +}); diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index 94d471f20689..a1c6a5a3d0d8 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -2,6 +2,7 @@ import { addTracingExtensions, Hub, makeMain, Scope } from '@sentry/core'; import { NodeClient } from '@sentry/node'; import type { Transaction } from '@sentry/types'; import type { Handle } from '@sveltejs/kit'; +import { redirect } from '@sveltejs/kit'; import { vi } from 'vitest'; import { sentryHandle, transformPageChunk } from '../../src/server/handle'; @@ -69,7 +70,18 @@ const enum Type { Async = 'async', } -function resolve(type: Type, isError: boolean): Parameters[0]['resolve'] { +function resolve( + type: Type, + isError: boolean, + throwSpecialError?: 'redirect' | 'http', +): Parameters[0]['resolve'] { + if (throwSpecialError === 'redirect') { + throw redirect(302, '/redirect'); + } + if (throwSpecialError === 'http') { + throw { status: 404, body: 'Not found' }; + } + if (type === Type.Sync) { return (..._args: unknown[]) => { if (isError) { @@ -292,6 +304,22 @@ describe('handleSentry', () => { } }); + it("doesn't send redirects in a request handler to Sentry", async () => { + try { + await sentryHandle()({ event: mockEvent(), resolve: resolve(type, false, 'redirect') }); + } catch (e) { + expect(mockCaptureException).toBeCalledTimes(0); + } + }); + + it("doesn't send Http 4xx errors in a request handler to Sentry", async () => { + try { + await sentryHandle()({ event: mockEvent(), resolve: resolve(type, false, 'http') }); + } catch (e) { + expect(mockCaptureException).toBeCalledTimes(0); + } + }); + it('calls `transformPageChunk`', async () => { const mockResolve = vi.fn().mockImplementation(resolve(type, isError)); const event = mockEvent(); From f359ef31f3e123ea4f61b44f1e759907928192e3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 26 May 2023 20:23:47 +0200 Subject: [PATCH 04/19] feat(replay): Throttle breadcrumbs to max 300/5s (#8086) This updates custom breadcrumb handling to be throttled to max. 300 breadcrumbs/5s. If we exceed this amount of breadcrumbs, we drop any further breadcrumbs and instead add a single breadcrumb with `category: 'replay.throttled'`, which the UI can use to indicate that _something_ was dropped. Closes https://github.com/getsentry/sentry-javascript/issues/8072 --------- Co-authored-by: Billy Vong --- .../suites/replay/throttleBreadcrumbs/init.js | 17 +++ .../replay/throttleBreadcrumbs/subject.js | 8 ++ .../replay/throttleBreadcrumbs/template.html | 9 ++ .../suites/replay/throttleBreadcrumbs/test.ts | 41 ++++++ .../utils/replayHelpers.ts | 30 +++-- packages/replay/.eslintrc.js | 4 +- .../coreHandlers/util/addBreadcrumbEvent.ts | 3 +- packages/replay/src/replay.ts | 51 ++++++- packages/replay/src/types.ts | 5 + .../replay/src/util/createPerformanceSpans.ts | 14 +- packages/replay/src/util/throttle.ts | 55 ++++++++ .../replay/test/unit/util/throttle.test.ts | 125 ++++++++++++++++++ 12 files changed, 344 insertions(+), 18 deletions(-) create mode 100644 packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/init.js create mode 100644 packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/subject.js create mode 100644 packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/template.html create mode 100644 packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts create mode 100644 packages/replay/src/util/throttle.ts create mode 100644 packages/replay/test/unit/util/throttle.test.ts diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/init.js b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/init.js new file mode 100644 index 000000000000..11207b23752d --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/init.js @@ -0,0 +1,17 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 5000, + flushMaxDelay: 5000, + useCompression: false, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 0, + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + + integrations: [window.Replay], +}); diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/subject.js b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/subject.js new file mode 100644 index 000000000000..30ba4b350700 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/subject.js @@ -0,0 +1,8 @@ +const COUNT = 400; + +document.querySelector('[data-console]').addEventListener('click', () => { + // Call console.log() many times + for (let i = 0; i < COUNT; i++) { + console.log(`testing ${i}`); + } +}); diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/template.html b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/template.html new file mode 100644 index 000000000000..07807a41f8c8 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts new file mode 100644 index 000000000000..17f4210624a0 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; + +const THROTTLE_LIMIT = 300; + +sentryTest( + 'throttles breadcrumbs when many `console.log` are made at the same time', + async ({ getLocalTestUrl, page, forceFlushReplay, browserName }) => { + if (shouldSkipReplayTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + await page.click('[data-console]'); + await forceFlushReplay(); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + // 1 click breadcrumb + 1 throttled breadcrumb is why console logs are less + // than throttle limit + expect(breadcrumbs.length).toBe(THROTTLE_LIMIT); + expect(breadcrumbs.filter(breadcrumb => breadcrumb.category === 'replay.throttled').length).toBe(1); + }, +); diff --git a/packages/browser-integration-tests/utils/replayHelpers.ts b/packages/browser-integration-tests/utils/replayHelpers.ts index 415dcd667414..ec332edea74d 100644 --- a/packages/browser-integration-tests/utils/replayHelpers.ts +++ b/packages/browser-integration-tests/utils/replayHelpers.ts @@ -34,6 +34,25 @@ export type IncrementalRecordingSnapshot = eventWithTime & { export type RecordingSnapshot = FullRecordingSnapshot | IncrementalRecordingSnapshot; +/** Returns the replay event from the given request, or undefined if this is not a replay request. */ +export function getReplayEventFromRequest(req: Request): ReplayEvent | undefined { + const postData = req.postData(); + if (!postData) { + return undefined; + } + + try { + const event = envelopeRequestParser(req); + + if (!isReplayEvent(event)) { + return undefined; + } + + return event; + } catch { + return undefined; + } +} /** * Waits for a replay request to be sent by the page and returns it. * @@ -58,18 +77,13 @@ export function waitForReplayRequest( res => { const req = res.request(); - const postData = req.postData(); - if (!postData) { + const event = getReplayEventFromRequest(req); + + if (!event) { return false; } try { - const event = envelopeRequestParser(req); - - if (!isReplayEvent(event)) { - return false; - } - if (callback) { return callback(event, res); } diff --git a/packages/replay/.eslintrc.js b/packages/replay/.eslintrc.js index da006cf432a2..6db928fcb1b9 100644 --- a/packages/replay/.eslintrc.js +++ b/packages/replay/.eslintrc.js @@ -8,7 +8,9 @@ module.exports = { overrides: [ { files: ['src/**/*.ts'], - rules: {}, + rules: { + '@sentry-internal/sdk/no-unsupported-es6-methods': 'off', + }, }, { files: ['jest.setup.ts', 'jest.config.ts'], diff --git a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts index cbb998d499d4..947fb12f1ae4 100644 --- a/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts +++ b/packages/replay/src/coreHandlers/util/addBreadcrumbEvent.ts @@ -3,7 +3,6 @@ import type { Breadcrumb } from '@sentry/types'; import { normalize } from '@sentry/utils'; import type { ReplayContainer } from '../../types'; -import { addEvent } from '../../util/addEvent'; /** * Add a breadcrumb event to replay. @@ -20,7 +19,7 @@ export function addBreadcrumbEvent(replay: ReplayContainer, breadcrumb: Breadcru } replay.addUpdate(() => { - void addEvent(replay, { + void replay.throttledAddEvent({ type: EventType.Custom, // TODO: We were converting from ms to seconds for breadcrumbs, spans, // but maybe we should just keep them as milliseconds diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 52c772f57c21..53f263b5fa3f 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -24,6 +24,7 @@ import type { EventBuffer, InternalEventContext, PopEventContext, + RecordingEvent, RecordingOptions, ReplayContainer as ReplayContainerInterface, ReplayPluginOptions, @@ -42,6 +43,8 @@ import { getHandleRecordingEmit } from './util/handleRecordingEmit'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { sendReplay } from './util/sendReplay'; +import type { SKIPPED } from './util/throttle'; +import { throttle, THROTTLED } from './util/throttle'; /** * The main replay container class, which holds all the state and methods for recording and sending replays. @@ -75,6 +78,11 @@ export class ReplayContainer implements ReplayContainerInterface { maxSessionLife: MAX_SESSION_LIFE, } as const; + private _throttledAddEvent: ( + event: RecordingEvent, + isCheckout?: boolean, + ) => typeof THROTTLED | typeof SKIPPED | Promise; + /** * Options to pass to `rrweb.record()` */ @@ -136,6 +144,14 @@ export class ReplayContainer implements ReplayContainerInterface { this._debouncedFlush = debounce(() => this._flush(), this._options.flushMinDelay, { maxWait: this._options.flushMaxDelay, }); + + this._throttledAddEvent = throttle( + (event: RecordingEvent, isCheckout?: boolean) => addEvent(this, event, isCheckout), + // Max 300 events... + 300, + // ... per 5s + 5, + ); } /** Get the event context. */ @@ -565,6 +581,39 @@ export class ReplayContainer implements ReplayContainerInterface { this._context.urls.push(url); } + /** + * Add a breadcrumb event, that may be throttled. + * If it was throttled, we add a custom breadcrumb to indicate that. + */ + public throttledAddEvent( + event: RecordingEvent, + isCheckout?: boolean, + ): typeof THROTTLED | typeof SKIPPED | Promise { + const res = this._throttledAddEvent(event, isCheckout); + + // If this is THROTTLED, it means we have throttled the event for the first time + // In this case, we want to add a breadcrumb indicating that something was skipped + if (res === THROTTLED) { + const breadcrumb = createBreadcrumb({ + category: 'replay.throttled', + }); + + this.addUpdate(() => { + void addEvent(this, { + type: EventType.Custom, + timestamp: breadcrumb.timestamp || 0, + data: { + tag: 'breadcrumb', + payload: breadcrumb, + metric: true, + }, + }); + }); + } + + return res; + } + /** * Initialize and start all listeners to varying events (DOM, * Performance Observer, Recording, Sentry SDK, etc) @@ -803,7 +852,7 @@ export class ReplayContainer implements ReplayContainerInterface { */ private _createCustomBreadcrumb(breadcrumb: Breadcrumb): void { this.addUpdate(() => { - void addEvent(this, { + void this.throttledAddEvent({ type: EventType.Custom, timestamp: breadcrumb.timestamp || 0, data: { diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index ebea87d0f363..3d1d8616bb79 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -8,6 +8,7 @@ import type { } from '@sentry/types'; import type { eventWithTime, recordOptions } from './types/rrweb'; +import type { SKIPPED, THROTTLED } from './util/throttle'; export type RecordingEvent = eventWithTime; export type RecordingOptions = recordOptions; @@ -522,6 +523,10 @@ export interface ReplayContainer { session: Session | undefined; recordingMode: ReplayRecordingMode; timeouts: Timeouts; + throttledAddEvent: ( + event: RecordingEvent, + isCheckout?: boolean, + ) => typeof THROTTLED | typeof SKIPPED | Promise; isEnabled(): boolean; isPaused(): boolean; getContext(): InternalEventContext; diff --git a/packages/replay/src/util/createPerformanceSpans.ts b/packages/replay/src/util/createPerformanceSpans.ts index cd7a20ddf74a..86de4eb9f958 100644 --- a/packages/replay/src/util/createPerformanceSpans.ts +++ b/packages/replay/src/util/createPerformanceSpans.ts @@ -1,17 +1,16 @@ import { EventType } from '@sentry-internal/rrweb'; import type { AddEventResult, AllEntryData, ReplayContainer, ReplayPerformanceEntry } from '../types'; -import { addEvent } from './addEvent'; /** - * Create a "span" for each performance entry. The parent transaction is `this.replayEvent`. + * Create a "span" for each performance entry. */ export function createPerformanceSpans( replay: ReplayContainer, entries: ReplayPerformanceEntry[], ): Promise[] { - return entries.map(({ type, start, end, name, data }) => - addEvent(replay, { + return entries.map(({ type, start, end, name, data }) => { + const response = replay.throttledAddEvent({ type: EventType.Custom, timestamp: start, data: { @@ -24,6 +23,9 @@ export function createPerformanceSpans( data, }, }, - }), - ); + }); + + // If response is a string, it means its either THROTTLED or SKIPPED + return typeof response === 'string' ? Promise.resolve(null) : response; + }); } diff --git a/packages/replay/src/util/throttle.ts b/packages/replay/src/util/throttle.ts new file mode 100644 index 000000000000..66ce6ed0cdfb --- /dev/null +++ b/packages/replay/src/util/throttle.ts @@ -0,0 +1,55 @@ +export const THROTTLED = '__THROTTLED'; +export const SKIPPED = '__SKIPPED'; + +/** + * Create a throttled function off a given function. + * When calling the throttled function, it will call the original function only + * if it hasn't been called more than `maxCount` times in the last `durationSeconds`. + * + * Returns `THROTTLED` if throttled for the first time, after that `SKIPPED`, + * or else the return value of the original function. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function throttle any>( + fn: T, + maxCount: number, + durationSeconds: number, +): (...rest: Parameters) => ReturnType | typeof THROTTLED | typeof SKIPPED { + const counter = new Map(); + + const _cleanup = (now: number): void => { + const threshold = now - durationSeconds; + counter.forEach((_value, key) => { + if (key < threshold) { + counter.delete(key); + } + }); + }; + + const _getTotalCount = (): number => { + return [...counter.values()].reduce((a, b) => a + b, 0); + }; + + let isThrottled = false; + + return (...rest: Parameters): ReturnType | typeof THROTTLED | typeof SKIPPED => { + // Date in second-precision, which we use as basis for the throttling + const now = Math.floor(Date.now() / 1000); + + // First, make sure to delete any old entries + _cleanup(now); + + // If already over limit, do nothing + if (_getTotalCount() >= maxCount) { + const wasThrottled = isThrottled; + isThrottled = true; + return wasThrottled ? SKIPPED : THROTTLED; + } + + isThrottled = false; + const count = counter.get(now) || 0; + counter.set(now, count + 1); + + return fn(...rest); + }; +} diff --git a/packages/replay/test/unit/util/throttle.test.ts b/packages/replay/test/unit/util/throttle.test.ts new file mode 100644 index 000000000000..4f1c0bc9f6a2 --- /dev/null +++ b/packages/replay/test/unit/util/throttle.test.ts @@ -0,0 +1,125 @@ +import { BASE_TIMESTAMP } from '../..'; +import { SKIPPED, throttle, THROTTLED } from '../../../src/util/throttle'; + +jest.useFakeTimers(); + +describe('Unit | util | throttle', () => { + it('executes when not hitting the limit', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + + const callback = jest.fn(); + const fn = throttle(callback, 100, 60); + + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(3); + + jest.advanceTimersByTime(59_000); + + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(5); + + jest.advanceTimersByTime(1_000); + + fn(); + + expect(callback).toHaveBeenCalledTimes(6); + + jest.advanceTimersByTime(1_000); + + fn(); + + expect(callback).toHaveBeenCalledTimes(7); + }); + + it('stops executing when hitting the limit', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + const callback = jest.fn(); + const fn = throttle(callback, 5, 60); + + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(3); + + jest.advanceTimersByTime(59_000); + + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(5); + + jest.advanceTimersByTime(1_000); + + fn(); + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(5); + + // Now, the first three will "expire", so we can add three more + jest.advanceTimersByTime(1_000); + + fn(); + fn(); + fn(); + fn(); + + expect(callback).toHaveBeenCalledTimes(8); + }); + + it('has correct return value', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + + const callback = jest.fn(() => 'foo'); + const fn = throttle(callback, 5, 60); + + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe(THROTTLED); + expect(fn()).toBe(SKIPPED); + expect(fn()).toBe(SKIPPED); + + // After 61s, should be reset + jest.advanceTimersByTime(61_000); + + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe('foo'); + expect(fn()).toBe(THROTTLED); + expect(fn()).toBe(SKIPPED); + expect(fn()).toBe(SKIPPED); + }); + + it('passes args correctly', () => { + const now = BASE_TIMESTAMP; + jest.setSystemTime(now); + + const originalFunction = (a: number, b: number) => a + b; + const callback = jest.fn(originalFunction); + const fn = throttle(callback, 5, 60); + + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(3); + expect(fn(1, 2)).toBe(THROTTLED); + expect(fn(1, 2)).toBe(SKIPPED); + expect(fn(1, 2)).toBe(SKIPPED); + }); +}); From 8df6a2966dda563c10011531c07dbe34fe98f430 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 30 May 2023 11:21:45 +0200 Subject: [PATCH 05/19] fix(replay): Guard against missing key (#8246) `key` _should_ always be there, but let's guard against this as it seems it may be undefined. Fixes https://github.com/getsentry/sentry-javascript/issues/8237 --- packages/replay/src/coreHandlers/handleKeyboardEvent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts index e50f5e6e3ab5..f7943d34fa4f 100644 --- a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -28,7 +28,7 @@ export function getKeyboardBreadcrumb(event: KeyboardEvent): Breadcrumb | null { const { metaKey, shiftKey, ctrlKey, altKey, key, target } = event; // never capture for input fields - if (!target || isInputElement(target as HTMLElement)) { + if (!target || isInputElement(target as HTMLElement) || !key) { return null; } From 557e84a7f6e58ac3666cc129a73116459787c79f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 30 May 2023 14:07:39 +0200 Subject: [PATCH 06/19] ci: Improve canary issue creation (#8253) 1. Do not create an issue when manually running canary tests 2. Update existing issue instead of creating a new one every time 3. Update issue name so we see which canary test is failing --- .github/workflows/canary.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index 04e7b444a173..3bd402dce4cf 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -53,13 +53,14 @@ jobs: cd packages/e2e-tests yarn test:e2e - name: Create Issue - if: failure() + if: failure() && github.event_name == 'schedule' uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} RUN_LINK: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} with: filename: .github/CANARY_FAILURE_TEMPLATE.md + update_existing: true job_ember_canary_test: name: Ember Canary Tests @@ -92,10 +93,12 @@ jobs: yarn ember try:one ${{ matrix.scenario }} --skip-cleanup=true - name: Create Issue - if: failure() + if: failure() && github.event_name == 'schedule' uses: JasonEtco/create-an-issue@e27dddc79c92bc6e4562f268fffa5ed752639abd env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} RUN_LINK: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} with: filename: .github/CANARY_FAILURE_TEMPLATE.md + update_existing: true + title: 'Ember Canary tests failed' From e95e574ac31012d1851d42e6f0efc508cfa97148 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 30 May 2023 08:26:13 -0400 Subject: [PATCH 07/19] test(replay): Skip firefox for flakey tests (#8222) These tests seem to only flake on firefox? --- .../suites/replay/sessionExpiry/test.ts | 5 +++-- .../suites/replay/slowClick/mutation/test.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts b/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts index f1765b2a3c22..bd303c9e68c3 100644 --- a/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts +++ b/packages/browser-integration-tests/suites/replay/sessionExpiry/test.ts @@ -14,8 +14,9 @@ import { // Session should expire after 2s - keep in sync with init.js const SESSION_TIMEOUT = 2000; -sentryTest('handles an expired session', async ({ getLocalTestPath, page }) => { - if (shouldSkipReplayTest()) { +sentryTest('handles an expired session', async ({ browserName, getLocalTestPath, page }) => { + // This test seems to only be flakey on firefox + if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) { sentryTest.skip(); } diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts index 2ed458213955..d7b5800f9eea 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -63,8 +63,9 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(2000); }); -sentryTest('immediate mutation does not trigger slow click', async ({ getLocalTestUrl, page }) => { - if (shouldSkipReplayTest()) { +sentryTest('immediate mutation does not trigger slow click', async ({ browserName, getLocalTestUrl, page }) => { + // This test seems to only be flakey on firefox + if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) { sentryTest.skip(); } From 252c9e9352f49d597218511a166ca6535587303a Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 30 May 2023 15:37:25 +0200 Subject: [PATCH 08/19] fix(sveltekit): Bump `magicast` to support `satisfied` keyword (#8254) Bump `magicast` to the latest version, 0.2.8, which fixes auto instrumentation of files with `satisfies` keywords. --- .../src/routes/building/+page.server.ts | 5 +++ .../src/routes/building/+page.svelte | 6 +++ .../sveltekit/src/routes/building/+page.ts | 5 +++ packages/sveltekit/package.json | 2 +- yarn.lock | 38 +++++++++---------- 5 files changed, 36 insertions(+), 20 deletions(-) create mode 100644 packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.server.ts create mode 100644 packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.svelte create mode 100644 packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.ts diff --git a/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.server.ts b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.server.ts new file mode 100644 index 000000000000..b07376ba97c9 --- /dev/null +++ b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.server.ts @@ -0,0 +1,5 @@ +import type { PageServerLoad } from './$types'; + +export const load = (async _event => { + return { name: 'building (server)' }; +}) satisfies PageServerLoad; diff --git a/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.svelte b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.svelte new file mode 100644 index 000000000000..fde274c60705 --- /dev/null +++ b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.svelte @@ -0,0 +1,6 @@ +

Check Build

+ +

+ This route only exists to check that Typescript definitions + and auto instrumentation are working when the project is built. +

diff --git a/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.ts b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.ts new file mode 100644 index 000000000000..049acdc1fafa --- /dev/null +++ b/packages/e2e-tests/test-applications/sveltekit/src/routes/building/+page.ts @@ -0,0 +1,5 @@ +import type { PageLoad } from './$types'; + +export const load = (async _event => { + return { name: 'building' }; +}) satisfies PageLoad; diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index dd3133ad6b88..f96ee58811a8 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -28,7 +28,7 @@ "@sentry/types": "7.53.1", "@sentry/utils": "7.53.1", "@sentry/vite-plugin": "^0.6.0", - "magicast": "0.2.6", + "magicast": "0.2.8", "sorcery": "0.11.0" }, "devDependencies": { diff --git a/yarn.lock b/yarn.lock index 4c8836ca0838..d118d336daf6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1032,10 +1032,10 @@ resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.20.15.tgz#eec9f36d8eaf0948bb88c87a46784b5ee9fd0c89" integrity sha512-DI4a1oZuf8wC+oAJA9RW6ga3Zbe8RZFt7kD9i4qAspz3I/yHet1VvC3DiSy/fsUvv5pvJuNPh0LPOdCcqinDPg== -"@babel/parser@^7.21.8": - version "7.21.8" - resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.21.8.tgz#642af7d0333eab9c0ad70b14ac5e76dbde7bfdf8" - integrity sha512-6zavDGdzG3gUqAdWvlLFfk+36RilI+Pwyuuh7HItyeScCWP3k6i8vKclAQ0bM/0y/Kz/xiwvxhMv9MgTJP5gmA== +"@babel/parser@^7.21.9": + version "7.22.4" + resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.22.4.tgz#a770e98fd785c231af9d93f6459d36770993fb32" + integrity sha512-VLLsx06XkEYqBtE5YGPwfSGwfrjnyPP5oiGty3S8pQLFDFLaS8VwWSIxkTXpcvr5zeYLE6+MBNl2npl/YnfofA== "@babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression@^7.18.6": version "7.18.6" @@ -6584,10 +6584,10 @@ ast-types@0.14.2: dependencies: tslib "^2.0.1" -ast-types@0.15.2: - version "0.15.2" - resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.15.2.tgz#39ae4809393c4b16df751ee563411423e85fb49d" - integrity sha512-c27loCv9QkZinsa5ProX751khO9DJl/AcB5c2KNtA6NRvHKS0PgLfcftz72KVq504vB0Gku5s2kUZzDBvQWvHg== +ast-types@^0.16.1: + version "0.16.1" + resolved "https://registry.yarnpkg.com/ast-types/-/ast-types-0.16.1.tgz#7a9da1617c9081bc121faafe91711b4c8bb81da2" + integrity sha512-6t10qk83GOG8p0vKmaCr8eiilZwO171AvbROMtvvNiwrTly62t+7XkA8RdIIVbpMhCASAsxgAzdRSwh6nw/5Dg== dependencies: tslib "^2.0.1" @@ -17998,14 +17998,14 @@ magic-string@^0.30.0: dependencies: "@jridgewell/sourcemap-codec" "^1.4.13" -magicast@0.2.6: - version "0.2.6" - resolved "https://registry.yarnpkg.com/magicast/-/magicast-0.2.6.tgz#08c9f1900177ca1896e9c07981912171d4ed8ec1" - integrity sha512-6bX0nVjGrA41o+qHSv9Duiv3VuF7jUyjT7dIb3E61YW/5mucvCBMgyZssUznRc+xlUMPYyXZZluZjE1k5z+2yQ== +magicast@0.2.8: + version "0.2.8" + resolved "https://registry.yarnpkg.com/magicast/-/magicast-0.2.8.tgz#02b298c65fbc5b7d1fce52ef779c59caf68cc9cf" + integrity sha512-zEnqeb3E6TfMKYXGyHv3utbuHNixr04o3/gVGviSzVQkbFiU46VZUd+Ea/1npKfvEsEWxBYuIksKzoztTDPg0A== dependencies: - "@babel/parser" "^7.21.8" + "@babel/parser" "^7.21.9" "@babel/types" "^7.21.5" - recast "^0.22.0" + recast "^0.23.2" make-dir@3.1.0, make-dir@^3.0.0, make-dir@^3.0.2, make-dir@^3.1.0, make-dir@~3.1.0: version "3.1.0" @@ -22937,13 +22937,13 @@ recast@^0.20.5: source-map "~0.6.1" tslib "^2.0.1" -recast@^0.22.0: - version "0.22.0" - resolved "https://registry.yarnpkg.com/recast/-/recast-0.22.0.tgz#1dd3bf1b86e5eb810b044221a1a734234ed3e9c0" - integrity sha512-5AAx+mujtXijsEavc5lWXBPQqrM4+Dl5qNH96N2aNeuJFUzpiiToKPsxQD/zAIJHspz7zz0maX0PCtCTFVlixQ== +recast@^0.23.2: + version "0.23.2" + resolved "https://registry.yarnpkg.com/recast/-/recast-0.23.2.tgz#d3dda3e8f0a3366860d7508c00e34a338ac52b41" + integrity sha512-Qv6cPfVZyMOtPszK6PgW70pUgm7gPlFitAPf0Q69rlOA0zLw2XdDcNmPbVGYicFGT9O8I7TZ/0ryJD+6COvIPw== dependencies: assert "^2.0.0" - ast-types "0.15.2" + ast-types "^0.16.1" esprima "~4.0.0" source-map "~0.6.1" tslib "^2.0.1" From 3bbc1a57676a90296e483d312113fbfd9f9d7eb8 Mon Sep 17 00:00:00 2001 From: Richard Simko <1245031+richardsimko@users.noreply.github.com> Date: Tue, 30 May 2023 17:08:05 +0200 Subject: [PATCH 09/19] fix(node): Strip query and fragment from request URLs without route parameters (#8213) Fix transaction names having query params or fragments attached when request URLs without any route parameters were encountered under [certain conditions](https://github.com/getsentry/sentry-javascript/pull/8213#issuecomment-1564243501), Fixes #6586 --- .../tracing-internal/src/node/integrations/express.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/tracing-internal/src/node/integrations/express.ts b/packages/tracing-internal/src/node/integrations/express.ts index 8b3bcb52fcf4..165494563eb7 100644 --- a/packages/tracing-internal/src/node/integrations/express.ts +++ b/packages/tracing-internal/src/node/integrations/express.ts @@ -1,6 +1,12 @@ /* eslint-disable max-lines */ import type { Hub, Integration, PolymorphicRequest, Transaction } from '@sentry/types'; -import { extractPathForTransaction, getNumberOfUrlSegments, isRegExp, logger } from '@sentry/utils'; +import { + extractPathForTransaction, + getNumberOfUrlSegments, + isRegExp, + logger, + stripUrlQueryAndFragment, +} from '@sentry/utils'; import { shouldDisableAutoInstrumentation } from './utils/node-utils'; @@ -335,7 +341,7 @@ function instrumentRouter(appOrRouter: ExpressRouter): void { if (urlLength === routeLength) { if (!req._hasParameters) { if (req._reconstructedRoute !== req.originalUrl) { - req._reconstructedRoute = req.originalUrl; + req._reconstructedRoute = req.originalUrl ? stripUrlQueryAndFragment(req.originalUrl) : req.originalUrl; } } From ec094dbdf577121eb5162ff229ac390361219fcf Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 30 May 2023 17:12:33 +0200 Subject: [PATCH 10/19] feat(sveltekit): Add option to control handling of unknown server routes (#8201) Adds a `handleUnknownRoutes` option to the `sentryHandle` server request handler. The option defaults to `false` which will effectively reduce noise, such as random bot requests. Previously such requests would yield e.g. `GET null` transactions because they could not be matched to an existing route by the framework. See #8199 --- packages/sveltekit/src/server/handle.ts | 55 ++++++++++++++----- packages/sveltekit/test/server/handle.test.ts | 33 +++++++++++ 2 files changed, 74 insertions(+), 14 deletions(-) diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index 4fa725a15a2e..6d41317cc044 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -8,6 +8,24 @@ import type { Handle, ResolveOptions } from '@sveltejs/kit'; import { isHttpError, isRedirect } from '../common/utils'; import { getTracePropagationData } from './utils'; +export type SentryHandleOptions = { + /** + * Controls whether the SDK should capture errors and traces in requests that don't belong to a + * route defined in your SvelteKit application. + * + * By default, this option is set to `false` to reduce noise (e.g. bots sending random requests to your server). + * + * Set this option to `true` if you want to monitor requests events without a route. This might be useful in certain + * scenarios, for instance if you registered other handlers that handle these requests. + * If you set this option, you might want adjust the the transaction name in the `beforeSendTransaction` + * callback of your server-side `Sentry.init` options. You can also use `beforeSendTransaction` to filter out + * transactions that you still don't want to be sent to Sentry. + * + * @default false + */ + handleUnknownRoutes?: boolean; +}; + function sendErrorToSentry(e: unknown): unknown { // In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can // store a seen flag on it. @@ -68,33 +86,42 @@ export const transformPageChunk: NonNullable { + // if there is an active transaction, we know that this handle call is nested and hence + // we don't create a new domain for it. If we created one, nested server calls would + // create new transactions instead of adding a child span to the currently active span. + if (getCurrentHub().getScope().getSpan()) { + return instrumentHandle(input, options); + } + return runWithAsyncContext(() => { + return instrumentHandle(input, options); + }); + }; + return sentryRequestHandler; } -const sentryRequestHandler: Handle = input => { - // if there is an active transaction, we know that this handle call is nested and hence - // we don't create a new domain for it. If we created one, nested server calls would - // create new transactions instead of adding a child span to the currently active span. - if (getCurrentHub().getScope().getSpan()) { - return instrumentHandle(input); +function instrumentHandle({ event, resolve }: Parameters[0], options: SentryHandleOptions): ReturnType { + if (!event.route?.id && !options.handleUnknownRoutes) { + return resolve(event); } - return runWithAsyncContext(() => { - return instrumentHandle(input); - }); -}; -function instrumentHandle({ event, resolve }: Parameters[0]): ReturnType { const { traceparentData, dynamicSamplingContext } = getTracePropagationData(event); return trace( { op: 'http.server', - name: `${event.request.method} ${event.route.id}`, + name: `${event.request.method} ${event.route?.id || event.url.pathname}`, status: 'ok', ...traceparentData, metadata: { - source: 'route', + source: event.route?.id ? 'route' : 'url', dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }, diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index a1c6a5a3d0d8..ffe1db0e9e75 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -333,6 +333,39 @@ describe('handleSentry', () => { expect(mockResolve).toHaveBeenCalledTimes(1); expect(mockResolve).toHaveBeenCalledWith(event, { transformPageChunk: expect.any(Function) }); }); + + it("doesn't create a transaction if there's no route", async () => { + let ref: any = undefined; + client.on('finishTransaction', (transaction: Transaction) => { + ref = transaction; + }); + + try { + await sentryHandle()({ event: mockEvent({ route: undefined }), resolve: resolve(type, isError) }); + } catch { + // + } + + expect(ref).toBeUndefined(); + }); + + it("Creates a transaction if there's no route but `handleUnknownRequests` is true", async () => { + let ref: any = undefined; + client.on('finishTransaction', (transaction: Transaction) => { + ref = transaction; + }); + + try { + await sentryHandle({ handleUnknownRoutes: true })({ + event: mockEvent({ route: undefined }), + resolve: resolve(type, isError), + }); + } catch { + // + } + + expect(ref).toBeDefined(); + }); }); }); From 8482c0a02960d191d369db7af712ec48b9033f44 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 30 May 2023 17:23:32 +0200 Subject: [PATCH 11/19] feat(sveltekit): Auto-detect SvelteKit adapters (#8193) First step for Vercel support: Detecting the used SvelteKit adapter. (This currently does nothing other than detecting the adapter; next step is to configure the source maps plugin correctly for the respective adapters) ref #8085 --- packages/sveltekit/src/vite/detectAdapter.ts | 62 +++++++++++++++ .../sveltekit/src/vite/sentryVitePlugins.ts | 21 +++++ .../sveltekit/test/vite/detectAdapter.test.ts | 79 +++++++++++++++++++ .../test/vite/sentrySvelteKitPlugins.test.ts | 9 ++- packages/types/src/package.ts | 2 + 5 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 packages/sveltekit/src/vite/detectAdapter.ts create mode 100644 packages/sveltekit/test/vite/detectAdapter.test.ts diff --git a/packages/sveltekit/src/vite/detectAdapter.ts b/packages/sveltekit/src/vite/detectAdapter.ts new file mode 100644 index 000000000000..95e26dfc9f6d --- /dev/null +++ b/packages/sveltekit/src/vite/detectAdapter.ts @@ -0,0 +1,62 @@ +import type { Package } from '@sentry/types'; +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Supported @sveltejs/adapters-[adapter] SvelteKit adapters + */ +export type SupportedSvelteKitAdapters = 'node' | 'auto' | 'vercel' | 'other'; + +/** + * Tries to detect the used adapter for SvelteKit by looking at the dependencies. + * returns the name of the adapter or 'other' if no supported adapter was found. + */ +export async function detectAdapter(debug?: boolean): Promise { + const pkgJson = await loadPackageJson(); + + const allDependencies = pkgJson ? { ...pkgJson.dependencies, ...pkgJson.devDependencies } : {}; + + let adapter: SupportedSvelteKitAdapters = 'other'; + if (allDependencies['@sveltejs/adapter-vercel']) { + adapter = 'vercel'; + } else if (allDependencies['@sveltejs/adapter-node']) { + adapter = 'node'; + } else if (allDependencies['@sveltejs/adapter-auto']) { + adapter = 'auto'; + } + + if (debug) { + if (adapter === 'other') { + // eslint-disable-next-line no-console + console.warn( + "[Sentry SvelteKit Plugin] Couldn't detect SvelteKit adapter. Please set the 'adapter' option manually.", + ); + } else { + // eslint-disable-next-line no-console + console.log(`[Sentry SvelteKit Plugin] Detected SvelteKit ${adapter} adapter`); + } + } + + return adapter; +} + +/** + * Imports the pacakge.json file and returns the parsed JSON object. + */ +async function loadPackageJson(): Promise { + const pkgFile = path.join(process.cwd(), 'package.json'); + + try { + if (!fs.existsSync(pkgFile)) { + throw new Error(`File ${pkgFile} doesn't exist}`); + } + + const pkgJsonContent = (await fs.promises.readFile(pkgFile, 'utf-8')).toString(); + + return JSON.parse(pkgJsonContent); + } catch (e) { + // eslint-disable-next-line no-console + console.warn("[Sentry SvelteKit Plugin] Couldn't load package.json:", e); + return undefined; + } +} diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index 1f983d7a9653..e78a25adea10 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -3,6 +3,8 @@ import type { Plugin } from 'vite'; import type { AutoInstrumentSelection } from './autoInstrument'; import { makeAutoInstrumentationPlugin } from './autoInstrument'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; +import { detectAdapter } from './detectAdapter'; import { makeCustomSentryVitePlugin } from './sourceMaps'; type SourceMapsUploadOptions = { @@ -39,6 +41,24 @@ export type SentrySvelteKitPluginOptions = { * @default false. */ debug?: boolean; + + /** + * Specify which SvelteKit adapter you're using. + * By default, the SDK will attempt auto-detect the used adapter at build time and apply the + * correct config for source maps upload or auto-instrumentation. + * + * Currently, the SDK supports the following adapters: + * - node (@sveltejs/adapter-node) + * - auto (@sveltejs/adapter-auto) only Vercel + * - vercel (@sveltejs/adapter-auto) only Serverless functions, no edge runtime + * + * Set this option, if the SDK detects the wrong adapter or you want to use an adapter + * that is not in this list. If you specify 'other', you'll most likely need to configure + * source maps upload yourself. + * + * @default {} the SDK attempts to auto-detect the used adapter at build time + */ + adapter?: SupportedSvelteKitAdapters; } & SourceMapsUploadOptions & AutoInstrumentOptions; @@ -59,6 +79,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} const mergedOptions = { ...DEFAULT_PLUGIN_OPTIONS, ...options, + adapter: options.adapter || (await detectAdapter(options.debug || false)), }; const sentryPlugins: Plugin[] = []; diff --git a/packages/sveltekit/test/vite/detectAdapter.test.ts b/packages/sveltekit/test/vite/detectAdapter.test.ts new file mode 100644 index 000000000000..7f4e05a1a44b --- /dev/null +++ b/packages/sveltekit/test/vite/detectAdapter.test.ts @@ -0,0 +1,79 @@ +import { vi } from 'vitest'; + +import { detectAdapter } from '../../src/vite/detectAdapter'; + +let existsFile = true; +const pkgJson = { + dependencies: {}, +}; +describe('detectAdapter', () => { + beforeEach(() => { + existsFile = true; + vi.clearAllMocks(); + pkgJson.dependencies = {}; + }); + + vi.mock('fs', () => { + return { + existsSync: () => existsFile, + promises: { + readFile: () => { + return Promise.resolve(JSON.stringify(pkgJson)); + }, + }, + }; + }); + + const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + it.each(['auto', 'vercel', 'node'])( + 'returns the adapter name (adapter %s) and logs it to the console', + async adapter => { + pkgJson.dependencies[`@sveltejs/adapter-${adapter}`] = '1.0.0'; + const detectedAdapter = await detectAdapter(true); + expect(detectedAdapter).toEqual(adapter); + expect(consoleLogSpy).toHaveBeenCalledTimes(1); + expect(consoleLogSpy).toHaveBeenCalledWith(expect.stringContaining(`Detected SvelteKit ${adapter} adapter`)); + }, + ); + + it('returns "other" if no supported adapter was found', async () => { + pkgJson.dependencies['@sveltejs/adapter-netlify'] = '1.0.0'; + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('other'); + }); + + it('logs a warning if in debug mode and no supported adapter was found', async () => { + pkgJson.dependencies['@sveltejs/adapter-netlify'] = '1.0.0'; + const detectedAdapter = await detectAdapter(true); + expect(detectedAdapter).toEqual('other'); + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Couldn't detect SvelteKit adapter")); + }); + + it('returns "other" if package.json isnt available and emits a warning log', async () => { + existsFile = false; + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('other'); + + expect(consoleWarnSpy).toHaveBeenCalledTimes(1); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Couldn't load package.json"), + expect.any(Error), + ); + }); + + it('prefers all other adapters over adapter auto', async () => { + pkgJson.dependencies['@sveltejs/adapter-auto'] = '1.0.0'; + pkgJson.dependencies['@sveltejs/adapter-vercel'] = '1.0.0'; + pkgJson.dependencies['@sveltejs/adapter-node'] = '1.0.0'; + + const detectedAdapter = await detectAdapter(); + expect(detectedAdapter).toEqual('vercel'); + + delete pkgJson.dependencies['@sveltejs/adapter-vercel']; + const detectedAdapter2 = await detectAdapter(); + expect(detectedAdapter2).toEqual('node'); + }); +}); diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 963844bdef70..026d347d777d 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -17,7 +17,14 @@ vi.mock('fs', async () => { }; }); -describe('sentryVite()', () => { +vi.spyOn(console, 'log').mockImplementation(() => { + /* noop */ +}); +vi.spyOn(console, 'warn').mockImplementation(() => { + /* noop */ +}); + +describe('sentrySvelteKit()', () => { it('returns an array of Vite plugins', async () => { const plugins = await sentrySvelteKit(); diff --git a/packages/types/src/package.ts b/packages/types/src/package.ts index c44d66e62950..15a61f3668d3 100644 --- a/packages/types/src/package.ts +++ b/packages/types/src/package.ts @@ -2,4 +2,6 @@ export interface Package { name: string; version: string; + dependencies?: Record; + devDependencies?: Record; } From 78454aa092b0cec121513bb0bf862cebbbdfae87 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 31 May 2023 15:24:50 +0200 Subject: [PATCH 12/19] fix(wasm): Avoid throwing an error when WASM modules are loaded from blobs (#8263) Try-catch the URL creation to avoid throwing an error if a funky URL is encountered. fixes #8259 This will not fix symbolication for blob/binary WASM modules that aren't directly fetched but at least we don't error anymore --- packages/wasm/src/registry.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/wasm/src/registry.ts b/packages/wasm/src/registry.ts index 765b47026f00..2005c840b630 100644 --- a/packages/wasm/src/registry.ts +++ b/packages/wasm/src/registry.ts @@ -45,11 +45,22 @@ export function registerModule(module: WebAssembly.Module, url: string): void { if (oldIdx >= 0) { IMAGES.splice(oldIdx, 1); } + + let debugFileUrl = null; + if (debugFile) { + try { + debugFileUrl = new URL(debugFile, url).href; + } catch { + // debugFile could be a blob URL which causes the URL constructor to throw + // for now we just ignore this case + } + } + IMAGES.push({ type: 'wasm', code_id: buildId, code_file: url, - debug_file: debugFile ? new URL(debugFile, url).href : null, + debug_file: debugFileUrl, debug_id: `${buildId.padEnd(32, '0').slice(0, 32)}0`, }); } From df5c84ae95fa608ed86b9cd65afdab08813799d2 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 31 May 2023 15:27:15 -0400 Subject: [PATCH 13/19] feat(replay): Promote `mutationBreadcrumbLimit` and `mutationLimit` to regular feature (#8228) Instead of taking a fullsnapshot when `mutationLimit` is reached, lets be aggressive and stop the replay to ensure end-users are not negatively affected performance wise. The default for showing a breadcrumb is at 750 mutations, and the default limit to stop recording is 10000 mutations. Closes #8002 --- .../largeMutations/mutationLimit/init.js | 4 +- .../largeMutations/mutationLimit/test.ts | 40 +++++++++---------- packages/replay/src/integration.ts | 5 +++ packages/replay/src/replay.ts | 10 ++--- packages/replay/src/types.ts | 16 +++++++- .../replay/test/utils/setupReplayContainer.ts | 4 ++ 6 files changed, 48 insertions(+), 31 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js index 5c30f352959c..3aa2548f1522 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/init.js @@ -4,9 +4,7 @@ window.Sentry = Sentry; window.Replay = new Sentry.Replay({ flushMinDelay: 500, flushMaxDelay: 500, - _experiments: { - mutationLimit: 250, - }, + mutationLimit: 250, }); Sentry.init({ diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts index 5d5dbb9d3f93..84f0113263d7 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts @@ -1,10 +1,15 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; -import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; +import { + getReplayRecordingContent, + getReplaySnapshot, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../../utils/replayHelpers'; sentryTest( - 'handles large mutations with _experiments.mutationLimit configured', + 'handles large mutations by stopping replay when `mutationLimit` configured', async ({ getLocalTestPath, page, forceFlushReplay, browserName }) => { if (shouldSkipReplayTest() || ['webkit', 'firefox'].includes(browserName)) { sentryTest.skip(); @@ -34,36 +39,29 @@ sentryTest( await forceFlushReplay(); const res1 = await reqPromise1; - const reqPromise2 = waitForReplayRequest(page); + // replay should be stopped due to mutation limit + let replay = await getReplaySnapshot(page); + expect(replay.session).toBe(undefined); + expect(replay._isEnabled).toBe(false); void page.click('#button-modify'); await forceFlushReplay(); - const res2 = await reqPromise2; - const reqPromise3 = waitForReplayRequest(page); - - void page.click('#button-remove'); + await page.click('#button-remove'); await forceFlushReplay(); - const res3 = await reqPromise3; const replayData0 = getReplayRecordingContent(res0); - const replayData1 = getReplayRecordingContent(res1); - const replayData2 = getReplayRecordingContent(res2); - const replayData3 = getReplayRecordingContent(res3); - expect(replayData0.fullSnapshots.length).toBe(1); expect(replayData0.incrementalSnapshots.length).toBe(0); - // This includes both a full snapshot as well as some incremental snapshots - expect(replayData1.fullSnapshots.length).toBe(1); + // Breadcrumbs (click and mutation); + const replayData1 = getReplayRecordingContent(res1); + expect(replayData1.fullSnapshots.length).toBe(0); expect(replayData1.incrementalSnapshots.length).toBeGreaterThan(0); + expect(replayData1.breadcrumbs.map(({ category }) => category).sort()).toEqual(['replay.mutations', 'ui.click']); - // This does not trigger mutations, for whatever reason - so no full snapshot either! - expect(replayData2.fullSnapshots.length).toBe(0); - expect(replayData2.incrementalSnapshots.length).toBeGreaterThan(0); - - // This includes both a full snapshot as well as some incremental snapshots - expect(replayData3.fullSnapshots.length).toBe(1); - expect(replayData3.incrementalSnapshots.length).toBeGreaterThan(0); + replay = await getReplaySnapshot(page); + expect(replay.session).toBe(undefined); + expect(replay._isEnabled).toBe(false); }, ); diff --git a/packages/replay/src/integration.ts b/packages/replay/src/integration.ts index 114d015f2702..0a8813c14d38 100644 --- a/packages/replay/src/integration.ts +++ b/packages/replay/src/integration.ts @@ -60,6 +60,9 @@ export class Replay implements Integration { maskAllInputs = true, blockAllMedia = true, + mutationBreadcrumbLimit = 750, + mutationLimit = 10_000, + networkDetailAllowUrls = [], networkCaptureBodies = true, networkRequestHeaders = [], @@ -127,6 +130,8 @@ export class Replay implements Integration { blockAllMedia, maskAllInputs, maskAllText, + mutationBreadcrumbLimit, + mutationLimit, networkDetailAllowUrls, networkCaptureBodies, networkRequestHeaders: _getMergedNetworkHeaders(networkRequestHeaders), diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 53f263b5fa3f..5642b66b8b5a 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1056,8 +1056,8 @@ export class ReplayContainer implements ReplayContainerInterface { private _onMutationHandler = (mutations: unknown[]): boolean => { const count = mutations.length; - const mutationLimit = this._options._experiments.mutationLimit || 0; - const mutationBreadcrumbLimit = this._options._experiments.mutationBreadcrumbLimit || 1000; + const mutationLimit = this._options.mutationLimit; + const mutationBreadcrumbLimit = this._options.mutationBreadcrumbLimit; const overMutationLimit = mutationLimit && count > mutationLimit; // Create a breadcrumb if a lot of mutations happen at the same time @@ -1067,15 +1067,15 @@ export class ReplayContainer implements ReplayContainerInterface { category: 'replay.mutations', data: { count, + limit: overMutationLimit, }, }); this._createCustomBreadcrumb(breadcrumb); } + // Stop replay if over the mutation limit if (overMutationLimit) { - // We want to skip doing an incremental snapshot if there are too many mutations - // Instead, we do a full snapshot - this._triggerFullSnapshot(false); + void this.stop('mutationLimit'); return false; } diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 3d1d8616bb79..393eed5802a8 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -272,6 +272,20 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { */ maskAllText: boolean; + /** + * A high number of DOM mutations (in a single event loop) can cause + * performance regressions in end-users' browsers. This setting will create + * a breadcrumb in the recording when the limit has been reached. + */ + mutationBreadcrumbLimit: number; + + /** + * A high number of DOM mutations (in a single event loop) can cause + * performance regressions in end-users' browsers. This setting will cause + * recording to stop when the limit has been reached. + */ + mutationLimit: number; + /** * Callback before adding a custom recording event * @@ -295,8 +309,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { _experiments: Partial<{ captureExceptions: boolean; traceInternals: boolean; - mutationLimit: number; - mutationBreadcrumbLimit: number; slowClicks: { threshold: number; timeout: number; diff --git a/packages/replay/test/utils/setupReplayContainer.ts b/packages/replay/test/utils/setupReplayContainer.ts index 83ced117c464..c7c302cce72b 100644 --- a/packages/replay/test/utils/setupReplayContainer.ts +++ b/packages/replay/test/utils/setupReplayContainer.ts @@ -15,6 +15,8 @@ const DEFAULT_OPTIONS = { networkCaptureBodies: true, networkRequestHeaders: [], networkResponseHeaders: [], + mutationLimit: 1500, + mutationBreadcrumbLimit: 500, _experiments: {}, }; @@ -54,6 +56,8 @@ export const DEFAULT_OPTIONS_EVENT_PAYLOAD = { maskAllText: false, maskAllInputs: false, useCompression: DEFAULT_OPTIONS.useCompression, + mutationLimit: DEFAULT_OPTIONS.mutationLimit, + mutationBreadcrumbLimit: DEFAULT_OPTIONS.mutationBreadcrumbLimit, networkDetailHasUrls: DEFAULT_OPTIONS.networkDetailAllowUrls.length > 0, networkCaptureBodies: DEFAULT_OPTIONS.networkCaptureBodies, networkRequestHeaders: DEFAULT_OPTIONS.networkRequestHeaders.length > 0, From d5551aa9ad2f3bafda78d2f6b5664a783477c670 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 1 Jun 2023 10:29:21 +0200 Subject: [PATCH 14/19] ref(replay): Capture parametrized route (#8095) --- packages/replay/src/replay.ts | 21 ++++++++++++++++++- packages/replay/src/types.ts | 3 +++ .../replay/src/util/addGlobalListeners.ts | 10 +++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 5642b66b8b5a..718b658ad82d 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -1,7 +1,7 @@ /* eslint-disable max-lines */ // TODO: We might want to split this file up import { EventType, record } from '@sentry-internal/rrweb'; import { captureException, getCurrentHub } from '@sentry/core'; -import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types'; +import type { Breadcrumb, ReplayRecordingMode, Transaction } from '@sentry/types'; import { logger } from '@sentry/utils'; import { @@ -68,6 +68,12 @@ export class ReplayContainer implements ReplayContainerInterface { */ public recordingMode: ReplayRecordingMode = 'session'; + /** + * The current or last active transcation. + * This is only available when performance is enabled. + */ + public lastTransaction?: Transaction; + /** * These are here so we can overwrite them in tests etc. * @hidden @@ -614,6 +620,19 @@ export class ReplayContainer implements ReplayContainerInterface { return res; } + /** + * This will get the parametrized route name of the current page. + * This is only available if performance is enabled, and if an instrumented router is used. + */ + public getCurrentRoute(): string | undefined { + const lastTransaction = this.lastTransaction || getCurrentHub().getScope().getTransaction(); + if (!lastTransaction || !['route', 'custom'].includes(lastTransaction.metadata.source)) { + return undefined; + } + + return lastTransaction.name; + } + /** * Initialize and start all listeners to varying events (DOM, * Performance Observer, Recording, Sentry SDK, etc) diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 393eed5802a8..42758c1b06d9 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -4,6 +4,7 @@ import type { ReplayRecordingData, ReplayRecordingMode, SentryWrappedXMLHttpRequest, + Transaction, XhrBreadcrumbHint, } from '@sentry/types'; @@ -535,6 +536,7 @@ export interface ReplayContainer { session: Session | undefined; recordingMode: ReplayRecordingMode; timeouts: Timeouts; + lastTransaction?: Transaction; throttledAddEvent: ( event: RecordingEvent, isCheckout?: boolean, @@ -559,6 +561,7 @@ export interface ReplayContainer { getSessionId(): string | undefined; checkAndHandleExpiredSession(): boolean | void; setInitialState(): void; + getCurrentRoute(): string | undefined; } export interface ReplayPerformanceEntry { diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index e5096f397fae..c1adbc72e787 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -40,6 +40,16 @@ export function addGlobalListeners(replay: ReplayContainer): void { dsc.replay_id = replayId; } }); + + client.on('startTransaction', transaction => { + replay.lastTransaction = transaction; + }); + + // We may be missing the initial startTransaction due to timing issues, + // so we capture it on finish again. + client.on('finishTransaction', transaction => { + replay.lastTransaction = transaction; + }); } } From 05cf332b4849cdbc46c077a3d23de6dc1d718471 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 1 Jun 2023 13:12:21 +0200 Subject: [PATCH 15/19] feat(sveltekit): Add source maps support for Vercel (lambda) (#8256) Adjust our automatic source maps upload setup in the SvelteKit SDK to support SvelteKit apps deployed to Vercel. This will only work for Lambda functions/Node runtime; not for the Vercel Edge runtime. This required a few changes in our custom vite plugin as well as on the server side of the SDK: * Based on the used adapter (manually set or detected via #8193) and the `svelte.config.js` we determine the output directory where the generated JS emitted to. * The determined output directory is injected into the global object on the server side * When an error occurs on the server side, we strip the absolute filename of each stack frame so that the relative path of the server-side code within the output directory is left. * We also use the determined output directory to build the correct `include` entries for the source map upload plugin. With this change, source maps upload should work for auto and Vercel adapters, as well as for the Node adapter. As for the Node adapter, the stackframe rewrite behaviour was also changed but it is now more in line with all supported adapters. --- packages/sveltekit/.eslintrc.js | 6 ++ packages/sveltekit/src/server/utils.ts | 25 ++++++-- .../sveltekit/src/vite/injectGlobalValues.ts | 41 ++++++++++++ .../sveltekit/src/vite/sentryVitePlugins.ts | 1 + packages/sveltekit/src/vite/sourceMaps.ts | 63 +++++++++++++++---- packages/sveltekit/src/vite/svelteConfig.ts | 32 ++++++++-- packages/sveltekit/test/server/utils.test.ts | 36 ++++++++++- .../test/vite/injectGlobalValues.test.ts | 34 ++++++++++ .../test/vite/sentrySvelteKitPlugins.test.ts | 2 + .../sveltekit/test/vite/sourceMaps.test.ts | 11 +++- .../sveltekit/test/vite/svelteConfig.test.ts | 32 +++++++++- 11 files changed, 257 insertions(+), 26 deletions(-) create mode 100644 packages/sveltekit/src/vite/injectGlobalValues.ts create mode 100644 packages/sveltekit/test/vite/injectGlobalValues.test.ts diff --git a/packages/sveltekit/.eslintrc.js b/packages/sveltekit/.eslintrc.js index c3ca0faa363b..4a8474637698 100644 --- a/packages/sveltekit/.eslintrc.js +++ b/packages/sveltekit/.eslintrc.js @@ -17,6 +17,12 @@ module.exports = { project: ['tsconfig.test.json'], }, }, + { + files: ['src/vite/**', 'src/server/**'], + rules: { + '@sentry-internal/sdk/no-optional-chaining': 'off', + }, + }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/sveltekit/src/server/utils.ts b/packages/sveltekit/src/server/utils.ts index 17fc855ebc16..8ef6acced314 100644 --- a/packages/sveltekit/src/server/utils.ts +++ b/packages/sveltekit/src/server/utils.ts @@ -1,8 +1,16 @@ import type { DynamicSamplingContext, StackFrame, TraceparentData } from '@sentry/types'; -import { baggageHeaderToDynamicSamplingContext, basename, extractTraceparentData } from '@sentry/utils'; +import { + baggageHeaderToDynamicSamplingContext, + basename, + escapeStringForRegex, + extractTraceparentData, + GLOBAL_OBJ, + join, +} from '@sentry/utils'; import type { RequestEvent } from '@sveltejs/kit'; import { WRAPPED_MODULE_SUFFIX } from '../vite/autoInstrument'; +import type { GlobalWithSentryValues } from '../vite/injectGlobalValues'; /** * Takes a request event and extracts traceparent and DSC data @@ -35,7 +43,8 @@ export function rewriteFramesIteratee(frame: StackFrame): StackFrame { if (!frame.filename) { return frame; } - + const globalWithSentryValues: GlobalWithSentryValues = GLOBAL_OBJ; + const svelteKitBuildOutDir = globalWithSentryValues.__sentry_sveltekit_output_dir; const prefix = 'app:///'; // Check if the frame filename begins with `/` or a Windows-style prefix such as `C:\` @@ -48,8 +57,16 @@ export function rewriteFramesIteratee(frame: StackFrame): StackFrame { .replace(/\\/g, '/') // replace all `\\` instances with `/` : frame.filename; - const base = basename(filename); - frame.filename = `${prefix}${base}`; + let strippedFilename; + if (svelteKitBuildOutDir) { + strippedFilename = filename.replace( + new RegExp(`^.*${escapeStringForRegex(join(svelteKitBuildOutDir, 'server'))}/`), + '', + ); + } else { + strippedFilename = basename(filename); + } + frame.filename = `${prefix}${strippedFilename}`; } delete frame.module; diff --git a/packages/sveltekit/src/vite/injectGlobalValues.ts b/packages/sveltekit/src/vite/injectGlobalValues.ts new file mode 100644 index 000000000000..d0f6424a338d --- /dev/null +++ b/packages/sveltekit/src/vite/injectGlobalValues.ts @@ -0,0 +1,41 @@ +import type { InternalGlobal } from '@sentry/utils'; + +export type GlobalSentryValues = { + __sentry_sveltekit_output_dir?: string; +}; + +/** + * Extend the `global` type with custom properties that are + * injected by the SvelteKit SDK at build time. + * @see packages/sveltekit/src/vite/sourcemaps.ts + */ +export type GlobalWithSentryValues = InternalGlobal & GlobalSentryValues; + +export const VIRTUAL_GLOBAL_VALUES_FILE = '\0sentry-inject-global-values-file'; + +/** + * @returns code that injects @param globalSentryValues into the global object. + */ +export function getGlobalValueInjectionCode(globalSentryValues: GlobalSentryValues): string { + if (Object.keys(globalSentryValues).length === 0) { + return ''; + } + + const sentryGlobal = '_global'; + + const globalCode = `var ${sentryGlobal} = + typeof window !== 'undefined' ? + window : + typeof globalThis !== 'undefined' ? + globalThis : + typeof global !== 'undefined' ? + global : + typeof self !== 'undefined' ? + self : + {};`; + const injectedValuesCode = Object.entries(globalSentryValues) + .map(([key, value]) => `${sentryGlobal}["${key}"] = ${JSON.stringify(value)};`) + .join('\n'); + + return `${globalCode}\n${injectedValuesCode}\n`; +} diff --git a/packages/sveltekit/src/vite/sentryVitePlugins.ts b/packages/sveltekit/src/vite/sentryVitePlugins.ts index e78a25adea10..60852424ded2 100644 --- a/packages/sveltekit/src/vite/sentryVitePlugins.ts +++ b/packages/sveltekit/src/vite/sentryVitePlugins.ts @@ -103,6 +103,7 @@ export async function sentrySvelteKit(options: SentrySvelteKitPluginOptions = {} const pluginOptions = { ...mergedOptions.sourceMapsUploadOptions, debug: mergedOptions.debug, // override the plugin's debug flag with the one from the top-level options + adapter: mergedOptions.adapter, }; sentryPlugins.push(await makeCustomSentryVitePlugin(pluginOptions)); } diff --git a/packages/sveltekit/src/vite/sourceMaps.ts b/packages/sveltekit/src/vite/sourceMaps.ts index 8c7662a93813..6f2b7086786a 100644 --- a/packages/sveltekit/src/vite/sourceMaps.ts +++ b/packages/sveltekit/src/vite/sourceMaps.ts @@ -11,7 +11,10 @@ import * as sorcery from 'sorcery'; import type { Plugin } from 'vite'; import { WRAPPED_MODULE_SUFFIX } from './autoInstrument'; -import { getAdapterOutputDir, loadSvelteConfig } from './svelteConfig'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; +import type { GlobalSentryValues } from './injectGlobalValues'; +import { getGlobalValueInjectionCode, VIRTUAL_GLOBAL_VALUES_FILE } from './injectGlobalValues'; +import { getAdapterOutputDir, getHooksFileName, loadSvelteConfig } from './svelteConfig'; // sorcery has no types, so these are some basic type definitions: type Chain = { @@ -26,6 +29,10 @@ type SentryVitePluginOptionsOptionalInclude = Omit { +export async function makeCustomSentryVitePlugin(options?: CustomSentryVitePluginOptions): Promise { const svelteConfig = await loadSvelteConfig(); - const outputDir = await getAdapterOutputDir(svelteConfig); + const usedAdapter = options?.adapter || 'other'; + const outputDir = await getAdapterOutputDir(svelteConfig, usedAdapter); const hasSentryProperties = fs.existsSync(path.resolve(process.cwd(), 'sentry.properties')); const defaultPluginOptions: SentryVitePluginOptions = { - include: [ - { paths: [`${outputDir}/client`] }, - { paths: [`${outputDir}/server/chunks`] }, - { paths: [`${outputDir}/server`], ignore: ['chunks/**'] }, - ], + include: [`${outputDir}/client`, `${outputDir}/server`], configFile: hasSentryProperties ? 'sentry.properties' : undefined, release, }; @@ -70,10 +74,16 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio const sentryPlugin: Plugin = sentryVitePlugin(mergedOptions); const { debug } = mergedOptions; - const { buildStart, resolveId, transform, renderChunk } = sentryPlugin; + const { buildStart, renderChunk } = sentryPlugin; let isSSRBuild = true; + const serverHooksFile = getHooksFileName(svelteConfig, 'server'); + + const globalSentryValues: GlobalSentryValues = { + __sentry_sveltekit_output_dir: outputDir, + }; + const customPlugin: Plugin = { name: 'sentry-upload-source-maps', apply: 'build', // only apply this plugin at build time @@ -82,9 +92,7 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio // These hooks are copied from the original Sentry Vite plugin. // They're mostly responsible for options parsing and release injection. buildStart, - resolveId, renderChunk, - transform, // Modify the config to generate source maps config: config => { @@ -99,6 +107,27 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio }; }, + resolveId: (id, _importer, _ref) => { + if (id === VIRTUAL_GLOBAL_VALUES_FILE) { + return { + id: VIRTUAL_GLOBAL_VALUES_FILE, + external: false, + moduleSideEffects: true, + }; + } + // @ts-ignore - this hook exists on the plugin! + return sentryPlugin.resolveId(id, _importer, _ref); + }, + + load: id => { + if (id === VIRTUAL_GLOBAL_VALUES_FILE) { + return { + code: getGlobalValueInjectionCode(globalSentryValues), + }; + } + return null; + }, + configResolved: config => { // The SvelteKit plugins trigger additional builds within the main (SSR) build. // We just need a mechanism to upload source maps only once. @@ -109,6 +138,18 @@ export async function makeCustomSentryVitePlugin(options?: SentryVitePluginOptio } }, + transform: async (code, id) => { + let modifiedCode = code; + const isServerHooksFile = new RegExp(`/${escapeStringForRegex(serverHooksFile)}(.(js|ts|mjs|mts))?`).test(id); + + if (isServerHooksFile) { + const globalValuesImport = `; import "${VIRTUAL_GLOBAL_VALUES_FILE}";`; + modifiedCode = `${code}\n${globalValuesImport}\n`; + } + // @ts-ignore - this hook exists on the plugin! + return sentryPlugin.transform(modifiedCode, id); + }, + // We need to start uploading source maps later than in the original plugin // because SvelteKit is invoking the adapter at closeBundle. // This means that we need to wait until the adapter is done before we start uploading. diff --git a/packages/sveltekit/src/vite/svelteConfig.ts b/packages/sveltekit/src/vite/svelteConfig.ts index 702e29cb9c3f..07c701e912f1 100644 --- a/packages/sveltekit/src/vite/svelteConfig.ts +++ b/packages/sveltekit/src/vite/svelteConfig.ts @@ -5,6 +5,8 @@ import * as fs from 'fs'; import * as path from 'path'; import * as url from 'url'; +import type { SupportedSvelteKitAdapters } from './detectAdapter'; + /** * Imports the svelte.config.js file and returns the config object. * The sveltekit plugins import the config in the same way. @@ -35,20 +37,38 @@ export async function loadSvelteConfig(): Promise { } } +/** + * Reads a custom hooks directory from the SvelteKit config. In case no custom hooks + * directory is specified, the default directory is returned. + */ +export function getHooksFileName(svelteConfig: Config, hookType: 'client' | 'server'): string { + return svelteConfig.kit?.files?.hooks?.[hookType] || `src/hooks.${hookType}`; +} + /** * Attempts to read a custom output directory that can be specidied in the options * of a SvelteKit adapter. If no custom output directory is specified, the default * directory is returned. - * - * To get the directory, we have to apply a hack and call the adapter's adapt method + */ +export async function getAdapterOutputDir(svelteConfig: Config, adapter: SupportedSvelteKitAdapters): Promise { + if (adapter === 'node') { + return await getNodeAdapterOutputDir(svelteConfig); + } + + // Auto and Vercel adapters simply use config.kit.outDir + // Let's also use this directory for the 'other' case + return path.join(svelteConfig.kit?.outDir || '.svelte-kit', 'output'); +} + +/** + * To get the Node adapter output directory, we have to apply a hack and call the adapter's adapt method * with a custom adapter `Builder` that only calls the `writeClient` method. * This method is the first method that is called with the output directory. * Once we obtained the output directory, we throw an error to exit the adapter. * * see: https://github.com/sveltejs/kit/blob/master/packages/adapter-node/index.js#L17 - * */ -export async function getAdapterOutputDir(svelteConfig: Config): Promise { +async function getNodeAdapterOutputDir(svelteConfig: Config): Promise { // 'build' is the default output dir for the node adapter let outputDir = 'build'; @@ -56,7 +76,7 @@ export async function getAdapterOutputDir(svelteConfig: Config): Promise return outputDir; } - const adapter = svelteConfig.kit.adapter; + const nodeAdapter = svelteConfig.kit.adapter; const adapterBuilder: Builder = { writeClient(dest: string) { @@ -85,7 +105,7 @@ export async function getAdapterOutputDir(svelteConfig: Config): Promise }; try { - await adapter.adapt(adapterBuilder); + await nodeAdapter.adapt(adapterBuilder); } catch (_) { // We expect the adapter to throw in writeClient! } diff --git a/packages/sveltekit/test/server/utils.test.ts b/packages/sveltekit/test/server/utils.test.ts index 179cc6682d85..2fd9b0492013 100644 --- a/packages/sveltekit/test/server/utils.test.ts +++ b/packages/sveltekit/test/server/utils.test.ts @@ -1,6 +1,8 @@ import { RewriteFrames } from '@sentry/integrations'; import type { StackFrame } from '@sentry/types'; +import { basename } from '@sentry/utils'; +import type { GlobalWithSentryValues } from '../../src/server/utils'; import { getTracePropagationData, rewriteFramesIteratee } from '../../src/server/utils'; const MOCK_REQUEST_EVENT: any = { @@ -69,7 +71,7 @@ describe('rewriteFramesIteratee', () => { expect(result).not.toHaveProperty('module'); }); - it('does the same filename modification as the default RewriteFrames iteratee', () => { + it('does the same filename modification as the default RewriteFrames iteratee if no output dir is available', () => { const frame: StackFrame = { filename: '/some/path/to/server/chunks/3-ab34d22f.js', lineno: 1, @@ -94,4 +96,36 @@ describe('rewriteFramesIteratee', () => { expect(result).toStrictEqual(defaultResult); }); + + it.each([ + ['adapter-node', 'build', '/absolute/path/to/build/server/chunks/3-ab34d22f.js', 'app:///chunks/3-ab34d22f.js'], + [ + 'adapter-auto', + '.svelte-kit/output', + '/absolute/path/to/.svelte-kit/output/server/entries/pages/page.ts.js', + 'app:///entries/pages/page.ts.js', + ], + ])( + 'removes the absolut path to the server output dir, if the output dir is available (%s)', + (_, outputDir, frameFilename, modifiedFilename) => { + (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir = outputDir; + + const frame: StackFrame = { + filename: frameFilename, + lineno: 1, + colno: 1, + module: basename(frameFilename), + }; + + const result = rewriteFramesIteratee({ ...frame }); + + expect(result).toStrictEqual({ + filename: modifiedFilename, + lineno: 1, + colno: 1, + }); + + delete (globalThis as GlobalWithSentryValues).__sentry_sveltekit_output_dir; + }, + ); }); diff --git a/packages/sveltekit/test/vite/injectGlobalValues.test.ts b/packages/sveltekit/test/vite/injectGlobalValues.test.ts new file mode 100644 index 000000000000..0fe6b07dc989 --- /dev/null +++ b/packages/sveltekit/test/vite/injectGlobalValues.test.ts @@ -0,0 +1,34 @@ +import { getGlobalValueInjectionCode } from '../../src/vite/injectGlobalValues'; + +describe('getGlobalValueInjectionCode', () => { + it('returns code that injects values into the global object', () => { + const injectionCode = getGlobalValueInjectionCode({ + // @ts-ignore - just want to test this with multiple values + something: 'else', + __sentry_sveltekit_output_dir: '.svelte-kit/output', + }); + expect(injectionCode).toEqual(`var _global = + typeof window !== 'undefined' ? + window : + typeof globalThis !== 'undefined' ? + globalThis : + typeof global !== 'undefined' ? + global : + typeof self !== 'undefined' ? + self : + {}; +_global["something"] = "else"; +_global["__sentry_sveltekit_output_dir"] = ".svelte-kit/output"; +`); + + // Check that the code above is in fact valid and works as expected + // The return value of eval here is the value of the last expression in the code + expect(eval(`${injectionCode}`)).toEqual('.svelte-kit/output'); + + delete globalThis.__sentry_sveltekit_output_dir; + }); + + it('returns empty string if no values are passed', () => { + expect(getGlobalValueInjectionCode({})).toEqual(''); + }); +}); diff --git a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts index 026d347d777d..923005b2e9f9 100644 --- a/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts +++ b/packages/sveltekit/test/vite/sentrySvelteKitPlugins.test.ts @@ -72,6 +72,7 @@ describe('sentrySvelteKit()', () => { ignore: ['bar.js'], }, autoInstrument: false, + adapter: 'vercel', }); const plugin = plugins[0]; @@ -80,6 +81,7 @@ describe('sentrySvelteKit()', () => { debug: true, ignore: ['bar.js'], include: ['foo.js'], + adapter: 'vercel', }); }); diff --git a/packages/sveltekit/test/vite/sourceMaps.test.ts b/packages/sveltekit/test/vite/sourceMaps.test.ts index 7d412811b92e..9d565aceab58 100644 --- a/packages/sveltekit/test/vite/sourceMaps.test.ts +++ b/packages/sveltekit/test/vite/sourceMaps.test.ts @@ -6,7 +6,7 @@ const mockedSentryVitePlugin = { buildStart: vi.fn(), resolveId: vi.fn(), renderChunk: vi.fn(), - transform: vi.fn(), + transform: vi.fn().mockImplementation((code: string, _id: string) => code), writeBundle: vi.fn(), }; @@ -54,6 +54,15 @@ describe('makeCustomSentryVitePlugin()', () => { }); }); + it('injects the output dir into the server hooks file', async () => { + const plugin = await makeCustomSentryVitePlugin(); + // @ts-ignore this function exists! + const transformedCode = await plugin.transform('foo', '/src/hooks.server.ts'); + const expectedtransformedCode = 'foo\n; import "\0sentry-inject-global-values-file";\n'; + expect(mockedSentryVitePlugin.transform).toHaveBeenCalledWith(expectedtransformedCode, '/src/hooks.server.ts'); + expect(transformedCode).toEqual(expectedtransformedCode); + }); + it('uploads source maps during the SSR build', async () => { const plugin = await makeCustomSentryVitePlugin(); // @ts-ignore this function exists! diff --git a/packages/sveltekit/test/vite/svelteConfig.test.ts b/packages/sveltekit/test/vite/svelteConfig.test.ts index 73f624c8b1be..5f079deb7c1a 100644 --- a/packages/sveltekit/test/vite/svelteConfig.test.ts +++ b/packages/sveltekit/test/vite/svelteConfig.test.ts @@ -1,6 +1,7 @@ import { vi } from 'vitest'; -import { getAdapterOutputDir, loadSvelteConfig } from '../../src/vite/svelteConfig'; +import type { SupportedSvelteKitAdapters } from '../../src/vite/detectAdapter'; +import { getAdapterOutputDir, getHooksFileName, loadSvelteConfig } from '../../src/vite/svelteConfig'; let existsFile; @@ -62,8 +63,33 @@ describe('getAdapterOutputDir', () => { }, }; - it('returns the output directory of the adapter', async () => { - const outputDir = await getAdapterOutputDir({ kit: { adapter: mockedAdapter } }); + it('returns the output directory of the Node adapter', async () => { + const outputDir = await getAdapterOutputDir({ kit: { adapter: mockedAdapter } }, 'node'); expect(outputDir).toEqual('customBuildDir'); }); + + it.each(['vercel', 'auto', 'other'] as SupportedSvelteKitAdapters[])( + 'returns the config.kit.outdir directory for adapter-%s', + async adapter => { + const outputDir = await getAdapterOutputDir({ kit: { outDir: 'customOutDir' } }, adapter); + expect(outputDir).toEqual('customOutDir/output'); + }, + ); + + it('falls back to the default out dir for all other adapters if outdir is not specified in the config', async () => { + const outputDir = await getAdapterOutputDir({ kit: {} }, 'vercel'); + expect(outputDir).toEqual('.svelte-kit/output'); + }); +}); + +describe('getHooksFileName', () => { + it('returns the default hooks file name if no custom hooks file is specified', () => { + const hooksFileName = getHooksFileName({}, 'server'); + expect(hooksFileName).toEqual('src/hooks.server'); + }); + + it('returns the custom hooks file name if specified in the config', () => { + const hooksFileName = getHooksFileName({ kit: { files: { hooks: { server: 'serverhooks' } } } }, 'server'); + expect(hooksFileName).toEqual('serverhooks'); + }); }); From 6208e576446ed6a25a54250f16141353e5f0d968 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 1 Jun 2023 14:00:19 +0200 Subject: [PATCH 16/19] feat(core): Add default ignoreTransactions for Healthchecks (#8191) Ignore health check transactions by default. Provide option to enable them via `InboundFilters` option `disableTransactionDefaults`. Ref: https://github.com/getsentry/team-webplatform-meta/issues/70 --- .../core/src/integrations/inboundfilters.ts | 20 +++++++++-- .../lib/integrations/inboundfilters.test.ts | 36 +++++++++++++++++++ .../test/client/tracingPageLoad.test.ts | 4 +-- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 998c7ab8203d..3b6b2e8933de 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -5,6 +5,16 @@ import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/u // this is the result of a script being pulled in from an external domain and CORS. const DEFAULT_IGNORE_ERRORS = [/^Script error\.?$/, /^Javascript error: Script error\.? on line 0$/]; +const DEFAULT_IGNORE_TRANSACTIONS = [ + /^.*healthcheck.*$/, + /^.*healthy.*$/, + /^.*live.*$/, + /^.*ready.*$/, + /^.*heartbeat.*$/, + /^.*\/health$/, + /^.*\/healthz$/, +]; + /** Options for the InboundFilters integration */ export interface InboundFiltersOptions { allowUrls: Array; @@ -12,6 +22,8 @@ export interface InboundFiltersOptions { ignoreErrors: Array; ignoreTransactions: Array; ignoreInternal: boolean; + disableErrorDefaults: boolean; + disableTransactionDefaults: boolean; } /** Inbound filters configurable by the user */ @@ -62,9 +74,13 @@ export function _mergeOptions( ignoreErrors: [ ...(internalOptions.ignoreErrors || []), ...(clientOptions.ignoreErrors || []), - ...DEFAULT_IGNORE_ERRORS, + ...(internalOptions.disableErrorDefaults ? [] : DEFAULT_IGNORE_ERRORS), + ], + ignoreTransactions: [ + ...(internalOptions.ignoreTransactions || []), + ...(clientOptions.ignoreTransactions || []), + ...(internalOptions.disableTransactionDefaults ? [] : DEFAULT_IGNORE_TRANSACTIONS), ], - ignoreTransactions: [...(internalOptions.ignoreTransactions || []), ...(clientOptions.ignoreTransactions || [])], ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, }; } diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index a528d765bfeb..88ab23f65909 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -208,6 +208,21 @@ const TRANSACTION_EVENT_3: Event = { type: 'transaction', }; +const TRANSACTION_EVENT_HEALTH: Event = { + transaction: 'GET /health', + type: 'transaction', +}; + +const TRANSACTION_EVENT_HEALTH_2: Event = { + transaction: 'GET /healthy', + type: 'transaction', +}; + +const TRANSACTION_EVENT_HEALTH_3: Event = { + transaction: 'GET /live', + type: 'transaction', +}; + describe('InboundFilters', () => { describe('_isSentryError', () => { it('should work as expected', () => { @@ -372,7 +387,28 @@ describe('InboundFilters', () => { it('uses default filters', () => { const eventProcessor = createInboundFiltersEventProcessor(); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); + }); + + it('disable default error filters', () => { + const eventProcessor = createInboundFiltersEventProcessor({ disableErrorDefaults: true }); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(SCRIPT_ERROR_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(null); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(null); + }); + + it('disable default transaction filters', () => { + const eventProcessor = createInboundFiltersEventProcessor({ disableTransactionDefaults: true }); + expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(null); expect(eventProcessor(TRANSACTION_EVENT, {})).toBe(TRANSACTION_EVENT); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH, {})).toBe(TRANSACTION_EVENT_HEALTH); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_2, {})).toBe(TRANSACTION_EVENT_HEALTH_2); + expect(eventProcessor(TRANSACTION_EVENT_HEALTH_3, {})).toBe(TRANSACTION_EVENT_HEALTH_3); }); }); diff --git a/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts b/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts index 028c8b2d7998..dc0f76d67644 100644 --- a/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts +++ b/packages/nextjs/test/integration/test/client/tracingPageLoad.test.ts @@ -4,7 +4,7 @@ import { Transaction } from '@sentry/types'; test('should report a `pageload` transaction', async ({ page }) => { const transaction = await getMultipleSentryEnvelopeRequests(page, 1, { - url: '/healthy', + url: '/testy', envelopeType: 'transaction', }); @@ -16,5 +16,5 @@ test('should report a `pageload` transaction', async ({ page }) => { }, }); - expect(await countEnvelopes(page, { url: '/healthy', envelopeType: 'transaction', timeout: 4000 })).toBe(1); + expect(await countEnvelopes(page, { url: '/testy', envelopeType: 'transaction', timeout: 4000 })).toBe(1); }); From c368bc8b79203804ae27094c22a6c5ae5da3afce Mon Sep 17 00:00:00 2001 From: Janeene Beeforth Date: Thu, 1 Jun 2023 22:06:36 +1000 Subject: [PATCH 17/19] fix(remix): Pass `loadContext` through wrapped document request function (#8268) Ensure that the loadContext created in the remix server is passed through to `app/entry.server.ts` in the document request handler. Fixes issue #8265. --- packages/remix/src/utils/instrumentServer.ts | 12 ++++++++++-- packages/remix/src/utils/types.ts | 10 +++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 96acde1db2aa..7fbff4bb6bd8 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -113,6 +113,7 @@ function makeWrappedDocumentRequestFunction( responseStatusCode: number, responseHeaders: Headers, context: Record, + loadContext?: Record, ): Promise { let res: Response; @@ -120,7 +121,7 @@ function makeWrappedDocumentRequestFunction( const currentScope = getCurrentHub().getScope(); if (!currentScope) { - return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context); + return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context, loadContext); } try { @@ -133,7 +134,14 @@ function makeWrappedDocumentRequestFunction( }, }); - res = await origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context); + res = await origDocumentRequestFunction.call( + this, + request, + responseStatusCode, + responseHeaders, + context, + loadContext, + ); span?.finish(); } catch (err) { diff --git a/packages/remix/src/utils/types.ts b/packages/remix/src/utils/types.ts index e57b8ad15e34..642f6eef76cb 100644 --- a/packages/remix/src/utils/types.ts +++ b/packages/remix/src/utils/types.ts @@ -127,9 +127,13 @@ export interface ServerBuild { } export interface HandleDocumentRequestFunction { - (request: RemixRequest, responseStatusCode: number, responseHeaders: Headers, context: EntryContext): - | Promise - | Response; + ( + request: RemixRequest, + responseStatusCode: number, + responseHeaders: Headers, + context: EntryContext, + loadContext?: AppLoadContext, + ): Promise | Response; } export interface HandleDataRequestFunction { From 8c6ad156829f7c4eec34e4a67e6dd866ba482d5d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 1 Jun 2023 13:09:41 +0100 Subject: [PATCH 18/19] fix(remix): Don't log missing parameters warning on server-side. (#8269) We are logging a warning when any of the required hooks / functions to Remix Browser SDK are not available in the Root component. This Root component is also rendered on server-side where we don't have access to those functions. And as we're not creating pageload / navigation transactions there, those functions are not required anyway. So this PR prevents that warning to be logged on server-side which is misleading. --- packages/remix/src/performance/client.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/remix/src/performance/client.tsx b/packages/remix/src/performance/client.tsx index 0f88a67c2592..879c93e51f42 100644 --- a/packages/remix/src/performance/client.tsx +++ b/packages/remix/src/performance/client.tsx @@ -1,7 +1,7 @@ -import type { ErrorBoundaryProps} from '@sentry/react'; +import type { ErrorBoundaryProps } from '@sentry/react'; import { WINDOW, withErrorBoundary } from '@sentry/react'; import type { Transaction, TransactionContext } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { isNodeEnv, logger } from '@sentry/utils'; import * as React from 'react'; const DEFAULT_TAGS = { @@ -101,6 +101,7 @@ export function withSentry

, R extends React.FC // Early return when any of the required functions is not available. if (!_useEffect || !_useLocation || !_useMatches || !_customStartTransaction) { __DEBUG_BUILD__ && + !isNodeEnv() && logger.warn('Remix SDK was unable to wrap your root because of one or more missing parameters.'); // @ts-ignore Setting more specific React Component typing for `R` generic above From 95f87563101540b759e28445af33fa86746e5adb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 1 Jun 2023 14:11:47 +0200 Subject: [PATCH 19/19] meta: Update changelog for 7.54.0 --- CHANGELOG.md | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c759af697841..090ec65be827 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,69 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.54.0 + +### Important Changes + +- **feat(core): Add default entries to `ignoreTransactions` for Healthchecks #8191** + + All SDKs now filter out health check transactions by default. + These are transactions where the transaction name matches typical API health check calls, such as `/^.*healthy.*$/` or `/^. *heartbeat.*$/`. Take a look at [this list](https://github.com/getsentry/sentry-javascript/blob/8c6ad156829f7c4eec34e4a67e6dd866ba482d5d/packages/core/src/integrations/inboundfilters.ts#L8C2-L16) to learn which regexes we currently use to match transaction names. + We believe that these transactions do not provide value in most cases and we want to save you some of your quota by filtering them out by default. + These filters are implemented as default values for the top level `ignoreTransactions` option. + + You can disable this filtering by manually specifiying the `InboundFilters` integration and setting the `disableTransactionDefaults` option: + ```js + Sentry.init({ + //... + integrations: [new InboundFilters({ disableTransactionDefaults: true })], + }) + ``` + +- **feat(replay): Add `mutationBreadcrumbLimit` and `mutationLimit` to Replay Options (#8228)** + + The previously experimental options `mutationBreadcumbLimit` and `mutationLimit` have been promoted to regular Replay integration options. + + A high number of DOM mutations (in a single event loop) can cause performance regressions in end-users' browsers. + Use `mutationBreadcrumbLimit` to send a breadcrumb along with your recording if the mutation limit was reached. + Use `mutationLimit` to stop recording if the mutation limit was reached. + +- **feat(sveltekit): Add source maps support for Vercel (lambda) (#8256)** + - feat(sveltekit): Auto-detect SvelteKit adapters (#8193) + + The SvelteKit SDK can now be used if you deploy your SvelteKit app to Vercel. + By default, the SDK's Vite plugin will detect the used adapter and adjust the source map uploading config as necessary. + If you want to override the default adapter detection, you can specify the `adapter` option in the `sentrySvelteKit` options: + + ```js + // vite.config.js + export default defineConfig({ + plugins: [ + sentrySvelteKit({ + adapter: 'vercel', + }), + sveltekit(), + ], + }); + ``` + + Currently, the Vite plugin will configure itself correctly for `@sveltejs/adapter-auto`, `@sveltejs/adapter-vercel` and `@sveltejs/adapter-node`. + + **Important:** The SvelteKit SDK is not yet compatible with Vercel's edge runtime. + It will only work for lambda functions. + +### Other Changes + +- feat(replay): Throttle breadcrumbs to max 300/5s (#8086) +- feat(sveltekit): Add option to control handling of unknown server routes (#8201) +- fix(node): Strip query and fragment from request URLs without route parameters (#8213) +- fix(remix): Don't log missing parameters warning on server-side. (#8269) +- fix(remix): Pass `loadContext` through wrapped document request function (#8268) +- fix(replay): Guard against missing key (#8246) +- fix(sveltekit): Avoid capturing redirects and 4xx Http errors in request Handlers (#8215) +- fix(sveltekit): Bump `magicast` to support `satisfied` keyword (#8254) +- fix(wasm): Avoid throwing an error when WASM modules are loaded from blobs (#8263) + ## 7.53.1 - chore(deps): bump socket.io-parser from 4.2.1 to 4.2.3 (#8196)