From 57220f45a2e35f48881d01467d08aebc4c79c35f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 1 Jun 2023 09:15:56 -0400 Subject: [PATCH 01/38] feat(replay): Do not restart replays on "keydown" (#8226) This is causing some low-value replays. If you search sentry for replays with duration < 5 seconds, you will see that most of them are re-newed sessions from idle that only have a "keydown" breadcrumb. I suspect users are alt-tabbing out of an idle Sentry page. Removing for now until we have a better way to detect when a session is truly resumed via keydown event. --- .../replay/src/coreHandlers/handleKeyboardEvent.ts | 5 ++++- packages/replay/src/replay.ts | 12 ++++++++++++ packages/replay/src/types.ts | 1 + 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts index f7943d34fa4f..0f7560f39584 100644 --- a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -12,7 +12,10 @@ export function handleKeyboardEvent(replay: ReplayContainer, event: KeyboardEven return; } - replay.triggerUserActivity(); + // Update user activity, but do not restart recording as it can create + // noisy/low-value replays (e.g. user comes back from idle, hits alt-tab, new + // session with a single "keydown" breadcrumb is created) + replay.updateUserActivity(); const breadcrumb = getKeyboardBreadcrumb(event); diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 718b658ad82d..4d5c918b7610 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -486,6 +486,18 @@ export class ReplayContainer implements ReplayContainerInterface { this._updateSessionActivity(); } + /** + * Updates the user activity timestamp *without* resuming + * recording. Some user events (e.g. keydown) can be create + * low-value replays that only contain the keypress as a + * breadcrumb. Instead this would require other events to + * create a new replay after a session has expired. + */ + public updateUserActivity(): void { + this._updateUserActivity(); + this._updateSessionActivity(); + } + /** * Only flush if `this.recordingMode === 'session'` */ diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index 42758c1b06d9..f52c163f6e69 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -556,6 +556,7 @@ export interface ReplayContainer { flushImmediate(): Promise; cancelFlush(): void; triggerUserActivity(): void; + updateUserActivity(): void; addUpdate(cb: AddUpdateCallback): void; getOptions(): ReplayPluginOptions; getSessionId(): string | undefined; From 1df2367f8c1b62625a9c65c36f80413b7beeb4e9 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 Jun 2023 10:45:32 +0200 Subject: [PATCH 02/38] fix(nextjs): Strip query params from transaction names of navigations to unknown routes (#8278) Fix a possible scenario in which transaction names can have query params, namely a navigation to an unknown route. --- packages/nextjs/src/client/performance.ts | 5 +++-- packages/nextjs/test/performance/client.test.ts | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/nextjs/src/client/performance.ts b/packages/nextjs/src/client/performance.ts index 2212092d5045..0dd0b109408f 100644 --- a/packages/nextjs/src/client/performance.ts +++ b/packages/nextjs/src/client/performance.ts @@ -143,7 +143,8 @@ export function nextRouterInstrumentation( if (startTransactionOnLocationChange) { Router.events.on('routeChangeStart', (navigationTarget: string) => { - const matchedRoute = getNextRouteFromPathname(stripUrlQueryAndFragment(navigationTarget)); + const strippedNavigationTarget = stripUrlQueryAndFragment(navigationTarget); + const matchedRoute = getNextRouteFromPathname(strippedNavigationTarget); let transactionName: string; let transactionSource: TransactionSource; @@ -152,7 +153,7 @@ export function nextRouterInstrumentation( transactionName = matchedRoute; transactionSource = 'route'; } else { - transactionName = navigationTarget; + transactionName = strippedNavigationTarget; transactionSource = 'url'; } diff --git a/packages/nextjs/test/performance/client.test.ts b/packages/nextjs/test/performance/client.test.ts index 0b3ae0e7437e..cc0f212cbf18 100644 --- a/packages/nextjs/test/performance/client.test.ts +++ b/packages/nextjs/test/performance/client.test.ts @@ -231,6 +231,7 @@ describe('nextRouterInstrumentation', () => { ['/news', '/news', 'route'], ['/news/', '/news', 'route'], ['/some-route-that-is-not-defined-12332', '/some-route-that-is-not-defined-12332', 'url'], // unknown route + ['/some-route-that-is-not-defined-12332?q=42', '/some-route-that-is-not-defined-12332', 'url'], // unknown route w/ query param ['/posts/42', '/posts/[id]', 'route'], ['/posts/42/', '/posts/[id]', 'route'], ['/posts/42?someParam=1', '/posts/[id]', 'route'], // query params are ignored From b532311af91a9153ef61cbdd6d85f0115d3bbe3c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 2 Jun 2023 13:43:02 +0200 Subject: [PATCH 03/38] chore(sveltekit): Update adapter compatibility in Readme (#8272) Update the readme now that we're compatible with more adapters than just the Node adapter. Co-authored-by: Francesco Novy --- packages/sveltekit/README.md | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/packages/sveltekit/README.md b/packages/sveltekit/README.md index 309b59fcd2ac..5ca2cff3e73d 100644 --- a/packages/sveltekit/README.md +++ b/packages/sveltekit/README.md @@ -16,10 +16,19 @@ ## Compatibility -Currently, the minimum supported version of SvelteKit is `1.0.0`. +The minimum supported version of SvelteKit is `1.0.0`. The SDK works best with Vite 4.2 and newer. Older Vite versions might not generate source maps correctly. +The SDK supports the following SvelteKit adapters: +- `@sveltejs/adapter-auto` - for Vercel with the Node runtime. Other deployment targets might work but we don't guarantee compatibility. +- `@sveltejs/adapter-vercel` - only for Node (Lambda) runtimes, not yet Vercel's edge runtime +- `@sveltejs/adapter-node` + +If you use the SDK with other adapters, we cannot guarantee that everything works as expected. +You might need to [manually configure source maps upload](#-configuring-source-maps-upload). +The SDK is currently not compatible with none-Node server runtimes, such as Vercel's Edge runtime or Cloudflare workers. + ## General This package is a wrapper around `@sentry/node` for the server and `@sentry/svelte` for the client side, with added functionality related to SvelteKit. @@ -40,9 +49,7 @@ If the setup through the wizard doesn't work for you, you can also set up the SD ### 1. Prerequesits & Installation -1. Ensure you've set up the [`@sveltejs/adapter-node` adapter](https://kit.svelte.dev/docs/adapter-node) - -2. Install the Sentry SvelteKit SDK: +1. Install the Sentry SvelteKit SDK: ```bash # Using npm @@ -310,12 +317,3 @@ export const load = wrapServerLoadWithSentry((event) => { }); ``` - -## Known Limitations - -This SDK is still under active development. -Take a look at our [SvelteKit SDK Development Roadmap](https://github.com/getsentry/sentry-javascript/issues/6692) to follow the progress. - -- **Adapters** other than `@sveltejs/adapter-node` are currently not supported. - We haven't yet tested other platforms like Vercel. - This is on our roadmap but it will come at a later time. From 9ed5cfdefeb012c6523ca04d0ea697d1829fc004 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 2 Jun 2023 08:39:59 -0400 Subject: [PATCH 04/38] feat(replay): Improve types for replay recording events (#8224) More specific types for our replay recording events. These are exported so that our UI can use them as well. ![image](https://github.com/getsentry/sentry-javascript/assets/79684/8b5d3ff1-ac44-4f30-b1b2-0057b98ec4b7) --- .../replay/src/coreHandlers/handleScope.ts | 17 +- packages/replay/src/index.ts | 8 + packages/replay/src/replay.ts | 9 +- packages/replay/src/types/index.ts | 4 + packages/replay/src/types/performance.ts | 160 ++++++++++++++++ .../replay/src/{types.ts => types/replay.ts} | 171 +---------------- packages/replay/src/types/replayFrame.ts | 174 ++++++++++++++++++ packages/replay/src/util/addEvent.ts | 8 +- packages/replay/src/util/createBreadcrumb.ts | 8 +- .../replay/src/util/handleRecordingEmit.ts | 4 +- .../beforeAddRecordingEvent.test.ts | 2 +- .../coreHandlers/handleScope.test.ts | 4 +- .../unit/coreHandlers/handleScope.test.ts | 10 +- .../unit/util/handleRecordingEmit.test.ts | 4 +- 14 files changed, 391 insertions(+), 192 deletions(-) create mode 100644 packages/replay/src/types/index.ts create mode 100644 packages/replay/src/types/performance.ts rename packages/replay/src/{types.ts => types/replay.ts} (73%) create mode 100644 packages/replay/src/types/replayFrame.ts diff --git a/packages/replay/src/coreHandlers/handleScope.ts b/packages/replay/src/coreHandlers/handleScope.ts index 8f937b140882..78d0b6dd3fd3 100644 --- a/packages/replay/src/coreHandlers/handleScope.ts +++ b/packages/replay/src/coreHandlers/handleScope.ts @@ -3,12 +3,19 @@ import { normalize } from '@sentry/utils'; import { CONSOLE_ARG_MAX_SIZE } from '../constants'; import type { ReplayContainer } from '../types'; +import type { ReplayFrame } from '../types/replayFrame'; import { createBreadcrumb } from '../util/createBreadcrumb'; import { fixJson } from '../util/truncateJson/fixJson'; import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; let _LAST_BREADCRUMB: null | Breadcrumb = null; +type BreadcrumbWithCategory = Required>; + +function isBreadcrumbWithCategory(breadcrumb: Breadcrumb): breadcrumb is BreadcrumbWithCategory { + return !!breadcrumb.category; +} + export const handleScopeListener: (replay: ReplayContainer) => (scope: Scope) => void = (replay: ReplayContainer) => (scope: Scope): void => { @@ -44,9 +51,9 @@ export function handleScope(scope: Scope): Breadcrumb | null { _LAST_BREADCRUMB = newBreadcrumb; if ( - newBreadcrumb.category && - (['fetch', 'xhr', 'sentry.event', 'sentry.transaction'].includes(newBreadcrumb.category) || - newBreadcrumb.category.startsWith('ui.')) + !isBreadcrumbWithCategory(newBreadcrumb) || + ['fetch', 'xhr', 'sentry.event', 'sentry.transaction'].includes(newBreadcrumb.category) || + newBreadcrumb.category.startsWith('ui.') ) { return null; } @@ -59,7 +66,9 @@ export function handleScope(scope: Scope): Breadcrumb | null { } /** exported for tests only */ -export function normalizeConsoleBreadcrumb(breadcrumb: Breadcrumb): Breadcrumb { +export function normalizeConsoleBreadcrumb( + breadcrumb: Omit & BreadcrumbWithCategory, +): ReplayFrame { const args = breadcrumb.data && breadcrumb.data.arguments; if (!Array.isArray(args) || args.length === 0) { diff --git a/packages/replay/src/index.ts b/packages/replay/src/index.ts index 0baefd4e9c37..239cd727d67f 100644 --- a/packages/replay/src/index.ts +++ b/packages/replay/src/index.ts @@ -1 +1,9 @@ export { Replay } from './integration'; +export type { + BreadcrumbFrame, + BreadcrumbFrameEvent, + ReplayFrame, + ReplayFrameEvent, + SpanFrame, + SpanFrameEvent, +} from './types/replayFrame'; diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 4d5c918b7610..e6f7e2128538 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, Transaction } from '@sentry/types'; +import type { ReplayRecordingMode, Transaction } from '@sentry/types'; import { logger } from '@sentry/utils'; import { @@ -21,6 +21,7 @@ import type { AddEventResult, AddUpdateCallback, AllPerformanceEntry, + BreadcrumbFrame, EventBuffer, InternalEventContext, PopEventContext, @@ -808,7 +809,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Tasks to run when we consider a page to be hidden (via blurring and/or visibility) */ - private _doChangeToBackgroundTasks(breadcrumb?: Breadcrumb): void { + private _doChangeToBackgroundTasks(breadcrumb?: BreadcrumbFrame): void { if (!this.session) { return; } @@ -828,7 +829,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Tasks to run when we consider a page to be visible (via focus and/or visibility) */ - private _doChangeToForegroundTasks(breadcrumb?: Breadcrumb): void { + private _doChangeToForegroundTasks(breadcrumb?: BreadcrumbFrame): void { if (!this.session) { return; } @@ -881,7 +882,7 @@ export class ReplayContainer implements ReplayContainerInterface { /** * Helper to create (and buffer) a replay breadcrumb from a core SDK breadcrumb */ - private _createCustomBreadcrumb(breadcrumb: Breadcrumb): void { + private _createCustomBreadcrumb(breadcrumb: BreadcrumbFrame): void { this.addUpdate(() => { void this.throttledAddEvent({ type: EventType.Custom, diff --git a/packages/replay/src/types/index.ts b/packages/replay/src/types/index.ts new file mode 100644 index 000000000000..2461e4f3ac8e --- /dev/null +++ b/packages/replay/src/types/index.ts @@ -0,0 +1,4 @@ +export * from './performance'; +export * from './replay'; +export * from './replayFrame'; +export * from './rrweb'; diff --git a/packages/replay/src/types/performance.ts b/packages/replay/src/types/performance.ts new file mode 100644 index 000000000000..0ff647e7d7d0 --- /dev/null +++ b/packages/replay/src/types/performance.ts @@ -0,0 +1,160 @@ +export type AllPerformanceEntry = PerformancePaintTiming | PerformanceResourceTiming | PerformanceNavigationTiming; + +// PerformancePaintTiming and PerformanceNavigationTiming are only available with TS 4.4 and newer +// Therefore, we're exporting them here to make them available in older TS versions +export type PerformancePaintTiming = PerformanceEntry; +export type PerformanceNavigationTiming = PerformanceEntry & + PerformanceResourceTiming & { + type: string; + transferSize: number; + + /** + * A DOMHighResTimeStamp representing the time immediately before the user agent + * sets the document's readyState to "interactive". + */ + domInteractive: number; + + /** + * A DOMHighResTimeStamp representing the time immediately before the current + * document's DOMContentLoaded event handler starts. + */ + domContentLoadedEventStart: number; + /** + * A DOMHighResTimeStamp representing the time immediately after the current + * document's DOMContentLoaded event handler completes. + */ + domContentLoadedEventEnd: number; + + /** + * A DOMHighResTimeStamp representing the time immediately before the current + * document's load event handler starts. + */ + loadEventStart: number; + + /** + * A DOMHighResTimeStamp representing the time immediately after the current + * document's load event handler completes. + */ + loadEventEnd: number; + + /** + * A DOMHighResTimeStamp representing the time immediately before the user agent + * sets the document's readyState to "complete". + */ + domComplete: number; + + /** + * A number representing the number of redirects since the last non-redirect + * navigation in the current browsing context. + */ + redirectCount: number; + }; +export type ExperimentalPerformanceResourceTiming = PerformanceResourceTiming & { + // Experimental, see: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus + // Requires Chrome 109 + responseStatus?: number; +}; + +export type PaintData = undefined; + +/** + * See https://developer.mozilla.org/en-US/docs/Web/API/PerformanceNavigationTiming + * + * Note `navigation.push` will not have any data + */ +export type NavigationData = Partial< + Pick< + PerformanceNavigationTiming, + | 'decodedBodySize' + | 'encodedBodySize' + | 'duration' + | 'domInteractive' + | 'domContentLoadedEventEnd' + | 'domContentLoadedEventStart' + | 'loadEventStart' + | 'loadEventEnd' + | 'domComplete' + | 'redirectCount' + > +> & { + /** + * Transfer size of resource + */ + size?: number; +}; + +export type ResourceData = Pick & { + /** + * Transfer size of resource + */ + size: number; + /** + * HTTP status code. Note this is experimental and not available on all browsers. + */ + statusCode?: number; +}; + +export interface LargestContentfulPaintData { + /** + * Render time (in ms) of the LCP + */ + value: number; + size: number; + /** + * The recording id of the LCP node. -1 if not found + */ + nodeId?: number; +} + +/** + * Entries that come from window.performance + */ +export type AllPerformanceEntryData = PaintData | NavigationData | ResourceData | LargestContentfulPaintData; + +export interface MemoryData { + memory: { + jsHeapSizeLimit: number; + totalJSHeapSize: number; + usedJSHeapSize: number; + }; +} + +export interface NetworkRequestData { + method?: string; + statusCode?: number; + requestBodySize?: number; + responseBodySize?: number; +} + +export interface HistoryData { + previous: string; +} + +export type AllEntryData = AllPerformanceEntryData | MemoryData | NetworkRequestData | HistoryData; + +export interface ReplayPerformanceEntry { + /** + * One of these types https://developer.mozilla.org/en-US/docs/Web/API/PerformanceEntry/entryType + */ + type: string; + + /** + * A more specific description of the performance entry + */ + name: string; + + /** + * The start timestamp in seconds + */ + start: number; + + /** + * The end timestamp in seconds + */ + end: number; + + /** + * Additional unstructured data to be included + */ + data: T; +} diff --git a/packages/replay/src/types.ts b/packages/replay/src/types/replay.ts similarity index 73% rename from packages/replay/src/types.ts rename to packages/replay/src/types/replay.ts index f52c163f6e69..262dcbf65a88 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types/replay.ts @@ -8,14 +8,14 @@ import type { XhrBreadcrumbHint, } from '@sentry/types'; -import type { eventWithTime, recordOptions } from './types/rrweb'; -import type { SKIPPED, THROTTLED } from './util/throttle'; +import type { SKIPPED, THROTTLED } from '../util/throttle'; +import type { AllPerformanceEntry } from './performance'; +import type { ReplayFrameEvent } from './replayFrame'; +import type { eventWithTime, recordOptions } from './rrweb'; -export type RecordingEvent = eventWithTime; +export type RecordingEvent = ReplayFrameEvent | eventWithTime; export type RecordingOptions = recordOptions; -export type AllPerformanceEntry = PerformancePaintTiming | PerformanceResourceTiming | PerformanceNavigationTiming; - export interface SendReplayData { recordingData: ReplayRecordingData; replayId: string; @@ -41,138 +41,6 @@ export interface WorkerRequest { arg?: string; } -// PerformancePaintTiming and PerformanceNavigationTiming are only available with TS 4.4 and newer -// Therefore, we're exporting them here to make them available in older TS versions -export type PerformancePaintTiming = PerformanceEntry; -export type PerformanceNavigationTiming = PerformanceEntry & - PerformanceResourceTiming & { - type: string; - transferSize: number; - - /** - * A DOMHighResTimeStamp representing the time immediately before the user agent - * sets the document's readyState to "interactive". - */ - domInteractive: number; - - /** - * A DOMHighResTimeStamp representing the time immediately before the current - * document's DOMContentLoaded event handler starts. - */ - domContentLoadedEventStart: number; - /** - * A DOMHighResTimeStamp representing the time immediately after the current - * document's DOMContentLoaded event handler completes. - */ - domContentLoadedEventEnd: number; - - /** - * A DOMHighResTimeStamp representing the time immediately before the current - * document's load event handler starts. - */ - loadEventStart: number; - - /** - * A DOMHighResTimeStamp representing the time immediately after the current - * document's load event handler completes. - */ - loadEventEnd: number; - - /** - * A DOMHighResTimeStamp representing the time immediately before the user agent - * sets the document's readyState to "complete". - */ - domComplete: number; - - /** - * A number representing the number of redirects since the last non-redirect - * navigation in the current browsing context. - */ - redirectCount: number; - }; -export type ExperimentalPerformanceResourceTiming = PerformanceResourceTiming & { - // Experimental, see: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus - // Requires Chrome 109 - responseStatus?: number; -}; - -export type PaintData = undefined; - -/** - * See https://developer.mozilla.org/en-US/docs/Web/API/PerformanceNavigationTiming - * - * Note `navigation.push` will not have any data - */ -export type NavigationData = Partial< - Pick< - PerformanceNavigationTiming, - | 'decodedBodySize' - | 'encodedBodySize' - | 'duration' - | 'domInteractive' - | 'domContentLoadedEventEnd' - | 'domContentLoadedEventStart' - | 'loadEventStart' - | 'loadEventEnd' - | 'domComplete' - | 'redirectCount' - > -> & { - /** - * Transfer size of resource - */ - size?: number; -}; - -export type ResourceData = Pick & { - /** - * Transfer size of resource - */ - size: number; - /** - * HTTP status code. Note this is experimental and not available on all browsers. - */ - statusCode?: number; -}; - -export interface LargestContentfulPaintData { - /** - * Render time (in ms) of the LCP - */ - value: number; - size: number; - /** - * The recording id of the LCP node. -1 if not found - */ - nodeId?: number; -} - -/** - * Entries that come from window.performance - */ -export type AllPerformanceEntryData = PaintData | NavigationData | ResourceData | LargestContentfulPaintData; - -export interface MemoryData { - memory: { - jsHeapSizeLimit: number; - totalJSHeapSize: number; - usedJSHeapSize: number; - }; -} - -export interface NetworkRequestData { - method?: string; - statusCode?: number; - requestBodySize?: number; - responseBodySize?: number; -} - -export interface HistoryData { - previous: string; -} - -export type AllEntryData = AllPerformanceEntryData | MemoryData | NetworkRequestData | HistoryData; - /** * The response from the worker */ @@ -186,7 +54,7 @@ export interface WorkerResponse { export type AddEventResult = void; export interface BeforeAddRecordingEvent { - (event: RecordingEvent): RecordingEvent | null | undefined; + (event: ReplayFrameEvent): ReplayFrameEvent | null | undefined; } export interface ReplayNetworkOptions { @@ -565,33 +433,6 @@ export interface ReplayContainer { getCurrentRoute(): string | undefined; } -export interface ReplayPerformanceEntry { - /** - * One of these types https://developer.mozilla.org/en-US/docs/Web/API/PerformanceEntry/entryType - */ - type: string; - - /** - * A more specific description of the performance entry - */ - name: string; - - /** - * The start timestamp in seconds - */ - start: number; - - /** - * The end timestamp in seconds - */ - end: number; - - /** - * Additional unstructured data to be included - */ - data: T; -} - type RequestBody = null | Blob | BufferSource | FormData | URLSearchParams | string; export type XhrHint = XhrBreadcrumbHint & { diff --git a/packages/replay/src/types/replayFrame.ts b/packages/replay/src/types/replayFrame.ts new file mode 100644 index 000000000000..463e462851e6 --- /dev/null +++ b/packages/replay/src/types/replayFrame.ts @@ -0,0 +1,174 @@ +import type { Breadcrumb, FetchBreadcrumbData, XhrBreadcrumbData } from '@sentry/types'; + +import type { AllEntryData } from './performance'; +import type { EventType } from './rrweb'; + +interface BaseReplayFrame { + timestamp: number; + /** + * For compatibility reasons + */ + type: string; + category: string; + data?: Record; + message?: string; +} + +interface BaseDomFrameData { + nodeId?: number; + node?: { + id: number; + tagName: string; + textContent: string; + attributes: Record; + }; +} + +/* Breadcrumbs from Core SDK */ +interface ConsoleFrameData { + logger: string; + arguments?: unknown[]; +} +interface ConsoleFrame extends BaseReplayFrame { + category: 'console'; + level: Breadcrumb['level']; + message: string; + data: ConsoleFrameData; +} + +type ClickFrameData = BaseDomFrameData; +interface ClickFrame extends BaseReplayFrame { + category: 'ui.click'; + message: string; + data: ClickFrameData; +} + +interface FetchFrame extends BaseReplayFrame { + category: 'fetch'; + type: 'http'; + data: FetchBreadcrumbData; +} + +interface InputFrame extends BaseReplayFrame { + category: 'ui.input'; + message: string; +} + +interface XhrFrame extends BaseReplayFrame { + category: 'xhr'; + type: 'http'; + data: XhrBreadcrumbData; +} + +/* Breadcrumbs from Replay */ +interface MutationFrameData { + count: number; + limit: boolean; +} +interface MutationFrame extends BaseReplayFrame { + category: 'replay.mutations'; + data: MutationFrameData; +} + +interface KeyboardEventFrameData extends BaseDomFrameData { + metaKey: boolean; + shiftKey: boolean; + ctrlKey: boolean; + altKey: boolean; + key: string; +} +interface KeyboardEventFrame extends BaseReplayFrame { + category: 'ui.keyDown'; + data: KeyboardEventFrameData; +} + +interface BlurFrame extends BaseReplayFrame { + category: 'ui.blur'; +} + +interface FocusFrame extends BaseReplayFrame { + category: 'ui.focus'; +} + +interface SlowClickFrameData extends ClickFrameData { + url: string; + timeAfterClickFs: number; + endReason: string; +} +interface SlowClickFrame extends BaseReplayFrame { + category: 'ui.slowClickDetected'; + data: SlowClickFrameData; +} + +interface OptionFrame { + sessionSampleRate: number; + errorSampleRate: number; + useCompressionOption: boolean; + blockAllMedia: boolean; + maskAllText: boolean; + maskAllInputs: boolean; + useCompression: boolean; + networkDetailHasUrls: boolean; + networkCaptureBodies: boolean; + networkRequestHasHeaders: boolean; + networkResponseHasHeaders: boolean; +} + +export type BreadcrumbFrame = + | ConsoleFrame + | ClickFrame + | FetchFrame + | InputFrame + | XhrFrame + | KeyboardEventFrame + | BlurFrame + | FocusFrame + | SlowClickFrame + | MutationFrame + | BaseReplayFrame; + +export interface SpanFrame { + op: string; + description: string; + startTimestamp: number; + endTimestamp: number; + data: AllEntryData; +} + +export type ReplayFrame = BreadcrumbFrame | SpanFrame; + +interface RecordingCustomEvent { + type: EventType.Custom; + timestamp: number; + data: { + tag: string; + payload: unknown; + }; +} + +export interface BreadcrumbFrameEvent extends RecordingCustomEvent { + data: { + tag: 'breadcrumb'; + payload: BreadcrumbFrame; + /** + * This will indicate to backend to additionally log as a metric + */ + metric?: boolean; + }; +} + +export interface SpanFrameEvent extends RecordingCustomEvent { + data: { + tag: 'performanceSpan'; + payload: SpanFrame; + }; +} + +export interface OptionFrameEvent extends RecordingCustomEvent { + data: { + tag: 'options'; + payload: OptionFrame; + }; +} + +export type ReplayFrameEvent = BreadcrumbFrameEvent | SpanFrameEvent | OptionFrameEvent; diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index e40a9c2f6486..2a5458887af6 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -1,10 +1,14 @@ import { getCurrentHub } from '@sentry/core'; import { logger } from '@sentry/utils'; -import type { AddEventResult, RecordingEvent, ReplayContainer } from '../types'; +import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent } from '../types'; import { EventType } from '../types/rrweb'; import { timestampToMs } from './timestampToMs'; +function isCustomEvent(event: RecordingEvent): event is ReplayFrameEvent { + return event.type === EventType.Custom; +} + /** * Add an event to the event buffer. * `isCheckout` is true if this is either the very first event, or an event triggered by `checkoutEveryNms`. @@ -42,7 +46,7 @@ export async function addEvent( const replayOptions = replay.getOptions(); const eventAfterPossibleCallback = - typeof replayOptions.beforeAddRecordingEvent === 'function' && event.type === EventType.Custom + typeof replayOptions.beforeAddRecordingEvent === 'function' && isCustomEvent(event) ? replayOptions.beforeAddRecordingEvent(event) : event; diff --git a/packages/replay/src/util/createBreadcrumb.ts b/packages/replay/src/util/createBreadcrumb.ts index b8ff6097d571..5cf044333876 100644 --- a/packages/replay/src/util/createBreadcrumb.ts +++ b/packages/replay/src/util/createBreadcrumb.ts @@ -1,13 +1,11 @@ -import type { Breadcrumb } from '@sentry/types'; - -type RequiredProperties = 'category' | 'message'; +import type { BreadcrumbFrame } from '../types/replayFrame'; /** * Create a breadcrumb for a replay. */ export function createBreadcrumb( - breadcrumb: Pick & Partial>, -): Breadcrumb { + breadcrumb: Omit & Partial>, +): BreadcrumbFrame { return { timestamp: Date.now() / 1000, type: 'default', diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index 3a9dcc211edd..e4d507d33456 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -1,7 +1,7 @@ import { logger } from '@sentry/utils'; import { saveSession } from '../session/saveSession'; -import type { AddEventResult, RecordingEvent, ReplayContainer } from '../types'; +import type { AddEventResult, OptionFrameEvent, RecordingEvent, ReplayContainer } from '../types'; import { EventType } from '../types/rrweb'; import { addEvent } from './addEvent'; @@ -121,7 +121,7 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa /** * Exported for tests */ -export function createOptionsEvent(replay: ReplayContainer): RecordingEvent { +export function createOptionsEvent(replay: ReplayContainer): OptionFrameEvent { const options = replay.getOptions(); return { type: EventType.Custom, diff --git a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts index c01140045389..0f9db6554ec9 100644 --- a/packages/replay/test/integration/beforeAddRecordingEvent.test.ts +++ b/packages/replay/test/integration/beforeAddRecordingEvent.test.ts @@ -37,7 +37,7 @@ describe('Integration | beforeAddRecordingEvent', () => { ({ replay, integration } = await mockSdk({ replayOptions: { beforeAddRecordingEvent: event => { - const eventData = event.data as Record; + const eventData = event.data; if (eventData.tag === 'breadcrumb' && eventData.payload.category === 'ui.click') { return { diff --git a/packages/replay/test/integration/coreHandlers/handleScope.test.ts b/packages/replay/test/integration/coreHandlers/handleScope.test.ts index ec17d430ff52..d9d30d710a6a 100644 --- a/packages/replay/test/integration/coreHandlers/handleScope.test.ts +++ b/packages/replay/test/integration/coreHandlers/handleScope.test.ts @@ -23,10 +23,10 @@ describe('Integration | coreHandlers | handleScope', () => { expect(mockHandleScopeListener).toHaveBeenCalledTimes(1); - getCurrentHub().getScope()?.addBreadcrumb({ message: 'testing' }); + getCurrentHub().getScope()?.addBreadcrumb({ category: 'console', message: 'testing' }); expect(mockHandleScope).toHaveBeenCalledTimes(1); - expect(mockHandleScope).toHaveReturnedWith(expect.objectContaining({ message: 'testing' })); + expect(mockHandleScope).toHaveReturnedWith(expect.objectContaining({ category: 'console', message: 'testing' })); mockHandleScope.mockClear(); diff --git a/packages/replay/test/unit/coreHandlers/handleScope.test.ts b/packages/replay/test/unit/coreHandlers/handleScope.test.ts index 1bce28f860c8..9ce56b89d1e0 100644 --- a/packages/replay/test/unit/coreHandlers/handleScope.test.ts +++ b/packages/replay/test/unit/coreHandlers/handleScope.test.ts @@ -63,21 +63,21 @@ describe('Unit | coreHandlers | handleScope', () => { describe('normalizeConsoleBreadcrumb', () => { it('handles console messages with no arguments', () => { - const breadcrumb: Breadcrumb = { category: 'console', message: 'test' }; + const breadcrumb = { category: 'console', message: 'test' }; const actual = HandleScope.normalizeConsoleBreadcrumb(breadcrumb); expect(actual).toMatchObject({ category: 'console', message: 'test' }); }); it('handles console messages with empty arguments', () => { - const breadcrumb: Breadcrumb = { category: 'console', message: 'test', data: { arguments: [] } }; + const breadcrumb = { category: 'console', message: 'test', data: { arguments: [] } }; const actual = HandleScope.normalizeConsoleBreadcrumb(breadcrumb); expect(actual).toMatchObject({ category: 'console', message: 'test', data: { arguments: [] } }); }); it('handles console messages with simple arguments', () => { - const breadcrumb: Breadcrumb = { + const breadcrumb = { category: 'console', message: 'test', data: { arguments: [1, 'a', true, null, undefined] }, @@ -94,7 +94,7 @@ describe('Unit | coreHandlers | handleScope', () => { }); it('truncates large strings', () => { - const breadcrumb: Breadcrumb = { + const breadcrumb = { category: 'console', message: 'test', data: { @@ -114,7 +114,7 @@ describe('Unit | coreHandlers | handleScope', () => { }); it('truncates large JSON objects', () => { - const breadcrumb: Breadcrumb = { + const breadcrumb = { category: 'console', message: 'test', data: { diff --git a/packages/replay/test/unit/util/handleRecordingEmit.test.ts b/packages/replay/test/unit/util/handleRecordingEmit.test.ts index a4c7f82c425d..7978939291bd 100644 --- a/packages/replay/test/unit/util/handleRecordingEmit.test.ts +++ b/packages/replay/test/unit/util/handleRecordingEmit.test.ts @@ -1,7 +1,7 @@ import { EventType } from '@sentry-internal/rrweb'; import { BASE_TIMESTAMP } from '../..'; -import type { RecordingEvent } from '../../../src/types'; +import type { OptionFrameEvent } from '../../../src/types'; import * as SentryAddEvent from '../../../src/util/addEvent'; import { createOptionsEvent, getHandleRecordingEmit } from '../../../src/util/handleRecordingEmit'; import { setupReplayContainer } from '../../utils/setupReplayContainer'; @@ -9,7 +9,7 @@ import { useFakeTimers } from '../../utils/use-fake-timers'; useFakeTimers(); -let optionsEvent: RecordingEvent; +let optionsEvent: OptionFrameEvent; describe('Unit | util | handleRecordingEmit', () => { let addEventMock: jest.SpyInstance; From 9304ec3ab4a7bf05a08813d37d6becbce49776f7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 5 Jun 2023 10:02:00 +0200 Subject: [PATCH 05/38] ref(svelte): Add Svelte 4 as a peer dependency (#8280) --- packages/svelte/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/package.json b/packages/svelte/package.json index fa63ba9d68e3..33114f3b9f3d 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -23,7 +23,7 @@ "tslib": "^1.9.3" }, "peerDependencies": { - "svelte": "3.x" + "svelte": "3.x || 4.x" }, "devDependencies": { "@testing-library/svelte": "^3.2.1", From bca567e56dc2beab367eeac22a6babea4a959d1e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 5 Jun 2023 13:02:44 +0200 Subject: [PATCH 06/38] test(e2e): Disable concurrent tests for canary tests (#8287) --- packages/e2e-tests/lib/runAllTestApps.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/e2e-tests/lib/runAllTestApps.ts b/packages/e2e-tests/lib/runAllTestApps.ts index ad30c34e4738..d5f4c247f2e6 100644 --- a/packages/e2e-tests/lib/runAllTestApps.ts +++ b/packages/e2e-tests/lib/runAllTestApps.ts @@ -8,7 +8,11 @@ export async function runAllTestApps( recipePaths: string[], envVarsToInject: Record, ): Promise { - const maxParallel = process.env.CI ? 3 : 6; + const maxParallel = process.env.CANARY_E2E_TEST + ? 1 // TODO: figure out why concurrent tests fail for Next.js and remove this concurrency limitation + : process.env.CI + ? 3 + : 6; const recipeInstances = constructRecipeInstances(recipePaths); From 62e72656104c4e386ca56afd3ecdb9fb34f6d631 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 6 Jun 2023 02:04:11 +0000 Subject: [PATCH 07/38] chore(deps-dev): bump vite from 4.0.0 to 4.0.5 Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 4.0.0 to 4.0.5. - [Release notes](https://github.com/vitejs/vite/releases) - [Changelog](https://github.com/vitejs/vite/blob/v4.0.5/packages/vite/CHANGELOG.md) - [Commits](https://github.com/vitejs/vite/commits/v4.0.5/packages/vite) --- updated-dependencies: - dependency-name: vite dependency-type: direct:development ... Signed-off-by: dependabot[bot] --- packages/sveltekit/package.json | 2 +- yarn.lock | 56 +++++++++++---------------------- 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index bdd7487c5e81..95289150547e 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -36,7 +36,7 @@ "rollup": "^3.20.2", "svelte": "^3.44.0", "typescript": "^4.9.3", - "vite": "4.0.0" + "vite": "4.0.5" }, "scripts": { "build": "run-p build:transpile build:types", diff --git a/yarn.lock b/yarn.lock index d118d336daf6..b4ab26edf619 100644 --- a/yarn.lock +++ b/yarn.lock @@ -18928,11 +18928,16 @@ nan@^2.12.1: resolved "https://registry.yarnpkg.com/nan/-/nan-2.14.2.tgz#f5376400695168f4cc694ac9393d0c9585eeea19" integrity sha512-M2ufzIiINKCuDfBSAUr1vWQ+vuVcA9kqx8JJUsbQi6yf1uGRyb7HfpdfUr5qLXf3B/t8dPvcjhKMmlfnP47EzQ== -nanoid@^3.1.16, nanoid@^3.1.20, nanoid@^3.1.23, nanoid@^3.3.4: +nanoid@^3.1.16, nanoid@^3.1.20, nanoid@^3.1.23: version "3.3.4" resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.3.4.tgz#730b67e3cd09e2deacf03c027c81c9d9dbc5e8ab" integrity sha512-MqBkQh/OHTS2egovRtLk45wEyNXwF+cokD+1YPf9u5VfJiRdAiRwB2froX5Co9Rh20xs4siNPm8naNotSD6RBw== +nanoid@^3.3.6: + version "3.3.6" + resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.3.6.tgz#443380c856d6e9f9824267d960b4236ad583ea4c" + integrity sha512-BGcqMMJuToF7i1rt+2PWSNVnWIkGCU78jBG3RxO/bZlnZPK2Cmi2QaffxGO/2RvWi9sL+FAiRiXMgsyxQ1DIDA== + nanomatch@^1.2.9: version "1.2.13" resolved "https://registry.yarnpkg.com/nanomatch/-/nanomatch-1.2.13.tgz#b87a8aa4fc0de8fe6be88895b38983ff265bd119" @@ -22042,21 +22047,12 @@ postcss@^7.0.0, postcss@^7.0.1, postcss@^7.0.14, postcss@^7.0.17, postcss@^7.0.2 picocolors "^0.2.1" source-map "^0.6.1" -postcss@^8.1.10, postcss@^8.1.7, postcss@^8.2.15: - version "8.4.19" - resolved "https://registry.yarnpkg.com/postcss/-/postcss-8.4.19.tgz#61178e2add236b17351897c8bcc0b4c8ecab56fc" - integrity sha512-h+pbPsyhlYj6N2ozBmHhHrs9DzGmbaarbLvWipMRO7RLS+v4onj26MPFXA5OBYFxyqYhUJK456SwDcY9H2/zsA== +postcss@^8.1.10, postcss@^8.1.7, postcss@^8.2.15, postcss@^8.2.4, postcss@^8.3.5, postcss@^8.3.7, postcss@^8.4.20, postcss@^8.4.21: + version "8.4.24" + resolved "https://registry.yarnpkg.com/postcss/-/postcss-8.4.24.tgz#f714dba9b2284be3cc07dbd2fc57ee4dc972d2df" + integrity sha512-M0RzbcI0sO/XJNucsGjvWU9ERWxb/ytp1w6dKtxTKgixdtQDq4rmx/g8W1hnaheq9jgwL/oyEdH5Bc4WwJKMqg== dependencies: - nanoid "^3.3.4" - picocolors "^1.0.0" - source-map-js "^1.0.2" - -postcss@^8.2.4, postcss@^8.3.5, postcss@^8.3.7, postcss@^8.4.19, postcss@^8.4.21: - version "8.4.21" - resolved "https://registry.yarnpkg.com/postcss/-/postcss-8.4.21.tgz#c639b719a57efc3187b13a1d765675485f4134f4" - integrity sha512-tP7u/Sn/dVxK2NnruI4H9BG+x+Wxz6oeZ1cJ8P6G/PZY0IKk4k/63TDsQf2kQq3+qoJeLm2kIBUNlZe3zgb4Zg== - dependencies: - nanoid "^3.3.4" + nanoid "^3.3.6" picocolors "^1.0.0" source-map-js "^1.0.2" @@ -23638,41 +23634,27 @@ rollup@2.26.5: optionalDependencies: fsevents "~2.1.2" -rollup@2.78.0, rollup@^2.67.1, rollup@^2.8.0: +rollup@2.78.0: version "2.78.0" resolved "https://registry.yarnpkg.com/rollup/-/rollup-2.78.0.tgz#00995deae70c0f712ea79ad904d5f6b033209d9e" integrity sha512-4+YfbQC9QEVvKTanHhIAFVUFSRsezvQF8vFOJwtGfb9Bb+r014S+qryr9PSmw8x6sMnPkmFBGAvIFVQxvJxjtg== optionalDependencies: fsevents "~2.3.2" -rollup@^2.45.1: +rollup@^2.45.1, rollup@^2.67.1, rollup@^2.8.0: version "2.79.1" resolved "https://registry.yarnpkg.com/rollup/-/rollup-2.79.1.tgz#bedee8faef7c9f93a2647ac0108748f497f081c7" integrity sha512-uKxbd0IhMZOhjAiD5oAFp7BqvkA4Dv47qpOCtaNvng4HBwdbWtdOh8f5nZNuk2rp51PMGk3bzfWu5oayNEuYnw== optionalDependencies: fsevents "~2.3.2" -rollup@^3.10.0: - version "3.19.1" - resolved "https://registry.yarnpkg.com/rollup/-/rollup-3.19.1.tgz#2b3a31ac1ff9f3afab2e523fa687fef5b0ee20fc" - integrity sha512-lAbrdN7neYCg/8WaoWn/ckzCtz+jr70GFfYdlf50OF7387HTg+wiuiqJRFYawwSPpqfqDNYqK7smY/ks2iAudg== - optionalDependencies: - fsevents "~2.3.2" - -rollup@^3.20.2: +rollup@^3.10.0, rollup@^3.20.2, rollup@^3.7.0: version "3.20.2" resolved "https://registry.yarnpkg.com/rollup/-/rollup-3.20.2.tgz#f798c600317f216de2e4ad9f4d9ab30a89b690ff" integrity sha512-3zwkBQl7Ai7MFYQE0y1MeQ15+9jsi7XxfrqwTb/9EK8D9C9+//EBR4M+CuA1KODRaNbFez/lWxA5vhEGZp4MUg== optionalDependencies: fsevents "~2.3.2" -rollup@^3.7.0: - version "3.18.0" - resolved "https://registry.yarnpkg.com/rollup/-/rollup-3.18.0.tgz#2354ba63ba66d6a09c652c3ea0dbcd9dad72bbde" - integrity sha512-J8C6VfEBjkvYPESMQYxKHxNOh4A5a3FlP+0BETGo34HEcE4eTlgCrO2+eWzlu2a/sHs2QUkZco+wscH7jhhgWg== - optionalDependencies: - fsevents "~2.3.2" - rsvp@^3.0.14, rsvp@^3.0.17, rsvp@^3.0.18, rsvp@^3.0.21, rsvp@^3.0.6, rsvp@^3.1.0: version "3.6.2" resolved "https://registry.yarnpkg.com/rsvp/-/rsvp-3.6.2.tgz#2e96491599a96cde1b515d5674a8f7a91452926a" @@ -27017,13 +26999,13 @@ vite-node@0.29.2: picocolors "^1.0.0" vite "^3.0.0 || ^4.0.0" -vite@4.0.0: - version "4.0.0" - resolved "https://registry.yarnpkg.com/vite/-/vite-4.0.0.tgz#b81b88349a06b2faaa53ae14cf96c942548e3454" - integrity sha512-ynad+4kYs8Jcnn8J7SacS9vAbk7eMy0xWg6E7bAhS1s79TK+D7tVFGXVZ55S7RNLRROU1rxoKlvZ/qjaB41DGA== +vite@4.0.5: + version "4.0.5" + resolved "https://registry.yarnpkg.com/vite/-/vite-4.0.5.tgz#634f0bd1edf8bb8468ed42a1c3fd938c67d2f94b" + integrity sha512-7m87RC+caiAxG+8j3jObveRLqaWA/neAdCat6JAZwMkSWqFHOvg8MYe5fAQxVBRAuKAQ1S6XDh3CBQuLNbY33w== dependencies: esbuild "^0.16.3" - postcss "^8.4.19" + postcss "^8.4.20" resolve "^1.22.1" rollup "^3.7.0" optionalDependencies: From 8ffde2a653eb089f33e5f648bd0eeca2176a5f36 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 6 Jun 2023 10:02:05 -0400 Subject: [PATCH 08/38] fix(replay): Ignore max session life for buffered sessions (#8258) Theres an edge case where a buffered session becomes expired, an error comes in, the link from error to replay is lost because a new session is created due to the session being expired. We should either ignore the max session life for a buffered session, or possibly check/refresh session when an error comes in. --- packages/replay/src/replay.ts | 16 ++- packages/replay/src/session/getSession.ts | 8 +- .../replay/src/util/handleRecordingEmit.ts | 4 + .../errorSampleRate-delayFlush.test.ts | 117 ++++++++++++++++-- .../test/integration/errorSampleRate.test.ts | 69 ++++++++--- .../test/unit/session/getSession.test.ts | 59 +++++++++ 6 files changed, 236 insertions(+), 37 deletions(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index e6f7e2128538..a00b96f23c19 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -403,6 +403,8 @@ export class ReplayContainer implements ReplayContainerInterface { return this.flushImmediate(); } + const activityTime = Date.now(); + // Allow flush to complete before resuming as a session recording, otherwise // the checkout from `startRecording` may be included in the payload. // Prefer to keep the error replay as a separate (and smaller) segment @@ -424,6 +426,18 @@ export class ReplayContainer implements ReplayContainerInterface { // Once this session ends, we do not want to refresh it if (this.session) { this.session.shouldRefresh = false; + + // It's possible that the session lifespan is > max session lifespan + // because we have been buffering beyond max session lifespan (we ignore + // expiration given that `shouldRefresh` is true). Since we flip + // `shouldRefresh`, the session could be considered expired due to + // lifespan, which is not what we want. Update session start date to be + // the current timestamp, so that session is not considered to be + // expired. This means that max replay duration can be MAX_SESSION_LIFE + + // (length of buffer), which we are ok with. + this._updateUserActivity(activityTime); + this._updateSessionActivity(activityTime); + this.session.started = activityTime; this._maybeSaveSession(); } @@ -689,7 +703,7 @@ export class ReplayContainer implements ReplayContainerInterface { stickySession: Boolean(this._options.stickySession), currentSession: this.session, sessionSampleRate: this._options.sessionSampleRate, - allowBuffering: this._options.errorSampleRate > 0, + allowBuffering: this._options.errorSampleRate > 0 || this.recordingMode === 'buffer', }); // If session was newly created (i.e. was not loaded from storage), then diff --git a/packages/replay/src/session/getSession.ts b/packages/replay/src/session/getSession.ts index ff993887e64b..73554a8860de 100644 --- a/packages/replay/src/session/getSession.ts +++ b/packages/replay/src/session/getSession.ts @@ -34,11 +34,13 @@ export function getSession({ // within "max session time"). const isExpired = isSessionExpired(session, timeouts); - if (!isExpired) { + if (!isExpired || (allowBuffering && session.shouldRefresh)) { return { type: 'saved', session }; } else if (!session.shouldRefresh) { - // In this case, stop - // This is the case if we have an error session that is completed (=triggered an error) + // This is the case if we have an error session that is completed + // (=triggered an error). Session will continue as session-based replay, + // and when this session is expired, it will not be renewed until user + // reloads. const discardedSession = makeSession({ sampled: false }); return { type: 'new', session: discardedSession }; } else { diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index e4d507d33456..ffb0a993dc89 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -89,6 +89,10 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa // a previous session ID. In this case, we want to buffer events // for a set amount of time before flushing. This can help avoid // capturing replays of users that immediately close the window. + // TODO: We should check `recordingMode` here and do nothing if it's + // buffer, instead of checking inside of timeout, this will make our + // tests a bit cleaner as we will need to wait on the delay in order to + // do nothing. setTimeout(() => replay.conditionalFlush(), options._experiments.delayFlushOnCheckout); // Cancel any previously debounced flushes to ensure there are no [near] diff --git a/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts b/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts index 20645b1b85a4..f691d8e953c1 100644 --- a/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts +++ b/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts @@ -573,6 +573,7 @@ describe('Integration | errorSampleRate with delayed flush', () => { it('has correct timestamps when error occurs much later than initial pageload/checkout', async () => { const ELAPSED = BUFFER_CHECKOUT_TIME; + const TICK = 20; const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockRecord._emitter(TEST_EVENT); @@ -593,26 +594,29 @@ describe('Integration | errorSampleRate with delayed flush', () => { const optionsEvent = createOptionsEvent(replay); jest.runAllTimers(); - jest.advanceTimersByTime(20); await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + captureException(new Error('testing')); await waitForBufferFlush(); - expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 20); + // See comments in `handleRecordingEmit.ts`, we perform a setTimeout into a + // noop when it can be skipped altogether + expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + DEFAULT_FLUSH_MIN_DELAY + TICK + TICK); // Does not capture mouse click expect(replay).toHaveSentReplay({ recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ // Make sure the old performance event is thrown out - replay_start_timestamp: (BASE_TIMESTAMP + ELAPSED + 20) / 1000, + replay_start_timestamp: (BASE_TIMESTAMP + ELAPSED + TICK) / 1000, }), recordingData: JSON.stringify([ { data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + ELAPSED + 20, + timestamp: BASE_TIMESTAMP + ELAPSED + TICK, type: 2, }, optionsEvent, @@ -662,7 +666,8 @@ describe('Integration | errorSampleRate with delayed flush', () => { expect(replay.isEnabled()).toBe(false); }); - it('stops replay when session exceeds max length', async () => { + it('stops replay when session exceeds max length after latest captured error', async () => { + const sessionId = replay.session?.id; jest.setSystemTime(BASE_TIMESTAMP); const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; @@ -674,34 +679,120 @@ describe('Integration | errorSampleRate with delayed flush', () => { jest.runAllTimers(); await new Promise(process.nextTick); + jest.advanceTimersByTime(2 * MAX_SESSION_LIFE); + captureException(new Error('testing')); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + // Flush due to exception + await new Promise(process.nextTick); + await waitForFlush(); + + expect(replay.session?.id).toBe(sessionId); + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + }); + + // This comes from `startRecording()` in `sendBufferedReplayOrFlush()` + await waitForFlush(); + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + recordingData: JSON.stringify([ + { + data: { + isCheckout: true, + }, + timestamp: BASE_TIMESTAMP + 2 * MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 40, + type: 2, + }, + ]), + }); + + // Now wait after session expires - should stop recording + mockRecord.takeFullSnapshot.mockClear(); + (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); + + jest.advanceTimersByTime(MAX_SESSION_LIFE); + await new Promise(process.nextTick); + + mockRecord._emitter(TEST_EVENT); + jest.runAllTimers(); await new Promise(process.nextTick); expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(false); + + // Once the session is stopped after capturing a replay already + // (buffer-mode), another error will not trigger a new replay + captureException(new Error('testing')); - // Wait a bit, shortly before session expires - jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000); await new Promise(process.nextTick); + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + }); + it('does not stop replay based on earliest event in buffer', async () => { + jest.setSystemTime(BASE_TIMESTAMP); + + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP - 60000, type: 3 }; mockRecord._emitter(TEST_EVENT); - replay.triggerUserActivity(); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(replay).not.toHaveLastSentReplay(); + captureException(new Error('testing')); + + await waitForBufferFlush(); expect(replay).toHaveLastSentReplay(); + // Flush from calling `stopRecording` + await waitForFlush(); + // Now wait after session expires - should stop recording mockRecord.takeFullSnapshot.mockClear(); (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); - jest.advanceTimersByTime(10_000); + expect(replay).not.toHaveLastSentReplay(); + + const TICKS = 80; + + // We advance time so that we are on the border of expiring, taking into + // account that TEST_EVENT timestamp is 60000 ms before BASE_TIMESTAMP. The + // 3 DEFAULT_FLUSH_MIN_DELAY is to account for the `waitForFlush` that has + // happened, and for the next two that will happen. The first following + // `waitForFlush` does not expire session, but the following one will. + jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION - 60000 - 3 * DEFAULT_FLUSH_MIN_DELAY - TICKS); await new Promise(process.nextTick); mockRecord._emitter(TEST_EVENT); - replay.triggerUserActivity(); + expect(replay).not.toHaveLastSentReplay(); + await waitForFlush(); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(true); + + mockRecord._emitter(TEST_EVENT); + expect(replay).not.toHaveLastSentReplay(); + await waitForFlush(); + + expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(true); + + // It's hard to test, but if we advance the below time less 1 ms, it should + // be enabled, but we can't trigger a session check via flush without + // incurring another DEFAULT_FLUSH_MIN_DELAY timeout. + jest.advanceTimersByTime(60000 - DEFAULT_FLUSH_MIN_DELAY); + mockRecord._emitter(TEST_EVENT); + expect(replay).not.toHaveLastSentReplay(); + await waitForFlush(); expect(replay).not.toHaveLastSentReplay(); expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 3145ba37e7f9..ea1825dd8429 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -432,6 +432,9 @@ describe('Integration | errorSampleRate', () => { ['MAX_SESSION_LIFE', MAX_SESSION_LIFE], ['SESSION_IDLE_EXPIRE_DURATION', SESSION_IDLE_EXPIRE_DURATION], ])('continues buffering replay if session had no error and exceeds %s', async (_label, waitTime) => { + const oldSessionId = replay.session?.id; + expect(oldSessionId).toBeDefined(); + expect(replay).not.toHaveLastSentReplay(); // Idle for given time @@ -475,13 +478,24 @@ describe('Integration | errorSampleRate', () => { jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); await new Promise(process.nextTick); - expect(replay).toHaveLastSentReplay({ + expect(replay.session?.id).toBe(oldSessionId); + + // Flush of buffered events + expect(replay).toHaveSentReplay({ recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', }), }); + // Checkout from `startRecording` + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + expect(replay.isEnabled()).toBe(true); expect(replay.isPaused()).toBe(false); expect(replay.recordingMode).toBe('session'); @@ -491,6 +505,9 @@ describe('Integration | errorSampleRate', () => { // Should behave the same as above test it('stops replay if user has been idle for more than SESSION_IDLE_EXPIRE_DURATION and does not start a new session thereafter', async () => { + const oldSessionId = replay.session?.id; + expect(oldSessionId).toBeDefined(); + // Idle for 15 minutes jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1); @@ -517,14 +534,24 @@ describe('Integration | errorSampleRate', () => { await new Promise(process.nextTick); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); await new Promise(process.nextTick); + expect(replay.session?.id).toBe(oldSessionId); - expect(replay).toHaveLastSentReplay({ + // buffered events + expect(replay).toHaveSentReplay({ recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', }), }); + // `startRecording` full checkout + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + expect(replay.isEnabled()).toBe(true); expect(replay.isPaused()).toBe(false); expect(replay.recordingMode).toBe('session'); @@ -595,17 +622,15 @@ describe('Integration | errorSampleRate', () => { const optionsEvent = createOptionsEvent(replay); jest.runAllTimers(); - jest.advanceTimersByTime(20); await new Promise(process.nextTick); captureException(new Error('testing')); await new Promise(process.nextTick); jest.runAllTimers(); - jest.advanceTimersByTime(20); await new Promise(process.nextTick); - expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 20); + expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 40); // Does not capture mouse click expect(replay).toHaveSentReplay({ @@ -667,7 +692,8 @@ describe('Integration | errorSampleRate', () => { expect(replay.isEnabled()).toBe(false); }); - it('stops replay when session exceeds max length', async () => { + it('stops replay when session exceeds max length after latest captured error', async () => { + const sessionId = replay.session?.id; jest.setSystemTime(BASE_TIMESTAMP); const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; @@ -679,37 +705,40 @@ describe('Integration | errorSampleRate', () => { jest.runAllTimers(); await new Promise(process.nextTick); + jest.advanceTimersByTime(2 * MAX_SESSION_LIFE); + captureException(new Error('testing')); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + // Flush due to exception await new Promise(process.nextTick); - expect(replay).not.toHaveLastSentReplay(); - - // Wait a bit, shortly before session expires - jest.advanceTimersByTime(MAX_SESSION_LIFE - 1000); + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); await new Promise(process.nextTick); - - mockRecord._emitter(TEST_EVENT); - replay.triggerUserActivity(); - + expect(replay.session?.id).toBe(sessionId); expect(replay).toHaveLastSentReplay(); - // Now wait after session expires - should stop recording + // Now wait after session expires - should re-start into buffering mode mockRecord.takeFullSnapshot.mockClear(); (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); - jest.advanceTimersByTime(10_000); + jest.advanceTimersByTime(MAX_SESSION_LIFE); await new Promise(process.nextTick); mockRecord._emitter(TEST_EVENT); - replay.triggerUserActivity(); - - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + jest.runAllTimers(); await new Promise(process.nextTick); expect(replay).not.toHaveLastSentReplay(); expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); expect(replay.isEnabled()).toBe(false); + + // Once the session is stopped after capturing a replay already + // (buffer-mode), another error should trigger a new replay + captureException(new Error('testing')); + + await new Promise(process.nextTick); + jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); + await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); }); }); diff --git a/packages/replay/test/unit/session/getSession.test.ts b/packages/replay/test/unit/session/getSession.test.ts index 2905e1bd72d6..aa3110d114f2 100644 --- a/packages/replay/test/unit/session/getSession.test.ts +++ b/packages/replay/test/unit/session/getSession.test.ts @@ -229,4 +229,63 @@ describe('Unit | session | getSession', () => { expect(session.id).toBe('test_session_uuid_2'); expect(session.segmentId).toBe(0); }); + + it('re-uses the same "buffer" session if it is expired and has never sent a buffered replay', function () { + const { type, session } = getSession({ + timeouts: { + sessionIdlePause: SESSION_IDLE_PAUSE_DURATION, + sessionIdleExpire: 1000, + maxSessionLife: MAX_SESSION_LIFE, + }, + stickySession: false, + ...SAMPLE_OPTIONS, + currentSession: makeSession({ + id: 'test_session_uuid_2', + lastActivity: +new Date() - MAX_SESSION_LIFE - 1, + started: +new Date() - MAX_SESSION_LIFE - 1, + segmentId: 0, + sampled: 'buffer', + }), + allowBuffering: true, + }); + + expect(FetchSession.fetchSession).not.toHaveBeenCalled(); + expect(CreateSession.createSession).not.toHaveBeenCalled(); + + expect(type).toBe('saved'); + expect(session.id).toBe('test_session_uuid_2'); + expect(session.sampled).toBe('buffer'); + expect(session.segmentId).toBe(0); + }); + + it('creates a new session if it is expired and it was a "buffer" session that has sent a replay', function () { + const currentSession = makeSession({ + id: 'test_session_uuid_2', + lastActivity: +new Date() - MAX_SESSION_LIFE - 1, + started: +new Date() - MAX_SESSION_LIFE - 1, + segmentId: 0, + sampled: 'buffer', + }); + currentSession.shouldRefresh = false; + + const { type, session } = getSession({ + timeouts: { + sessionIdlePause: SESSION_IDLE_PAUSE_DURATION, + sessionIdleExpire: 1000, + maxSessionLife: MAX_SESSION_LIFE, + }, + stickySession: false, + ...SAMPLE_OPTIONS, + currentSession, + allowBuffering: true, + }); + + expect(FetchSession.fetchSession).not.toHaveBeenCalled(); + expect(CreateSession.createSession).not.toHaveBeenCalled(); + + expect(type).toBe('new'); + expect(session.id).not.toBe('test_session_uuid_2'); + expect(session.sampled).toBe(false); + expect(session.segmentId).toBe(0); + }); }); From 1b375173046f9a583327e4e95d0b7990c8a3e247 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 7 Jun 2023 15:08:52 +0200 Subject: [PATCH 09/38] feat(replay): Capture slow clicks (GA) (#8298) This moves the slow click detection out of GA and makes it generally available. You can opt-out of this by setting `slowClickTimeout: 0`. It also adds `disabled` & `aria-disabled` attributes to captured DOM elements. note: We now capture this for ``, ` + Link Link external @@ -69,6 +70,9 @@

Bottom

console.log('DONE'); }, 3001); }); + document.getElementById('mouseDownButton').addEventListener('mousedown', () => { + document.getElementById('out').innerHTML += 'mutationButton clicked
'; + }); // Do nothing on these elements document diff --git a/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts index fef742681614..7e94e0b68f15 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts @@ -38,8 +38,10 @@ sentryTest('mutation after timeout results in slow click', async ({ getLocalTest expect(slowClickBreadcrumbs).toEqual([ { category: 'ui.slowClickDetected', + type: 'default', data: { endReason: 'timeout', + clickCount: 1, node: { attributes: { id: 'mutationButtonLate', @@ -93,8 +95,10 @@ sentryTest('console.log results in slow click', async ({ getLocalTestUrl, page } expect(slowClickBreadcrumbs).toEqual([ { category: 'ui.slowClickDetected', + type: 'default', data: { endReason: 'timeout', + clickCount: 1, node: { attributes: { id: 'consoleLogButton', diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index 1801c34a4e8e..120079ebb857 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -42,3 +42,5 @@ export const CONSOLE_ARG_MAX_SIZE = 5_000; export const SLOW_CLICK_THRESHOLD = 3_000; /* For scroll actions after a click, we only look for a very short time period to detect programmatic scrolling. */ export const SLOW_CLICK_SCROLL_TIMEOUT = 300; +/* Clicks in this time period are considered e.g. double/triple clicks. */ +export const MULTI_CLICK_TIMEOUT = 1_000; diff --git a/packages/replay/src/coreHandlers/handleClick.ts b/packages/replay/src/coreHandlers/handleClick.ts new file mode 100644 index 000000000000..5a75413b00d1 --- /dev/null +++ b/packages/replay/src/coreHandlers/handleClick.ts @@ -0,0 +1,306 @@ +import type { Breadcrumb } from '@sentry/types'; + +import { WINDOW } from '../constants'; +import type { MultiClickFrame, ReplayClickDetector, ReplayContainer, SlowClickConfig, SlowClickFrame } from '../types'; +import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; +import { getClickTargetNode } from './util/domUtils'; + +type ClickBreadcrumb = Breadcrumb & { + timestamp: number; +}; + +interface Click { + timestamp: number; + mutationAfter?: number; + scrollAfter?: number; + clickBreadcrumb: ClickBreadcrumb; + clickCount: number; + node: HTMLElement; +} + +/** Handle a click. */ +export function handleClick(clickDetector: ReplayClickDetector, clickBreadcrumb: Breadcrumb, node: HTMLElement): void { + clickDetector.handleClick(clickBreadcrumb, node); +} + +/** A click detector class that can be used to detect slow or rage clicks on elements. */ +export class ClickDetector implements ReplayClickDetector { + // protected for testing + protected _lastMutation = 0; + protected _lastScroll = 0; + + private _clicks: Click[] = []; + private _teardown: undefined | (() => void); + + private _multiClickTimeout: number; + private _threshold: number; + private _scollTimeout: number; + private _timeout: number; + private _ignoreSelector: string; + + private _replay: ReplayContainer; + private _checkClickTimeout?: ReturnType; + private _addBreadcrumbEvent: typeof addBreadcrumbEvent; + + public constructor( + replay: ReplayContainer, + slowClickConfig: SlowClickConfig, + // Just for easier testing + _addBreadcrumbEvent = addBreadcrumbEvent, + ) { + // We want everything in s, but options are in ms + this._timeout = slowClickConfig.timeout / 1000; + this._multiClickTimeout = slowClickConfig.multiClickTimeout / 1000; + this._threshold = slowClickConfig.threshold / 1000; + this._scollTimeout = slowClickConfig.scrollTimeout / 1000; + this._replay = replay; + this._ignoreSelector = slowClickConfig.ignoreSelector; + this._addBreadcrumbEvent = _addBreadcrumbEvent; + } + + /** Register click detection handlers on mutation or scroll. */ + public addListeners(): void { + const mutationHandler = (): void => { + this._lastMutation = nowInSeconds(); + }; + + const scrollHandler = (): void => { + this._lastScroll = nowInSeconds(); + }; + + const clickHandler = (event: MouseEvent): void => { + if (!event.target) { + return; + } + + const node = getClickTargetNode(event); + if (node) { + this._handleMultiClick(node as HTMLElement); + } + }; + + const obs = new MutationObserver(mutationHandler); + + obs.observe(WINDOW.document.documentElement, { + attributes: true, + characterData: true, + childList: true, + subtree: true, + }); + + WINDOW.addEventListener('scroll', scrollHandler, { passive: true }); + WINDOW.addEventListener('click', clickHandler, { passive: true }); + + this._teardown = () => { + WINDOW.removeEventListener('scroll', scrollHandler); + WINDOW.removeEventListener('click', clickHandler); + + obs.disconnect(); + this._clicks = []; + this._lastMutation = 0; + this._lastScroll = 0; + }; + } + + /** Clean up listeners. */ + public removeListeners(): void { + if (this._teardown) { + this._teardown(); + } + + if (this._checkClickTimeout) { + clearTimeout(this._checkClickTimeout); + } + } + + /** Handle a click */ + public handleClick(breadcrumb: Breadcrumb, node: HTMLElement): void { + if (ignoreElement(node, this._ignoreSelector) || !isClickBreadcrumb(breadcrumb)) { + return; + } + + const click = this._getClick(node); + + if (click) { + // this means a click on the same element was captured in the last 1s, so we consider this a multi click + return; + } + + const newClick: Click = { + timestamp: breadcrumb.timestamp, + clickBreadcrumb: breadcrumb, + // Set this to 0 so we know it originates from the click breadcrumb + clickCount: 0, + node, + }; + this._clicks.push(newClick); + + // If this is the first new click, set a timeout to check for multi clicks + if (this._clicks.length === 1) { + this._scheduleCheckClicks(); + } + } + + /** Count multiple clicks on elements. */ + private _handleMultiClick(node: HTMLElement): void { + const click = this._getClick(node); + + if (!click) { + return; + } + + click.clickCount++; + } + + /** Try to get an existing click on the given element. */ + private _getClick(node: HTMLElement): Click | undefined { + const now = nowInSeconds(); + + // Find any click on the same element in the last second + // If one exists, we consider this click as a double/triple/etc click + return this._clicks.find(click => click.node === node && now - click.timestamp < this._multiClickTimeout); + } + + /** Check the clicks that happened. */ + private _checkClicks(): void { + const timedOutClicks: Click[] = []; + + const now = nowInSeconds(); + + this._clicks.forEach(click => { + if (!click.mutationAfter && this._lastMutation) { + click.mutationAfter = click.timestamp <= this._lastMutation ? this._lastMutation - click.timestamp : undefined; + } + if (!click.scrollAfter && this._lastScroll) { + click.scrollAfter = click.timestamp <= this._lastScroll ? this._lastScroll - click.timestamp : undefined; + } + + // If an action happens after the multi click threshold, we can skip waiting and handle the click right away + const actionTime = click.scrollAfter || click.mutationAfter || 0; + if (actionTime && actionTime >= this._multiClickTimeout) { + timedOutClicks.push(click); + return; + } + + if (click.timestamp + this._timeout <= now) { + timedOutClicks.push(click); + } + }); + + // Remove "old" clicks + for (const click of timedOutClicks) { + this._generateBreadcrumbs(click); + + const pos = this._clicks.indexOf(click); + if (pos !== -1) { + this._clicks.splice(pos, 1); + } + } + + // Trigger new check, unless no clicks left + if (this._clicks.length) { + this._scheduleCheckClicks(); + } + } + + /** Generate matching breadcrumb(s) for the click. */ + private _generateBreadcrumbs(click: Click): void { + const replay = this._replay; + const hadScroll = click.scrollAfter && click.scrollAfter <= this._scollTimeout; + const hadMutation = click.mutationAfter && click.mutationAfter <= this._threshold; + + const isSlowClick = !hadScroll && !hadMutation; + const { clickCount, clickBreadcrumb } = click; + + // Slow click + if (isSlowClick) { + // If `mutationAfter` is set, it means a mutation happened after the threshold, but before the timeout + // If not, it means we just timed out without scroll & mutation + const timeAfterClickMs = Math.min(click.mutationAfter || this._timeout, this._timeout) * 1000; + const endReason = timeAfterClickMs < this._timeout * 1000 ? 'mutation' : 'timeout'; + + const breadcrumb: SlowClickFrame = { + type: 'default', + message: clickBreadcrumb.message, + timestamp: clickBreadcrumb.timestamp, + category: 'ui.slowClickDetected', + data: { + ...clickBreadcrumb.data, + url: WINDOW.location.href, + route: replay.getCurrentRoute(), + timeAfterClickMs, + endReason, + // If clickCount === 0, it means multiClick was not correctly captured here + // - we still want to send 1 in this case + clickCount: clickCount || 1, + }, + }; + + this._addBreadcrumbEvent(replay, breadcrumb); + return; + } + + // Multi click + if (clickCount > 1) { + const breadcrumb: MultiClickFrame = { + type: 'default', + message: clickBreadcrumb.message, + timestamp: clickBreadcrumb.timestamp, + category: 'ui.multiClick', + data: { + ...clickBreadcrumb.data, + url: WINDOW.location.href, + route: replay.getCurrentRoute(), + clickCount, + metric: true, + }, + }; + + this._addBreadcrumbEvent(replay, breadcrumb); + } + } + + /** Schedule to check current clicks. */ + private _scheduleCheckClicks(): void { + this._checkClickTimeout = setTimeout(() => this._checkClicks(), 1000); + } +} + +const SLOW_CLICK_TAGS = ['A', 'BUTTON', 'INPUT']; + +/** exported for tests only */ +export function ignoreElement(node: HTMLElement, ignoreSelector: string): boolean { + if (!SLOW_CLICK_TAGS.includes(node.tagName)) { + return true; + } + + // If tag, we only want to consider input[type='submit'] & input[type='button'] + if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) { + return true; + } + + // If tag, detect special variants that may not lead to an action + // If target !== _self, we may open the link somewhere else, which would lead to no action + // Also, when downloading a file, we may not leave the page, but still not trigger an action + if ( + node.tagName === 'A' && + (node.hasAttribute('download') || (node.hasAttribute('target') && node.getAttribute('target') !== '_self')) + ) { + return true; + } + + if (ignoreSelector && node.matches(ignoreSelector)) { + return true; + } + + return false; +} + +function isClickBreadcrumb(breadcrumb: Breadcrumb): breadcrumb is ClickBreadcrumb { + return !!(breadcrumb.data && typeof breadcrumb.data.nodeId === 'number' && breadcrumb.timestamp); +} + +// This is good enough for us, and is easier to test/mock than `timestampInSeconds` +function nowInSeconds(): number { + return Date.now() / 1000; +} diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 54ab7ec8bb09..71fb211ee3fe 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -3,32 +3,17 @@ import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; import { htmlTreeAsString } from '@sentry/utils'; -import { SLOW_CLICK_SCROLL_TIMEOUT, SLOW_CLICK_THRESHOLD } from '../constants'; -import type { ReplayContainer, SlowClickConfig } from '../types'; +import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; -import { detectSlowClick } from './handleSlowClick'; +import { handleClick } from './handleClick'; import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; +import type { DomHandlerData } from './util/domUtils'; +import { getClickTargetNode, getTargetNode } from './util/domUtils'; import { getAttributesToRecord } from './util/getAttributesToRecord'; -export interface DomHandlerData { - name: string; - event: Node | { target: EventTarget }; -} - export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHandlerData) => void = ( replay: ReplayContainer, ) => { - const { slowClickTimeout, slowClickIgnoreSelectors } = replay.getOptions(); - - const slowClickConfig: SlowClickConfig | undefined = slowClickTimeout - ? { - threshold: Math.min(SLOW_CLICK_THRESHOLD, slowClickTimeout), - timeout: slowClickTimeout, - scrollTimeout: SLOW_CLICK_SCROLL_TIMEOUT, - ignoreSelector: slowClickIgnoreSelectors ? slowClickIgnoreSelectors.join(',') : '', - } - : undefined; - return (handlerData: DomHandlerData): void => { if (!replay.isEnabled()) { return; @@ -43,11 +28,10 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa const isClick = handlerData.name === 'click'; const event = isClick && (handlerData.event as PointerEvent); // Ignore clicks if ctrl/alt/meta keys are held down as they alter behavior of clicks (e.g. open in new tab) - if (isClick && slowClickConfig && event && !event.altKey && !event.metaKey && !event.ctrlKey) { - detectSlowClick( - replay, - slowClickConfig, - result as Breadcrumb & { timestamp: number }, + if (isClick && replay.clickDetector && event && !event.altKey && !event.metaKey && !event.ctrlKey) { + handleClick( + replay.clickDetector, + result as Breadcrumb & { timestamp: number; data: { nodeId: number } }, getClickTargetNode(handlerData.event) as HTMLElement, ); } @@ -118,32 +102,3 @@ function getDomTarget(handlerData: DomHandlerData): { target: Node | INode | nul function isRrwebNode(node: EventTarget): node is INode { return '__sn' in node; } - -function getTargetNode(event: Node | { target: EventTarget | null }): Node | INode | null { - if (isEventWithTarget(event)) { - return event.target as Node | null; - } - - return event; -} - -const INTERACTIVE_SELECTOR = 'button,a'; - -// For clicks, we check if the target is inside of a button or link -// If so, we use this as the target instead -// This is useful because if you click on the image in , -// The target will be the image, not the button, which we don't want here -function getClickTargetNode(event: DomHandlerData['event']): Node | INode | null { - const target = getTargetNode(event); - - if (!target || !(target instanceof Element)) { - return target; - } - - const closestInteractive = target.closest(INTERACTIVE_SELECTOR); - return closestInteractive || target; -} - -function isEventWithTarget(event: unknown): event is { target: EventTarget | null } { - return typeof event === 'object' && !!event && 'target' in event; -} diff --git a/packages/replay/src/coreHandlers/handleSlowClick.ts b/packages/replay/src/coreHandlers/handleSlowClick.ts deleted file mode 100644 index c939a990f87a..000000000000 --- a/packages/replay/src/coreHandlers/handleSlowClick.ts +++ /dev/null @@ -1,145 +0,0 @@ -import type { Breadcrumb } from '@sentry/types'; - -import { WINDOW } from '../constants'; -import type { ReplayContainer, SlowClickConfig } from '../types'; -import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; - -type ClickBreadcrumb = Breadcrumb & { - timestamp: number; -}; - -/** - * Detect a slow click on a button/a tag, - * and potentially create a corresponding breadcrumb. - */ -export function detectSlowClick( - replay: ReplayContainer, - config: SlowClickConfig, - clickBreadcrumb: ClickBreadcrumb, - node: HTMLElement, -): void { - if (ignoreElement(node, config)) { - return; - } - - /* - We consider a slow click a click on a button/a, which does not trigger one of: - - DOM mutation - - Scroll (within 100ms) - Within the given threshold time. - After time timeout time, we stop listening and mark it as a slow click anyhow. - */ - - let cleanup: () => void = () => { - // replaced further down - }; - - // After timeout time, def. consider this a slow click, and stop watching for mutations - const timeout = setTimeout(() => { - handleSlowClick(replay, clickBreadcrumb, config.timeout, 'timeout'); - cleanup(); - }, config.timeout); - - const mutationHandler = (): void => { - maybeHandleSlowClick(replay, clickBreadcrumb, config.threshold, config.timeout, 'mutation'); - cleanup(); - }; - - const scrollHandler = (): void => { - maybeHandleSlowClick(replay, clickBreadcrumb, config.scrollTimeout, config.timeout, 'scroll'); - cleanup(); - }; - - const obs = new MutationObserver(mutationHandler); - - obs.observe(WINDOW.document.documentElement, { - attributes: true, - characterData: true, - childList: true, - subtree: true, - }); - - WINDOW.addEventListener('scroll', scrollHandler); - - // Stop listening to scroll timeouts early - const scrollTimeout = setTimeout(() => { - WINDOW.removeEventListener('scroll', scrollHandler); - }, config.scrollTimeout); - - cleanup = (): void => { - clearTimeout(timeout); - clearTimeout(scrollTimeout); - obs.disconnect(); - WINDOW.removeEventListener('scroll', scrollHandler); - }; -} - -function maybeHandleSlowClick( - replay: ReplayContainer, - clickBreadcrumb: ClickBreadcrumb, - threshold: number, - timeout: number, - endReason: string, -): boolean { - const now = Date.now(); - const timeAfterClickMs = now - clickBreadcrumb.timestamp * 1000; - - if (timeAfterClickMs > threshold) { - handleSlowClick(replay, clickBreadcrumb, Math.min(timeAfterClickMs, timeout), endReason); - return true; - } - - return false; -} - -function handleSlowClick( - replay: ReplayContainer, - clickBreadcrumb: ClickBreadcrumb, - timeAfterClickMs: number, - endReason: string, -): void { - const breadcrumb = { - message: clickBreadcrumb.message, - timestamp: clickBreadcrumb.timestamp, - category: 'ui.slowClickDetected', - data: { - ...clickBreadcrumb.data, - url: WINDOW.location.href, - // TODO FN: add parametrized route, when possible - timeAfterClickMs, - endReason, - }, - }; - - addBreadcrumbEvent(replay, breadcrumb); -} - -const SLOW_CLICK_TAGS = ['A', 'BUTTON', 'INPUT']; - -/** exported for tests only */ -export function ignoreElement(node: HTMLElement, config: SlowClickConfig): boolean { - if (!SLOW_CLICK_TAGS.includes(node.tagName)) { - return true; - } - - // If tag, we only want to consider input[type='submit'] & input[type='button'] - if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) { - return true; - } - - // If tag, detect special variants that may not lead to an action - // If target !== _self, we may open the link somewhere else, which would lead to no action - // Also, when downloading a file, we may not leave the page, but still not trigger an action - if ( - node.tagName === 'A' && - (node.hasAttribute('download') || (node.hasAttribute('target') && node.getAttribute('target') !== '_self')) - ) { - return true; - } - - if (config.ignoreSelector && node.matches(config.ignoreSelector)) { - return true; - } - - return false; -} diff --git a/packages/replay/src/coreHandlers/util/domUtils.ts b/packages/replay/src/coreHandlers/util/domUtils.ts new file mode 100644 index 000000000000..6091dc7fe837 --- /dev/null +++ b/packages/replay/src/coreHandlers/util/domUtils.ts @@ -0,0 +1,38 @@ +import type { INode } from '@sentry-internal/rrweb-snapshot'; + +export interface DomHandlerData { + name: string; + event: Node | { target: EventTarget }; +} + +const INTERACTIVE_SELECTOR = 'button,a'; + +/** + * For clicks, we check if the target is inside of a button or link + * If so, we use this as the target instead + * This is useful because if you click on the image in , + * The target will be the image, not the button, which we don't want here + */ +export function getClickTargetNode(event: DomHandlerData['event'] | MouseEvent): Node | INode | null { + const target = getTargetNode(event); + + if (!target || !(target instanceof Element)) { + return target; + } + + const closestInteractive = target.closest(INTERACTIVE_SELECTOR); + return closestInteractive || target; +} + +/** Get the event target node. */ +export function getTargetNode(event: Node | { target: EventTarget | null }): Node | INode | null { + if (isEventWithTarget(event)) { + return event.target as Node | null; + } + + return event; +} + +function isEventWithTarget(event: unknown): event is { target: EventTarget | null } { + return typeof event === 'object' && !!event && 'target' in event; +} diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index a00b96f23c19..acb2980e608c 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -7,10 +7,14 @@ import { logger } from '@sentry/utils'; import { BUFFER_CHECKOUT_TIME, MAX_SESSION_LIFE, + MULTI_CLICK_TIMEOUT, SESSION_IDLE_EXPIRE_DURATION, SESSION_IDLE_PAUSE_DURATION, + SLOW_CLICK_SCROLL_TIMEOUT, + SLOW_CLICK_THRESHOLD, WINDOW, } from './constants'; +import { ClickDetector } from './coreHandlers/handleClick'; import { handleKeyboardEvent } from './coreHandlers/handleKeyboardEvent'; import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; import { createEventBuffer } from './eventBuffer'; @@ -31,6 +35,7 @@ import type { ReplayPluginOptions, SendBufferedReplayOptions, Session, + SlowClickConfig, Timeouts, } from './types'; import { addEvent } from './util/addEvent'; @@ -60,6 +65,8 @@ export class ReplayContainer implements ReplayContainerInterface { public session: Session | undefined; + public clickDetector: ClickDetector | undefined; + /** * Recording can happen in one of three modes: * - session: Record the whole session, sending it continuously @@ -159,6 +166,22 @@ export class ReplayContainer implements ReplayContainerInterface { // ... per 5s 5, ); + + const { slowClickTimeout, slowClickIgnoreSelectors } = this.getOptions(); + + const slowClickConfig: SlowClickConfig | undefined = slowClickTimeout + ? { + threshold: Math.min(SLOW_CLICK_THRESHOLD, slowClickTimeout), + timeout: slowClickTimeout, + scrollTimeout: SLOW_CLICK_SCROLL_TIMEOUT, + ignoreSelector: slowClickIgnoreSelectors ? slowClickIgnoreSelectors.join(',') : '', + multiClickTimeout: MULTI_CLICK_TIMEOUT, + } + : undefined; + + if (slowClickConfig) { + this.clickDetector = new ClickDetector(this, slowClickConfig); + } } /** Get the event context. */ @@ -737,6 +760,10 @@ export class ReplayContainer implements ReplayContainerInterface { WINDOW.addEventListener('focus', this._handleWindowFocus); WINDOW.addEventListener('keydown', this._handleKeyboardEvent); + if (this.clickDetector) { + this.clickDetector.addListeners(); + } + // There is no way to remove these listeners, so ensure they are only added once if (!this._hasInitializedCoreListeners) { addGlobalListeners(this); @@ -766,6 +793,10 @@ export class ReplayContainer implements ReplayContainerInterface { WINDOW.removeEventListener('focus', this._handleWindowFocus); WINDOW.removeEventListener('keydown', this._handleKeyboardEvent); + if (this.clickDetector) { + this.clickDetector.removeListeners(); + } + if (this._performanceObserver) { this._performanceObserver.disconnect(); this._performanceObserver = null; diff --git a/packages/replay/src/types/replay.ts b/packages/replay/src/types/replay.ts index 2ec6e18346ee..f058b2c9011a 100644 --- a/packages/replay/src/types/replay.ts +++ b/packages/replay/src/types/replay.ts @@ -1,4 +1,5 @@ import type { + Breadcrumb, FetchBreadcrumbHint, HandlerDataFetch, ReplayRecordingData, @@ -407,8 +408,15 @@ export interface SendBufferedReplayOptions { continueRecording?: boolean; } +export interface ReplayClickDetector { + addListeners(): void; + removeListeners(): void; + handleClick(breadcrumb: Breadcrumb, node: HTMLElement): void; +} + export interface ReplayContainer { eventBuffer: EventBuffer | null; + clickDetector: ReplayClickDetector | undefined; performanceEvents: AllPerformanceEntry[]; session: Session | undefined; recordingMode: ReplayRecordingMode; @@ -468,4 +476,5 @@ export interface SlowClickConfig { timeout: number; scrollTimeout: number; ignoreSelector: string; + multiClickTimeout: number; } diff --git a/packages/replay/src/types/replayFrame.ts b/packages/replay/src/types/replayFrame.ts index f0107fdbf77a..379dbab91605 100644 --- a/packages/replay/src/types/replayFrame.ts +++ b/packages/replay/src/types/replayFrame.ts @@ -88,14 +88,28 @@ interface FocusFrame extends BaseBreadcrumbFrame { interface SlowClickFrameData extends ClickFrameData { url: string; - timeAfterClickFs: number; + route?: string; + timeAfterClickMs: number; endReason: string; + clickCount: number; } -interface SlowClickFrame extends BaseBreadcrumbFrame { +export interface SlowClickFrame extends BaseBreadcrumbFrame { category: 'ui.slowClickDetected'; data: SlowClickFrameData; } +interface MultiClickFrameData extends ClickFrameData { + url: string; + route?: string; + clickCount: number; + metric: true; +} + +export interface MultiClickFrame extends BaseBreadcrumbFrame { + category: 'ui.multiClick'; + data: MultiClickFrameData; +} + interface OptionFrame { blockAllMedia: boolean; errorSampleRate: number; @@ -118,6 +132,7 @@ export type BreadcrumbFrame = | BlurFrame | FocusFrame | SlowClickFrame + | MultiClickFrame | MutationFrame | BaseBreadcrumbFrame; diff --git a/packages/replay/test/unit/coreHandlers/handleClick.test.ts b/packages/replay/test/unit/coreHandlers/handleClick.test.ts new file mode 100644 index 000000000000..8b06e4683656 --- /dev/null +++ b/packages/replay/test/unit/coreHandlers/handleClick.test.ts @@ -0,0 +1,542 @@ +import type { Breadcrumb } from '@sentry/types'; + +import { BASE_TIMESTAMP } from '../..'; +import { ClickDetector, ignoreElement } from '../../../src/coreHandlers/handleClick'; +import type { ReplayContainer } from '../../../src/types'; + +jest.useFakeTimers(); + +describe('Unit | coreHandlers | handleClick', () => { + describe('ClickDetector', () => { + beforeEach(() => { + jest.setSystemTime(BASE_TIMESTAMP); + }); + + test('it captures a single click', async () => { + const replay = { + getCurrentRoute: () => 'test-route', + } as ReplayContainer; + + const mockAddBreadcrumbEvent = jest.fn(); + + const detector = new ClickDetector( + replay, + { + threshold: 1_000, + timeout: 3_000, + scrollTimeout: 200, + ignoreSelector: '', + multiClickTimeout: 1_000, + }, + mockAddBreadcrumbEvent, + ); + + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + clickCount: 1, + endReason: 'timeout', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 3000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: expect.any(Number), + }); + + jest.advanceTimersByTime(5_000); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + }); + + test('it groups multiple clicks together', async () => { + const replay = { + getCurrentRoute: () => 'test-route', + } as ReplayContainer; + + const mockAddBreadcrumbEvent = jest.fn(); + + const detector = new ClickDetector( + replay, + { + threshold: 1_000, + timeout: 3_000, + scrollTimeout: 200, + ignoreSelector: '', + multiClickTimeout: 1_000, + }, + mockAddBreadcrumbEvent, + ); + + const breadcrumb1: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const breadcrumb2: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000 + 0.2, + data: { + nodeId: 1, + }, + }; + const breadcrumb3: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000 + 0.6, + data: { + nodeId: 1, + }, + }; + const breadcrumb4: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000 + 2, + data: { + nodeId: 1, + }, + }; + const breadcrumb5: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000 + 2.9, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb1, node); + + detector.handleClick(breadcrumb2, node); + + detector.handleClick(breadcrumb3, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(2_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + detector.handleClick(breadcrumb4, node); + detector.handleClick(breadcrumb5, node); + + jest.advanceTimersByTime(1_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + // count is not actually correct, because this is identified by a different click handler + clickCount: 1, + endReason: 'timeout', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 3000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: expect.any(Number), + }); + + jest.advanceTimersByTime(2_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(2); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + // count is not actually correct, because this is identified by a different click handler + clickCount: 1, + endReason: 'timeout', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 3000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: expect.any(Number), + }); + + jest.advanceTimersByTime(5_000); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(2); + }); + + test('it captures clicks on different elements', async () => { + const replay = { + getCurrentRoute: () => 'test-route', + } as ReplayContainer; + + const mockAddBreadcrumbEvent = jest.fn(); + + const detector = new ClickDetector( + replay, + { + threshold: 1_000, + timeout: 3_000, + scrollTimeout: 200, + ignoreSelector: '', + multiClickTimeout: 1_000, + }, + mockAddBreadcrumbEvent, + ); + + const breadcrumb1: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const breadcrumb2: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 2, + }, + }; + const breadcrumb3: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 3, + }, + }; + const node1 = document.createElement('button'); + const node2 = document.createElement('button'); + const node3 = document.createElement('button'); + detector.handleClick(breadcrumb1, node1); + detector.handleClick(breadcrumb2, node2); + detector.handleClick(breadcrumb3, node3); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(3_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(3); + + jest.advanceTimersByTime(5_000); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(3); + }); + + test('it ignores clicks on ignored elements', async () => { + const replay = { + getCurrentRoute: () => 'test-route', + } as ReplayContainer; + + const mockAddBreadcrumbEvent = jest.fn(); + + const detector = new ClickDetector( + replay, + { + threshold: 1_000, + timeout: 3_000, + scrollTimeout: 200, + ignoreSelector: '', + multiClickTimeout: 1_000, + }, + mockAddBreadcrumbEvent, + ); + + const breadcrumb1: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const breadcrumb2: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 2, + }, + }; + const breadcrumb3: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 3, + }, + }; + const node1 = document.createElement('div'); + const node2 = document.createElement('div'); + const node3 = document.createElement('div'); + detector.handleClick(breadcrumb1, node1); + detector.handleClick(breadcrumb2, node2); + detector.handleClick(breadcrumb3, node3); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(3_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + }); + + describe('mutations', () => { + let detector: ClickDetector; + let mockAddBreadcrumbEvent = jest.fn(); + + const replay = { + getCurrentRoute: () => 'test-route', + } as ReplayContainer; + + beforeEach(() => { + jest.setSystemTime(BASE_TIMESTAMP); + + mockAddBreadcrumbEvent = jest.fn(); + + detector = new ClickDetector( + replay, + { + threshold: 1_000, + timeout: 3_000, + scrollTimeout: 200, + ignoreSelector: '', + multiClickTimeout: 1_000, + }, + mockAddBreadcrumbEvent, + ); + }); + + test('it does not consider clicks with mutation before threshold as slow click', async () => { + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(500); + + // Pretend a mutation happend + detector['_lastMutation'] = BASE_TIMESTAMP / 1000 + 0.5; + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(3_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + }); + + test('it considers clicks with mutation after threshold as slow click', async () => { + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + // Pretend a mutation happend + detector['_lastMutation'] = BASE_TIMESTAMP / 1000 + 2; + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(3_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + clickCount: 1, + endReason: 'mutation', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 2000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: expect.any(Number), + }); + + jest.advanceTimersByTime(5_000); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + }); + + test('it caps timeout', async () => { + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(1_000); + + // Pretend a mutation happend + detector['_lastMutation'] = BASE_TIMESTAMP / 1000 + 5; + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(5_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + clickCount: 1, + endReason: 'timeout', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 3000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: expect.any(Number), + }); + + jest.advanceTimersByTime(5_000); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + }); + }); + + describe('scroll', () => { + let detector: ClickDetector; + let mockAddBreadcrumbEvent = jest.fn(); + + const replay = { + getCurrentRoute: () => 'test-route', + } as ReplayContainer; + + beforeEach(() => { + jest.setSystemTime(BASE_TIMESTAMP); + + mockAddBreadcrumbEvent = jest.fn(); + + detector = new ClickDetector( + replay, + { + threshold: 1_000, + timeout: 3_000, + scrollTimeout: 200, + ignoreSelector: '', + multiClickTimeout: 1_000, + }, + mockAddBreadcrumbEvent, + ); + }); + + test('it does not consider clicks with scroll before threshold as slow click', async () => { + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(100); + + // Pretend a mutation happend + detector['_lastScroll'] = BASE_TIMESTAMP / 1000 + 0.15; + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(3_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + }); + + test('it considers clicks with scroll after threshold as slow click', async () => { + const breadcrumb: Breadcrumb = { + timestamp: BASE_TIMESTAMP / 1000, + data: { + nodeId: 1, + }, + }; + const node = document.createElement('button'); + detector.handleClick(breadcrumb, node); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(300); + + // Pretend a mutation happend + detector['_lastScroll'] = BASE_TIMESTAMP / 1000 + 0.3; + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(0); + + jest.advanceTimersByTime(3_000); + + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledWith(replay, { + category: 'ui.slowClickDetected', + type: 'default', + data: { + clickCount: 1, + endReason: 'timeout', + nodeId: 1, + route: 'test-route', + timeAfterClickMs: 3000, + url: 'http://localhost/', + }, + message: undefined, + timestamp: expect.any(Number), + }); + + jest.advanceTimersByTime(5_000); + expect(mockAddBreadcrumbEvent).toHaveBeenCalledTimes(1); + }); + }); + }); + + describe('ignoreElement', () => { + it.each([ + ['div', {}, true], + ['button', {}, false], + ['a', {}, false], + ['input', {}, true], + ['input', { type: 'text' }, true], + ['input', { type: 'button' }, false], + ['input', { type: 'submit' }, false], + ['a', { target: '_self' }, false], + ['a', { target: '_blank' }, true], + ['a', { download: '' }, true], + ['a', { href: 'xx' }, false], + ])('it works with <%s> & %p', (tagName, attributes, expected) => { + const node = document.createElement(tagName); + Object.entries(attributes).forEach(([key, value]) => { + node.setAttribute(key, value); + }); + expect(ignoreElement(node, '')).toBe(expected); + }); + + test('it ignored selectors matching ignoreSelector', () => { + const button = document.createElement('button'); + const a = document.createElement('a'); + + expect(ignoreElement(button, 'button')).toBe(true); + expect(ignoreElement(a, 'button')).toBe(false); + }); + }); +}); diff --git a/packages/replay/test/unit/coreHandlers/handleDom.test.ts b/packages/replay/test/unit/coreHandlers/handleDom.test.ts index dc1fff0b5ff2..99fa5dc1e367 100644 --- a/packages/replay/test/unit/coreHandlers/handleDom.test.ts +++ b/packages/replay/test/unit/coreHandlers/handleDom.test.ts @@ -1,5 +1,5 @@ -import type { DomHandlerData } from '../../../src/coreHandlers/handleDom'; import { handleDom } from '../../../src/coreHandlers/handleDom'; +import type { DomHandlerData } from '../../../src/coreHandlers/util/domUtils'; describe('Unit | coreHandlers | handleDom', () => { test('it works with a basic click event on a div', () => { diff --git a/packages/replay/test/unit/coreHandlers/handleSlowClick.test.ts b/packages/replay/test/unit/coreHandlers/handleSlowClick.test.ts deleted file mode 100644 index 2d0922272115..000000000000 --- a/packages/replay/test/unit/coreHandlers/handleSlowClick.test.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { ignoreElement } from '../../../src/coreHandlers/handleSlowClick'; -import type { SlowClickConfig } from '../../../src/types'; - -describe('Unit | coreHandlers | handleSlowClick', () => { - describe('ignoreElement', () => { - it.each([ - ['div', {}, true], - ['button', {}, false], - ['a', {}, false], - ['input', {}, true], - ['input', { type: 'text' }, true], - ['input', { type: 'button' }, false], - ['input', { type: 'submit' }, false], - ['a', { target: '_self' }, false], - ['a', { target: '_blank' }, true], - ['a', { download: '' }, true], - ['a', { href: 'xx' }, false], - ])('it works with <%s> & %p', (tagName, attributes, expected) => { - const node = document.createElement(tagName); - Object.entries(attributes).forEach(([key, value]) => { - node.setAttribute(key, value); - }); - expect(ignoreElement(node, {} as SlowClickConfig)).toBe(expected); - }); - - test('it ignored selectors matching ignoreSelector', () => { - const button = document.createElement('button'); - const a = document.createElement('a'); - - expect(ignoreElement(button, { ignoreSelector: 'button' } as SlowClickConfig)).toBe(true); - expect(ignoreElement(a, { ignoreSelector: 'button' } as SlowClickConfig)).toBe(false); - }); - }); -}); From a4c858ff5557d8f088b30e88740a6c86377bbe93 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 16 Jun 2023 13:28:20 +0200 Subject: [PATCH 25/38] fix(core): Temporarily store debug IDs in stack frame and only put them into `debug_meta` before sending (#8347) --- packages/core/src/utils/prepareEvent.ts | 64 ++++++++++++++----- packages/core/test/lib/prepareEvent.test.ts | 71 ++++++++++++++++----- 2 files changed, 104 insertions(+), 31 deletions(-) diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 474a95c7823b..84bfd404c56f 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -38,9 +38,9 @@ export function prepareEvent( applyClientOptions(prepared, options); applyIntegrationsMetadata(prepared, integrations); - // Only apply debug metadata to error events. + // Only put debug IDs onto frames for error events. if (event.type === undefined) { - applyDebugMetadata(prepared, options.stackParser); + applyDebugIds(prepared, options.stackParser); } // If we have scope given to us, use it as the base for further modifications. @@ -75,6 +75,14 @@ export function prepareEvent( } return result.then(evt => { + if (evt) { + // We apply the debug_meta field only after all event processors have ran, so that if any event processors modified + // file names (e.g.the RewriteFrames integration) the filename -> debug ID relationship isn't destroyed. + // This should not cause any PII issues, since we're only moving data that is already on the event and not adding + // any new data + applyDebugMeta(evt); + } + if (typeof normalizeDepth === 'number' && normalizeDepth > 0) { return normalizeEvent(evt, normalizeDepth, normalizeMaxBreadth); } @@ -121,9 +129,9 @@ function applyClientOptions(event: Event, options: ClientOptions): void { const debugIdStackParserCache = new WeakMap>(); /** - * Applies debug metadata images to the event in order to apply source maps by looking up their debug ID. + * Puts debug IDs into the stack frames of an error event. */ -export function applyDebugMetadata(event: Event, stackParser: StackParser): void { +export function applyDebugIds(event: Event, stackParser: StackParser): void { const debugIdMap = GLOBAL_OBJ._sentryDebugIds; if (!debugIdMap) { @@ -160,15 +168,39 @@ export function applyDebugMetadata(event: Event, stackParser: StackParser): void return acc; }, {}); - // Get a Set of filenames in the stack trace - const errorFileNames = new Set(); try { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion event!.exception!.values!.forEach(exception => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion exception.stacktrace!.frames!.forEach(frame => { if (frame.filename) { - errorFileNames.add(frame.filename); + frame.debug_id = filenameDebugIdMap[frame.filename]; + } + }); + }); + } catch (e) { + // To save bundle size we're just try catching here instead of checking for the existence of all the different objects. + } +} + +/** + * Moves debug IDs from the stack frames of an error event into the debug_meta field. + */ +export function applyDebugMeta(event: Event): void { + // Extract debug IDs and filenames from the stack frames on the event. + const filenameDebugIdMap: Record = {}; + try { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + event.exception!.values!.forEach(exception => { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + exception.stacktrace!.frames!.forEach(frame => { + if (frame.debug_id) { + if (frame.abs_path) { + filenameDebugIdMap[frame.abs_path] = frame.debug_id; + } else if (frame.filename) { + filenameDebugIdMap[frame.filename] = frame.debug_id; + } + delete frame.debug_id; } }); }); @@ -176,18 +208,20 @@ export function applyDebugMetadata(event: Event, stackParser: StackParser): void // To save bundle size we're just try catching here instead of checking for the existence of all the different objects. } + if (Object.keys(filenameDebugIdMap).length === 0) { + return; + } + // Fill debug_meta information event.debug_meta = event.debug_meta || {}; event.debug_meta.images = event.debug_meta.images || []; const images = event.debug_meta.images; - errorFileNames.forEach(filename => { - if (filenameDebugIdMap[filename]) { - images.push({ - type: 'sourcemap', - code_file: filename, - debug_id: filenameDebugIdMap[filename], - }); - } + Object.keys(filenameDebugIdMap).forEach(filename => { + images.push({ + type: 'sourcemap', + code_file: filename, + debug_id: filenameDebugIdMap[filename], + }); }); } diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index d08b4ace221d..1e8b53e60f8c 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -1,14 +1,14 @@ import type { Event } from '@sentry/types'; import { createStackParser, GLOBAL_OBJ } from '@sentry/utils'; -import { applyDebugMetadata } from '../../src/utils/prepareEvent'; +import { applyDebugIds, applyDebugMeta } from '../../src/utils/prepareEvent'; -describe('applyDebugMetadata', () => { +describe('applyDebugIds', () => { afterEach(() => { GLOBAL_OBJ._sentryDebugIds = undefined; }); - it('should put debug source map images in debug_meta field', () => { + it("should put debug IDs into an event's stack frames", () => { GLOBAL_OBJ._sentryDebugIds = { 'filename1.js\nfilename1.js': 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa', 'filename2.js\nfilename2.js': 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb', @@ -34,35 +34,74 @@ describe('applyDebugMetadata', () => { }, }; - applyDebugMetadata(event, stackParser); + applyDebugIds(event, stackParser); - expect(event.debug_meta?.images).toContainEqual({ - type: 'sourcemap', - code_file: 'filename1.js', + expect(event.exception?.values?.[0].stacktrace?.frames).toContainEqual({ + filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa', }); - expect(event.debug_meta?.images).toContainEqual({ - type: 'sourcemap', - code_file: 'filename2.js', + expect(event.exception?.values?.[0].stacktrace?.frames).toContainEqual({ + filename: 'filename2.js', debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb', }); // expect not to contain an image for the stack frame that doesn't have a corresponding debug id - expect(event.debug_meta?.images).not.toContainEqual( + expect(event.exception?.values?.[0].stacktrace?.frames).not.toContainEqual( expect.objectContaining({ - type: 'sourcemap', - code_file: 'filename3.js', + filename3: 'filename3.js', + debug_id: expect.any(String), }), ); // expect not to contain an image for the debug id mapping that isn't contained in the stack trace - expect(event.debug_meta?.images).not.toContainEqual( + expect(event.exception?.values?.[0].stacktrace?.frames).not.toContainEqual( expect.objectContaining({ - type: 'sourcemap', - code_file: 'filename4.js', + filename3: 'filename4.js', debug_id: 'cccccccc-cccc-4ccc-cccc-cccccccccc', }), ); }); }); + +describe('applyDebugMeta', () => { + it("should move the debug IDs inside an event's stack frame into the debug_meta field", () => { + const event: Event = { + exception: { + values: [ + { + stacktrace: { + frames: [ + { filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' }, + { filename: 'filename2.js', debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb' }, + { filename: 'filename1.js', debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa' }, + { filename: 'filename3.js' }, + ], + }, + }, + ], + }, + }; + + applyDebugMeta(event); + + expect(event.exception?.values?.[0].stacktrace?.frames).toEqual([ + { filename: 'filename1.js' }, + { filename: 'filename2.js' }, + { filename: 'filename1.js' }, + { filename: 'filename3.js' }, + ]); + + expect(event.debug_meta?.images).toContainEqual({ + type: 'sourcemap', + code_file: 'filename1.js', + debug_id: 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaa', + }); + + expect(event.debug_meta?.images).toContainEqual({ + type: 'sourcemap', + code_file: 'filename2.js', + debug_id: 'bbbbbbbb-bbbb-4bbb-bbbb-bbbbbbbbbb', + }); + }); +}); From e6ea5375bfa45bebf6b46861169ecf848f2e7762 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 19 Jun 2023 12:53:53 +0200 Subject: [PATCH 26/38] fix(vue): Don't call `next` in Vue router 4 instrumentation (#8351) Vue Router 4 no longer exposes a `next` resolver function to call inside a `beforeEach` [router guard callback](https://router.vuejs.org/guide/advanced/navigation-guards.html#global-before-guards). Instead it will resolve the hook when the callback function returns. This PR checks if `next` is available and only calls it then. Also it adjusts the types accordingly and adds a test that previously failed. fixes #8349 --- packages/vue/src/router.ts | 9 +++++++-- packages/vue/test/router.test.ts | 28 +++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index d8f87f14ad5d..25408f24f6b6 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -39,7 +39,7 @@ export type Route = { interface VueRouter { onError: (fn: (err: Error) => void) => void; - beforeEach: (fn: (to: Route, from: Route, next: () => void) => void) => void; + beforeEach: (fn: (to: Route, from: Route, next?: () => void) => void) => void; } /** @@ -129,7 +129,12 @@ export function vueRouterInstrumentation( }); } - next(); + // Vue Router 4 no longer exposes the `next` function, so we need to + // check if it's available before calling it. + // `next` needs to be called in Vue Router 3 so that the hook is resolved. + if (next) { + next(); + } }); }; } diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index 2e8fd9a01d0d..044228181f50 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -9,7 +9,7 @@ const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException'); const mockVueRouter = { onError: jest.fn void]>(), - beforeEach: jest.fn void) => void]>(), + beforeEach: jest.fn void) => void]>(), }; const mockStartTransaction = jest.fn(); @@ -324,4 +324,30 @@ describe('vueRouterInstrumentation()', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(expectedCallsAmount + 1); }, ); + + it("doesn't throw when `next` is not available in the beforeEach callback (Vue Router 4)", () => { + const instrument = vueRouterInstrumentation(mockVueRouter, { routeLabel: 'path' }); + instrument(mockStartTransaction, true, true); + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes.normalRoute1; + const to = testRoutes.namedRoute; + beforeEachCallback(to, from, undefined); + + // first startTx call happens when the instrumentation is initialized (for pageloads) + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/login', + metadata: { + source: 'route', + }, + data: { + params: to.params, + query: to.query, + }, + op: 'navigation', + tags: { + 'routing.instrumentation': 'vue-router', + }, + }); + }); }); From 34bb40317bc0af261205fdddca81542748afcd56 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 19 Jun 2023 13:36:09 +0200 Subject: [PATCH 27/38] feat(replay): Stop replay when event buffer exceeds max. size (#8315) When the buffer exceeds ~20MB, stop the replay. Closes https://github.com/getsentry/sentry-javascript/issues/7657 Closes https://github.com/getsentry/team-replay/issues/94 --- packages/replay/src/constants.ts | 3 + .../src/eventBuffer/EventBufferArray.ts | 12 +++- .../EventBufferCompressionWorker.ts | 18 ++++- packages/replay/src/eventBuffer/index.ts | 8 +++ packages/replay/src/util/addEvent.ts | 5 +- .../unit/eventBuffer/EventBufferArray.test.ts | 55 ++++++++++++++- .../EventBufferCompressionWorker.test.ts | 70 ++++++++++++++++++- .../replay/test/unit/util/addEvent.test.ts | 28 ++++++++ 8 files changed, 192 insertions(+), 7 deletions(-) diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index 120079ebb857..699cefe8b784 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -44,3 +44,6 @@ export const SLOW_CLICK_THRESHOLD = 3_000; export const SLOW_CLICK_SCROLL_TIMEOUT = 300; /* Clicks in this time period are considered e.g. double/triple clicks. */ export const MULTI_CLICK_TIMEOUT = 1_000; + +/** When encountering a total segment size exceeding this size, stop the replay (as we cannot properly ingest it). */ +export const REPLAY_MAX_EVENT_BUFFER_SIZE = 20_000_000; // ~20MB diff --git a/packages/replay/src/eventBuffer/EventBufferArray.ts b/packages/replay/src/eventBuffer/EventBufferArray.ts index eaebd1b174e7..a7b363891026 100644 --- a/packages/replay/src/eventBuffer/EventBufferArray.ts +++ b/packages/replay/src/eventBuffer/EventBufferArray.ts @@ -1,5 +1,7 @@ +import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants'; import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { timestampToMs } from '../util/timestampToMs'; +import { EventBufferSizeExceededError } from '.'; /** * A basic event buffer that does not do any compression. @@ -8,6 +10,7 @@ import { timestampToMs } from '../util/timestampToMs'; export class EventBufferArray implements EventBuffer { /** All the events that are buffered to be sent. */ public events: RecordingEvent[]; + private _totalSize = 0; public constructor() { this.events = []; @@ -30,6 +33,12 @@ export class EventBufferArray implements EventBuffer { /** @inheritdoc */ public async addEvent(event: RecordingEvent): Promise { + const eventSize = JSON.stringify(event).length; + this._totalSize += eventSize; + if (this._totalSize > REPLAY_MAX_EVENT_BUFFER_SIZE) { + throw new EventBufferSizeExceededError(); + } + this.events.push(event); } @@ -40,7 +49,7 @@ export class EventBufferArray implements EventBuffer { // events member so that we do not lose new events while uploading // attachment. const eventsRet = this.events; - this.events = []; + this.clear(); resolve(JSON.stringify(eventsRet)); }); } @@ -48,6 +57,7 @@ export class EventBufferArray implements EventBuffer { /** @inheritdoc */ public clear(): void { this.events = []; + this._totalSize = 0; } /** @inheritdoc */ diff --git a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts index 45696ea46bc9..5b4c0eb4487a 100644 --- a/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts +++ b/packages/replay/src/eventBuffer/EventBufferCompressionWorker.ts @@ -1,7 +1,9 @@ import type { ReplayRecordingData } from '@sentry/types'; +import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants'; import type { AddEventResult, EventBuffer, EventBufferType, RecordingEvent } from '../types'; import { timestampToMs } from '../util/timestampToMs'; +import { EventBufferSizeExceededError } from '.'; import { WorkerHandler } from './WorkerHandler'; /** @@ -11,6 +13,7 @@ import { WorkerHandler } from './WorkerHandler'; export class EventBufferCompressionWorker implements EventBuffer { private _worker: WorkerHandler; private _earliestTimestamp: number | null; + private _totalSize = 0; public constructor(worker: Worker) { this._worker = new WorkerHandler(worker); @@ -53,7 +56,14 @@ export class EventBufferCompressionWorker implements EventBuffer { this._earliestTimestamp = timestamp; } - return this._sendEventToWorker(event); + const data = JSON.stringify(event); + this._totalSize += data.length; + + if (this._totalSize > REPLAY_MAX_EVENT_BUFFER_SIZE) { + return Promise.reject(new EventBufferSizeExceededError()); + } + + return this._sendEventToWorker(data); } /** @@ -66,6 +76,7 @@ export class EventBufferCompressionWorker implements EventBuffer { /** @inheritdoc */ public clear(): void { this._earliestTimestamp = null; + this._totalSize = 0; // We do not wait on this, as we assume the order of messages is consistent for the worker void this._worker.postMessage('clear'); } @@ -78,8 +89,8 @@ export class EventBufferCompressionWorker implements EventBuffer { /** * Send the event to the worker. */ - private _sendEventToWorker(event: RecordingEvent): Promise { - return this._worker.postMessage('addEvent', JSON.stringify(event)); + private _sendEventToWorker(data: string): Promise { + return this._worker.postMessage('addEvent', data); } /** @@ -89,6 +100,7 @@ export class EventBufferCompressionWorker implements EventBuffer { const response = await this._worker.postMessage('finish'); this._earliestTimestamp = null; + this._totalSize = 0; return response; } diff --git a/packages/replay/src/eventBuffer/index.ts b/packages/replay/src/eventBuffer/index.ts index f0eb83c68243..fe58b76f3c7b 100644 --- a/packages/replay/src/eventBuffer/index.ts +++ b/packages/replay/src/eventBuffer/index.ts @@ -1,6 +1,7 @@ import { getWorkerURL } from '@sentry-internal/replay-worker'; import { logger } from '@sentry/utils'; +import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../constants'; import type { EventBuffer } from '../types'; import { EventBufferArray } from './EventBufferArray'; import { EventBufferProxy } from './EventBufferProxy'; @@ -30,3 +31,10 @@ export function createEventBuffer({ useCompression }: CreateEventBufferParams): __DEBUG_BUILD__ && logger.log('[Replay] Using simple buffer'); return new EventBufferArray(); } + +/** This error indicates that the event buffer size exceeded the limit.. */ +export class EventBufferSizeExceededError extends Error { + public constructor() { + super(`Event buffer exceeded maximum size of ${REPLAY_MAX_EVENT_BUFFER_SIZE}.`); + } +} diff --git a/packages/replay/src/util/addEvent.ts b/packages/replay/src/util/addEvent.ts index 16f653e9fc5d..79bf4ff2f362 100644 --- a/packages/replay/src/util/addEvent.ts +++ b/packages/replay/src/util/addEvent.ts @@ -2,6 +2,7 @@ import { EventType } from '@sentry-internal/rrweb'; import { getCurrentHub } from '@sentry/core'; import { logger } from '@sentry/utils'; +import { EventBufferSizeExceededError } from '../eventBuffer'; import type { AddEventResult, RecordingEvent, ReplayContainer, ReplayFrameEvent } from '../types'; import { timestampToMs } from './timestampToMs'; @@ -56,8 +57,10 @@ export async function addEvent( return await replay.eventBuffer.addEvent(eventAfterPossibleCallback); } catch (error) { + const reason = error && error instanceof EventBufferSizeExceededError ? 'addEventSizeExceeded' : 'addEvent'; + __DEBUG_BUILD__ && logger.error(error); - await replay.stop('addEvent'); + await replay.stop(reason); const client = getCurrentHub().getClient(); diff --git a/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts b/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts index 37827610e4cc..c7b0a4bd7e90 100644 --- a/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts +++ b/packages/replay/test/unit/eventBuffer/EventBufferArray.test.ts @@ -1,4 +1,5 @@ -import { createEventBuffer } from './../../../src/eventBuffer'; +import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../../../src/constants'; +import { createEventBuffer, EventBufferSizeExceededError } from './../../../src/eventBuffer'; import { BASE_TIMESTAMP } from './../../index'; const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; @@ -44,4 +45,56 @@ describe('Unit | eventBuffer | EventBufferArray', () => { expect(result1).toEqual(JSON.stringify([TEST_EVENT])); expect(result2).toEqual(JSON.stringify([])); }); + + describe('size limit', () => { + it('rejects if size exceeds limit', async function () { + const buffer = createEventBuffer({ useCompression: false }); + + const largeEvent = { + data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + + await buffer.addEvent(largeEvent); + await buffer.addEvent(largeEvent); + + // Now it should error + await expect(() => buffer.addEvent(largeEvent)).rejects.toThrowError(EventBufferSizeExceededError); + }); + + it('resets size limit on clear', async function () { + const buffer = createEventBuffer({ useCompression: false }); + + const largeEvent = { + data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + + await buffer.addEvent(largeEvent); + await buffer.addEvent(largeEvent); + + await buffer.clear(); + + await buffer.addEvent(largeEvent); + }); + + it('resets size limit on finish', async function () { + const buffer = createEventBuffer({ useCompression: false }); + + const largeEvent = { + data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + + await buffer.addEvent(largeEvent); + await buffer.addEvent(largeEvent); + + await buffer.finish(); + + await buffer.addEvent(largeEvent); + }); + }); }); diff --git a/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts b/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts index 74819a2cdf75..6c3e5948fac1 100644 --- a/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts +++ b/packages/replay/test/unit/eventBuffer/EventBufferCompressionWorker.test.ts @@ -3,8 +3,9 @@ import 'jsdom-worker'; import pako from 'pako'; import { BASE_TIMESTAMP } from '../..'; +import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../../../src/constants'; import { EventBufferProxy } from '../../../src/eventBuffer/EventBufferProxy'; -import { createEventBuffer } from './../../../src/eventBuffer'; +import { createEventBuffer, EventBufferSizeExceededError } from './../../../src/eventBuffer'; const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; @@ -146,4 +147,71 @@ describe('Unit | eventBuffer | EventBufferCompressionWorker', () => { await expect(() => buffer.addEvent({ data: { o: 3 }, timestamp: BASE_TIMESTAMP, type: 3 })).rejects.toBeDefined(); }); + + describe('size limit', () => { + it('rejects if size exceeds limit', async function () { + const buffer = createEventBuffer({ + useCompression: true, + }) as EventBufferProxy; + + expect(buffer).toBeInstanceOf(EventBufferProxy); + await buffer.ensureWorkerIsLoaded(); + + const largeEvent = { + data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + + await buffer.addEvent(largeEvent); + await buffer.addEvent(largeEvent); + + // Now it should error + await expect(() => buffer.addEvent(largeEvent)).rejects.toThrowError(EventBufferSizeExceededError); + }); + + it('resets size limit on clear', async function () { + const buffer = createEventBuffer({ + useCompression: true, + }) as EventBufferProxy; + + expect(buffer).toBeInstanceOf(EventBufferProxy); + await buffer.ensureWorkerIsLoaded(); + + const largeEvent = { + data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + + await buffer.addEvent(largeEvent); + await buffer.addEvent(largeEvent); + + await buffer.clear(); + + await buffer.addEvent(largeEvent); + }); + + it('resets size limit on finish', async function () { + const buffer = createEventBuffer({ + useCompression: true, + }) as EventBufferProxy; + + expect(buffer).toBeInstanceOf(EventBufferProxy); + await buffer.ensureWorkerIsLoaded(); + + const largeEvent = { + data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + + await buffer.addEvent(largeEvent); + await buffer.addEvent(largeEvent); + + await buffer.finish(); + + await buffer.addEvent(largeEvent); + }); + }); }); diff --git a/packages/replay/test/unit/util/addEvent.test.ts b/packages/replay/test/unit/util/addEvent.test.ts index 6cc2e6b6ffdf..f00dc82d338f 100644 --- a/packages/replay/test/unit/util/addEvent.test.ts +++ b/packages/replay/test/unit/util/addEvent.test.ts @@ -1,6 +1,7 @@ import 'jsdom-worker'; import { BASE_TIMESTAMP } from '../..'; +import { REPLAY_MAX_EVENT_BUFFER_SIZE } from '../../../src/constants'; import type { EventBufferProxy } from '../../../src/eventBuffer/EventBufferProxy'; import { addEvent } from '../../../src/util/addEvent'; import { setupReplayContainer } from '../../utils/setupReplayContainer'; @@ -29,4 +30,31 @@ describe('Unit | util | addEvent', () => { expect(replay.isEnabled()).toEqual(false); }); + + it('stops when exceeding buffer size limit', async function () { + jest.setSystemTime(BASE_TIMESTAMP); + + const replay = setupReplayContainer({ + options: { + useCompression: true, + }, + }); + + const largeEvent = { + data: { a: 'a'.repeat(REPLAY_MAX_EVENT_BUFFER_SIZE / 3) }, + timestamp: BASE_TIMESTAMP, + type: 3, + }; + + await (replay.eventBuffer as EventBufferProxy).ensureWorkerIsLoaded(); + + await addEvent(replay, largeEvent); + await addEvent(replay, largeEvent); + + expect(replay.isEnabled()).toEqual(true); + + await addEvent(replay, largeEvent); + + expect(replay.isEnabled()).toEqual(false); + }); }); From 1bdf2d942eabc696e4dc8e2df69b3b7c80b9e279 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 19 Jun 2023 14:56:30 +0200 Subject: [PATCH 28/38] feat(replay): Consider `window.open` for slow clicks (#8308) When a click triggers `window.open()`, do not consider it a slow click. Closes https://github.com/getsentry/sentry-javascript/issues/8301 --- .../suites/replay/slowClick/template.html | 4 ++ .../replay/slowClick/windowOpen/test.ts | 69 +++++++++++++++++++ .../replay/src/coreHandlers/handleClick.ts | 7 ++ .../src/coreHandlers/util/onWindowOpen.ts | 44 ++++++++++++ 4 files changed, 124 insertions(+) create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts create mode 100644 packages/replay/src/coreHandlers/util/onWindowOpen.ts diff --git a/packages/browser-integration-tests/suites/replay/slowClick/template.html b/packages/browser-integration-tests/suites/replay/slowClick/template.html index f49c8b1d410d..19bd283db90e 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/template.html +++ b/packages/browser-integration-tests/suites/replay/slowClick/template.html @@ -19,6 +19,7 @@ + Link Link external @@ -73,6 +74,9 @@

Bottom

document.getElementById('mouseDownButton').addEventListener('mousedown', () => { document.getElementById('out').innerHTML += 'mutationButton clicked
'; }); + document.getElementById('windowOpenButton').addEventListener('click', () => { + window.open('https://example.com/', '_blank'); + }); // Do nothing on these elements document diff --git a/packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts new file mode 100644 index 000000000000..4c9401234ea2 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts @@ -0,0 +1,69 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +sentryTest('window.open() is considered for slow click', async ({ getLocalTestUrl, page, browser }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + + 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; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + // Ensure window.open() still works as expected + const context = browser.contexts()[0]; + const waitForNewPage = context.waitForEvent('page'); + + await page.click('#windowOpenButton'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + // Filter out potential blur breadcrumb, as otherwise this can be flaky + const filteredBreadcrumb = breadcrumbs.filter(breadcrumb => breadcrumb.category !== 'ui.blur'); + + expect(filteredBreadcrumb).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + id: 'windowOpenButton', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '****** ****', + }, + nodeId: expect.any(Number), + }, + message: 'body > button#windowOpenButton', + timestamp: expect.any(Number), + type: 'default', + }, + ]); + + await waitForNewPage; + + const pages = context.pages(); + + expect(pages.length).toBe(2); + expect(pages[1].url()).toBe('https://example.com/'); +}); diff --git a/packages/replay/src/coreHandlers/handleClick.ts b/packages/replay/src/coreHandlers/handleClick.ts index 5a75413b00d1..c5a65ddfdca0 100644 --- a/packages/replay/src/coreHandlers/handleClick.ts +++ b/packages/replay/src/coreHandlers/handleClick.ts @@ -4,6 +4,7 @@ import { WINDOW } from '../constants'; import type { MultiClickFrame, ReplayClickDetector, ReplayContainer, SlowClickConfig, SlowClickFrame } from '../types'; import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; import { getClickTargetNode } from './util/domUtils'; +import { onWindowOpen } from './util/onWindowOpen'; type ClickBreadcrumb = Breadcrumb & { timestamp: number; @@ -68,6 +69,11 @@ export class ClickDetector implements ReplayClickDetector { this._lastScroll = nowInSeconds(); }; + const cleanupWindowOpen = onWindowOpen(() => { + // Treat window.open as mutation + this._lastMutation = nowInSeconds(); + }); + const clickHandler = (event: MouseEvent): void => { if (!event.target) { return; @@ -94,6 +100,7 @@ export class ClickDetector implements ReplayClickDetector { this._teardown = () => { WINDOW.removeEventListener('scroll', scrollHandler); WINDOW.removeEventListener('click', clickHandler); + cleanupWindowOpen(); obs.disconnect(); this._clicks = []; diff --git a/packages/replay/src/coreHandlers/util/onWindowOpen.ts b/packages/replay/src/coreHandlers/util/onWindowOpen.ts new file mode 100644 index 000000000000..e3b6b7ac92ed --- /dev/null +++ b/packages/replay/src/coreHandlers/util/onWindowOpen.ts @@ -0,0 +1,44 @@ +import { fill } from '@sentry/utils'; + +import { WINDOW } from '../../constants'; + +type WindowOpenHandler = () => void; + +let handlers: undefined | WindowOpenHandler[]; + +/** + * Register a handler to be called when `window.open()` is called. + * Returns a cleanup function. + */ +export function onWindowOpen(cb: WindowOpenHandler): () => void { + // Ensure to only register this once + if (!handlers) { + handlers = []; + monkeyPatchWindowOpen(); + } + + handlers.push(cb); + + return () => { + const pos = handlers ? handlers.indexOf(cb) : -1; + if (pos > -1) { + (handlers as WindowOpenHandler[]).splice(pos, 1); + } + }; +} + +function monkeyPatchWindowOpen(): void { + fill(WINDOW, 'open', function (originalWindowOpen: () => void): () => void { + return function (...args: unknown[]): void { + if (handlers) { + try { + handlers.forEach(handler => handler()); + } catch (e) { + // ignore errors in here + } + } + + return originalWindowOpen.apply(WINDOW, args); + }; + }); +} From e41ce248471ca6c7f66be6ec9384950c5ff13e53 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 19 Jun 2023 13:37:43 +0200 Subject: [PATCH 29/38] meta(changelog): Update changelog for 7.56.0 --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ceb218a3a05..da270fb1ccec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.56.0 + +- feat(replay): Rework slow click & multi click detection (#8322) +- feat(replay): Stop replay when event buffer exceeds max. size (#8315) +- feat(replay): Consider `window.open` for slow clicks (#8308) +- fix(core): Temporarily store debug IDs in stack frame and only put them into `debug_meta` before sending (#8347) +- fix(remix): Extract deferred responses correctly in root loaders. (#8305) +- fix(vue): Don't call `next` in Vue router 4 instrumentation (#8351) + ## 7.55.2 - fix(replay): Stop exporting `EventType` from `@sentry-internal/rrweb` (#8334) From 527bea7483f9e20b14a6e5187787f911feaeecd5 Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Mon, 19 Jun 2023 13:28:05 +0000 Subject: [PATCH 30/38] release: 7.56.0 --- lerna.json | 2 +- packages/angular-ivy/package.json | 8 ++++---- packages/angular/package.json | 8 ++++---- packages/browser-integration-tests/package.json | 2 +- packages/browser/package.json | 14 +++++++------- packages/core/package.json | 6 +++--- packages/core/src/version.ts | 2 +- packages/e2e-tests/package.json | 2 +- packages/ember/package.json | 8 ++++---- packages/eslint-config-sdk/package.json | 6 +++--- packages/eslint-plugin-sdk/package.json | 2 +- packages/gatsby/package.json | 10 +++++----- packages/hub/package.json | 8 ++++---- packages/integration-shims/package.json | 4 ++-- packages/integrations/package.json | 8 ++++---- packages/nextjs/package.json | 14 +++++++------- packages/node-integration-tests/package.json | 2 +- packages/node/package.json | 10 +++++----- packages/opentelemetry-node/package.json | 10 +++++----- packages/overhead-metrics/package.json | 2 +- packages/react/package.json | 8 ++++---- packages/remix/package.json | 12 ++++++------ packages/replay-worker/package.json | 2 +- packages/replay/package.json | 10 +++++----- packages/serverless/package.json | 10 +++++----- packages/svelte/package.json | 8 ++++---- packages/sveltekit/package.json | 16 ++++++++-------- packages/tracing-internal/package.json | 8 ++++---- packages/tracing/package.json | 14 +++++++------- packages/types/package.json | 2 +- packages/typescript/package.json | 2 +- packages/utils/package.json | 4 ++-- packages/vue/package.json | 10 +++++----- packages/wasm/package.json | 8 ++++---- 34 files changed, 121 insertions(+), 121 deletions(-) diff --git a/lerna.json b/lerna.json index 3628d7861208..5485606bb926 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "7.55.2", + "version": "7.56.0", "npmClient": "yarn", "useWorkspaces": true } diff --git a/packages/angular-ivy/package.json b/packages/angular-ivy/package.json index 75d9eb5e4626..b18a7e014fe3 100644 --- a/packages/angular-ivy/package.json +++ b/packages/angular-ivy/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/angular-ivy", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Angular with full Ivy Support", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/angular-ivy", @@ -21,9 +21,9 @@ "rxjs": "^6.5.5 || ^7.x" }, "dependencies": { - "@sentry/browser": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/browser": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^2.3.0" }, "devDependencies": { diff --git a/packages/angular/package.json b/packages/angular/package.json index 773361801b90..39a12c3303a7 100644 --- a/packages/angular/package.json +++ b/packages/angular/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/angular", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Angular", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/angular", @@ -21,9 +21,9 @@ "rxjs": "^6.5.5 || ^7.x" }, "dependencies": { - "@sentry/browser": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/browser": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^2.0.0" }, "devDependencies": { diff --git a/packages/browser-integration-tests/package.json b/packages/browser-integration-tests/package.json index 4135f14e0ec8..63e61ebf57b7 100644 --- a/packages/browser-integration-tests/package.json +++ b/packages/browser-integration-tests/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/browser-integration-tests", - "version": "7.55.2", + "version": "7.56.0", "main": "index.js", "license": "MIT", "engines": { diff --git a/packages/browser/package.json b/packages/browser/package.json index 199d52b0f714..d59b14ce1b48 100644 --- a/packages/browser/package.json +++ b/packages/browser/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/browser", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for browsers", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/browser", @@ -16,15 +16,15 @@ "access": "public" }, "dependencies": { - "@sentry-internal/tracing": "7.55.2", - "@sentry/core": "7.55.2", - "@sentry/replay": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry-internal/tracing": "7.56.0", + "@sentry/core": "7.56.0", + "@sentry/replay": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^1.9.3" }, "devDependencies": { - "@sentry-internal/integration-shims": "7.55.2", + "@sentry-internal/integration-shims": "7.56.0", "@types/md5": "2.1.33", "btoa": "^1.2.1", "chai": "^4.1.2", diff --git a/packages/core/package.json b/packages/core/package.json index 8ff612ce2d24..2464552af0f8 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/core", - "version": "7.55.2", + "version": "7.56.0", "description": "Base implementation for all Sentry JavaScript SDKs", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/core", @@ -16,8 +16,8 @@ "access": "public" }, "dependencies": { - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^1.9.3" }, "scripts": { diff --git a/packages/core/src/version.ts b/packages/core/src/version.ts index caf9d9d03f45..c265c7ab0178 100644 --- a/packages/core/src/version.ts +++ b/packages/core/src/version.ts @@ -1 +1 @@ -export const SDK_VERSION = '7.55.2'; +export const SDK_VERSION = '7.56.0'; diff --git a/packages/e2e-tests/package.json b/packages/e2e-tests/package.json index e4b7b6459502..13560617e1b2 100644 --- a/packages/e2e-tests/package.json +++ b/packages/e2e-tests/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/e2e-tests", - "version": "7.55.2", + "version": "7.56.0", "license": "MIT", "engines": { "node": ">=10" diff --git a/packages/ember/package.json b/packages/ember/package.json index f43eaebd7624..2e184fc17a7e 100644 --- a/packages/ember/package.json +++ b/packages/ember/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/ember", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Ember.js", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/ember", @@ -29,9 +29,9 @@ }, "dependencies": { "@embroider/macros": "^1.9.0", - "@sentry/browser": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/browser": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "ember-auto-import": "^1.12.1 || ^2.4.3", "ember-cli-babel": "^7.26.11", "ember-cli-htmlbars": "^6.1.1", diff --git a/packages/eslint-config-sdk/package.json b/packages/eslint-config-sdk/package.json index dca0fab40c70..bc4fb7a8c778 100644 --- a/packages/eslint-config-sdk/package.json +++ b/packages/eslint-config-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/eslint-config-sdk", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK eslint config", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/eslint-config-sdk", @@ -19,8 +19,8 @@ "access": "public" }, "dependencies": { - "@sentry-internal/eslint-plugin-sdk": "7.55.2", - "@sentry-internal/typescript": "7.55.2", + "@sentry-internal/eslint-plugin-sdk": "7.56.0", + "@sentry-internal/typescript": "7.56.0", "@typescript-eslint/eslint-plugin": "^5.48.0", "@typescript-eslint/parser": "^5.48.0", "eslint-config-prettier": "^6.11.0", diff --git a/packages/eslint-plugin-sdk/package.json b/packages/eslint-plugin-sdk/package.json index 9d51c40cd436..12fa738a0be4 100644 --- a/packages/eslint-plugin-sdk/package.json +++ b/packages/eslint-plugin-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/eslint-plugin-sdk", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK eslint plugin", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/eslint-plugin-sdk", diff --git a/packages/gatsby/package.json b/packages/gatsby/package.json index 0af92c72f520..337af4a11f32 100644 --- a/packages/gatsby/package.json +++ b/packages/gatsby/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/gatsby", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Gatsby.js", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/gatsby", @@ -20,10 +20,10 @@ "access": "public" }, "dependencies": { - "@sentry/core": "7.55.2", - "@sentry/react": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/core": "7.56.0", + "@sentry/react": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "@sentry/webpack-plugin": "1.19.0" }, "peerDependencies": { diff --git a/packages/hub/package.json b/packages/hub/package.json index af288ce8d529..ca34143369db 100644 --- a/packages/hub/package.json +++ b/packages/hub/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/hub", - "version": "7.55.2", + "version": "7.56.0", "description": "Sentry hub which handles global state managment.", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/hub", @@ -16,9 +16,9 @@ "access": "public" }, "dependencies": { - "@sentry/core": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/core": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^1.9.3" }, "scripts": { diff --git a/packages/integration-shims/package.json b/packages/integration-shims/package.json index b1a776879cdb..472198466754 100644 --- a/packages/integration-shims/package.json +++ b/packages/integration-shims/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/integration-shims", - "version": "7.55.2", + "version": "7.56.0", "description": "Shims for integrations in Sentry SDK.", "main": "build/cjs/index.js", "module": "build/esm/index.js", @@ -34,7 +34,7 @@ "url": "https://github.com/getsentry/sentry-javascript/issues" }, "dependencies": { - "@sentry/types": "7.55.2" + "@sentry/types": "7.56.0" }, "engines": { "node": ">=12" diff --git a/packages/integrations/package.json b/packages/integrations/package.json index 6077048b5bfc..13e1f5a04343 100644 --- a/packages/integrations/package.json +++ b/packages/integrations/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/integrations", - "version": "7.55.2", + "version": "7.56.0", "description": "Pluggable integrations that can be used to enhance JS SDKs", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/integrations", @@ -16,13 +16,13 @@ "module": "build/npm/esm/index.js", "types": "build/npm/types/index.d.ts", "dependencies": { - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "localforage": "^1.8.1", "tslib": "^1.9.3" }, "devDependencies": { - "@sentry/browser": "7.55.2", + "@sentry/browser": "7.56.0", "chai": "^4.1.2" }, "scripts": { diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 69c4482a065e..69b24dcfbb44 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/nextjs", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Next.js", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/nextjs", @@ -18,12 +18,12 @@ }, "dependencies": { "@rollup/plugin-commonjs": "24.0.0", - "@sentry/core": "7.55.2", - "@sentry/integrations": "7.55.2", - "@sentry/node": "7.55.2", - "@sentry/react": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/core": "7.56.0", + "@sentry/integrations": "7.56.0", + "@sentry/node": "7.56.0", + "@sentry/react": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "@sentry/webpack-plugin": "1.20.0", "chalk": "3.0.0", "rollup": "2.78.0", diff --git a/packages/node-integration-tests/package.json b/packages/node-integration-tests/package.json index bea1e5f93b8c..2c6d3fc29ff1 100644 --- a/packages/node-integration-tests/package.json +++ b/packages/node-integration-tests/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/node-integration-tests", - "version": "7.55.2", + "version": "7.56.0", "license": "MIT", "engines": { "node": ">=10" diff --git a/packages/node/package.json b/packages/node/package.json index 5443313cccaf..c24c033ab70f 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/node", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Node.js", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/node", @@ -16,10 +16,10 @@ "access": "public" }, "dependencies": { - "@sentry-internal/tracing": "7.55.2", - "@sentry/core": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry-internal/tracing": "7.56.0", + "@sentry/core": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "cookie": "^0.4.1", "https-proxy-agent": "^5.0.0", "lru_map": "^0.3.3", diff --git a/packages/opentelemetry-node/package.json b/packages/opentelemetry-node/package.json index b0a3cb75dc59..4cd6cde98e76 100644 --- a/packages/opentelemetry-node/package.json +++ b/packages/opentelemetry-node/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/opentelemetry-node", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for OpenTelemetry Node.js", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/opentelemetry-node", @@ -16,9 +16,9 @@ "access": "public" }, "dependencies": { - "@sentry/core": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2" + "@sentry/core": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0" }, "peerDependencies": { "@opentelemetry/api": "1.x", @@ -32,7 +32,7 @@ "@opentelemetry/sdk-trace-base": "^1.7.0", "@opentelemetry/sdk-trace-node": "^1.7.0", "@opentelemetry/semantic-conventions": "^1.7.0", - "@sentry/node": "7.55.2" + "@sentry/node": "7.56.0" }, "scripts": { "build": "run-p build:transpile build:types", diff --git a/packages/overhead-metrics/package.json b/packages/overhead-metrics/package.json index 621b2806f7bc..910f6b0e001c 100644 --- a/packages/overhead-metrics/package.json +++ b/packages/overhead-metrics/package.json @@ -1,6 +1,6 @@ { "private": true, - "version": "7.55.2", + "version": "7.56.0", "name": "@sentry-internal/overhead-metrics", "main": "index.js", "author": "Sentry", diff --git a/packages/react/package.json b/packages/react/package.json index a89156197031..b0f360019949 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/react", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for React.js", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/react", @@ -16,9 +16,9 @@ "access": "public" }, "dependencies": { - "@sentry/browser": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/browser": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "hoist-non-react-statics": "^3.3.2", "tslib": "^1.9.3" }, diff --git a/packages/remix/package.json b/packages/remix/package.json index 8e389d438a10..e964b237149c 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/remix", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Remix", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/remix", @@ -21,11 +21,11 @@ }, "dependencies": { "@sentry/cli": "2.2.0", - "@sentry/core": "7.55.2", - "@sentry/node": "7.55.2", - "@sentry/react": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/core": "7.56.0", + "@sentry/node": "7.56.0", + "@sentry/react": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^1.9.3", "yargs": "^17.6.0" }, diff --git a/packages/replay-worker/package.json b/packages/replay-worker/package.json index 8bbe38d8a33d..3641e152f07e 100644 --- a/packages/replay-worker/package.json +++ b/packages/replay-worker/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/replay-worker", - "version": "7.55.2", + "version": "7.56.0", "description": "Worker for @sentry/replay", "main": "build/npm/esm/index.js", "module": "build/npm/esm/index.js", diff --git a/packages/replay/package.json b/packages/replay/package.json index d54e53bc59af..f3b1cc69d2b9 100644 --- a/packages/replay/package.json +++ b/packages/replay/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/replay", - "version": "7.55.2", + "version": "7.56.0", "description": "User replays for Sentry", "main": "build/npm/cjs/index.js", "module": "build/npm/esm/index.js", @@ -44,16 +44,16 @@ "homepage": "https://docs.sentry.io/platforms/javascript/session-replay/", "devDependencies": { "@babel/core": "^7.17.5", - "@sentry-internal/replay-worker": "7.55.2", + "@sentry-internal/replay-worker": "7.56.0", "@sentry-internal/rrweb": "1.108.0", "@sentry-internal/rrweb-snapshot": "1.108.0", "jsdom-worker": "^0.2.1", "tslib": "^1.9.3" }, "dependencies": { - "@sentry/core": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2" + "@sentry/core": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0" }, "engines": { "node": ">=12" diff --git a/packages/serverless/package.json b/packages/serverless/package.json index 6bb1777c34d4..167e3ac936f7 100644 --- a/packages/serverless/package.json +++ b/packages/serverless/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/serverless", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for various serverless solutions", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/serverless", @@ -16,10 +16,10 @@ "access": "public" }, "dependencies": { - "@sentry/core": "7.55.2", - "@sentry/node": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/core": "7.56.0", + "@sentry/node": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "@types/aws-lambda": "^8.10.62", "@types/express": "^4.17.14", "tslib": "^1.9.3" diff --git a/packages/svelte/package.json b/packages/svelte/package.json index f9ef94fb1f08..1aa37e2101bf 100644 --- a/packages/svelte/package.json +++ b/packages/svelte/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/svelte", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Svelte", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/svelte", @@ -16,9 +16,9 @@ "access": "public" }, "dependencies": { - "@sentry/browser": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/browser": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "magic-string": "^0.30.0", "tslib": "^1.9.3" }, diff --git a/packages/sveltekit/package.json b/packages/sveltekit/package.json index da1015dcf2c5..ef3a0ccbadc5 100644 --- a/packages/sveltekit/package.json +++ b/packages/sveltekit/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/sveltekit", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for SvelteKit", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/sveltekit", @@ -20,13 +20,13 @@ "@sveltejs/kit": "1.x" }, "dependencies": { - "@sentry-internal/tracing": "7.55.2", - "@sentry/core": "7.55.2", - "@sentry/integrations": "7.55.2", - "@sentry/node": "7.55.2", - "@sentry/svelte": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry-internal/tracing": "7.56.0", + "@sentry/core": "7.56.0", + "@sentry/integrations": "7.56.0", + "@sentry/node": "7.56.0", + "@sentry/svelte": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "@sentry/vite-plugin": "^0.6.0", "magicast": "0.2.8", "sorcery": "0.11.0" diff --git a/packages/tracing-internal/package.json b/packages/tracing-internal/package.json index 614fb4f51a87..9c77cb6c6334 100644 --- a/packages/tracing-internal/package.json +++ b/packages/tracing-internal/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/tracing", - "version": "7.55.2", + "version": "7.56.0", "description": "Sentry Internal Tracing Package", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/tracing-internal", @@ -16,9 +16,9 @@ "access": "public" }, "dependencies": { - "@sentry/core": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/core": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^1.9.3" }, "devDependencies": { diff --git a/packages/tracing/package.json b/packages/tracing/package.json index f0dabb89c27d..c177599cdcfe 100644 --- a/packages/tracing/package.json +++ b/packages/tracing/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/tracing", - "version": "7.55.2", + "version": "7.56.0", "description": "Sentry Performance Monitoring Package", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/tracing", @@ -16,14 +16,14 @@ "access": "public" }, "dependencies": { - "@sentry-internal/tracing": "7.55.2" + "@sentry-internal/tracing": "7.56.0" }, "devDependencies": { - "@sentry-internal/integration-shims": "7.55.2", - "@sentry/browser": "7.55.2", - "@sentry/core": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry-internal/integration-shims": "7.56.0", + "@sentry/browser": "7.56.0", + "@sentry/core": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "@types/express": "^4.17.14" }, "scripts": { diff --git a/packages/types/package.json b/packages/types/package.json index 5a01717aac7d..736924037fc0 100644 --- a/packages/types/package.json +++ b/packages/types/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/types", - "version": "7.55.2", + "version": "7.56.0", "description": "Types for all Sentry JavaScript SDKs", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/types", diff --git a/packages/typescript/package.json b/packages/typescript/package.json index d245845a227a..44c0379492fc 100644 --- a/packages/typescript/package.json +++ b/packages/typescript/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/typescript", - "version": "7.55.2", + "version": "7.56.0", "description": "Typescript configuration used at Sentry", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/typescript", diff --git a/packages/utils/package.json b/packages/utils/package.json index 88c8c4fcb4f3..625cf7652e13 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/utils", - "version": "7.55.2", + "version": "7.56.0", "description": "Utilities for all Sentry JavaScript SDKs", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/utils", @@ -16,7 +16,7 @@ "access": "public" }, "dependencies": { - "@sentry/types": "7.55.2", + "@sentry/types": "7.56.0", "tslib": "^1.9.3" }, "devDependencies": { diff --git a/packages/vue/package.json b/packages/vue/package.json index 236435b93e01..b82ca6ae4e61 100644 --- a/packages/vue/package.json +++ b/packages/vue/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/vue", - "version": "7.55.2", + "version": "7.56.0", "description": "Official Sentry SDK for Vue.js", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/vue", @@ -16,10 +16,10 @@ "access": "public" }, "dependencies": { - "@sentry/browser": "7.55.2", - "@sentry/core": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/browser": "7.56.0", + "@sentry/core": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^1.9.3" }, "peerDependencies": { diff --git a/packages/wasm/package.json b/packages/wasm/package.json index 6a9275e9a1bb..fcd2bef8a637 100644 --- a/packages/wasm/package.json +++ b/packages/wasm/package.json @@ -1,6 +1,6 @@ { "name": "@sentry/wasm", - "version": "7.55.2", + "version": "7.56.0", "description": "Support for WASM.", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/wasm", @@ -16,9 +16,9 @@ "access": "public" }, "dependencies": { - "@sentry/browser": "7.55.2", - "@sentry/types": "7.55.2", - "@sentry/utils": "7.55.2", + "@sentry/browser": "7.56.0", + "@sentry/types": "7.56.0", + "@sentry/utils": "7.56.0", "tslib": "^1.9.3" }, "scripts": { From df4f4ab8734192cef2f04e2b672c76274da1dfdb Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 19 Jun 2023 10:49:47 -0400 Subject: [PATCH 31/38] feat(replay): Do not capture replays < 5 seconds (GA) (#8277) --- .../suites/replay/bufferMode/test.ts | 19 +- .../suites/replay/fileInput/test.ts | 7 +- .../largeMutations/defaultOptions/test.ts | 4 - .../largeMutations/mutationLimit/test.ts | 3 - .../suites/replay/slowClick/mutation/test.ts | 94 +- .../suites/replay/throttleBreadcrumbs/test.ts | 13 +- .../tests/fixtures/ReplayRecordingData.ts | 22 - packages/replay/src/replay.ts | 7 + packages/replay/src/types/replay.ts | 2 +- .../replay/src/util/handleRecordingEmit.ts | 33 +- .../coreHandlers/handleAfterSendEvent.test.ts | 6 +- .../errorSampleRate-delayFlush.test.ts | 865 ------------------ .../test/integration/errorSampleRate.test.ts | 217 +++-- 13 files changed, 222 insertions(+), 1070 deletions(-) delete mode 100644 packages/replay/test/integration/errorSampleRate-delayFlush.test.ts diff --git a/packages/browser-integration-tests/suites/replay/bufferMode/test.ts b/packages/browser-integration-tests/suites/replay/bufferMode/test.ts index a9a9dbbe86e5..4785c1a5b158 100644 --- a/packages/browser-integration-tests/suites/replay/bufferMode/test.ts +++ b/packages/browser-integration-tests/suites/replay/bufferMode/test.ts @@ -25,7 +25,6 @@ sentryTest( let errorEventId: string | undefined; const reqPromise0 = waitForReplayRequest(page, 0); const reqPromise1 = waitForReplayRequest(page, 1); - const reqPromise2 = waitForReplayRequest(page, 2); const reqErrorPromise = waitForErrorRequest(page); await page.route('https://dsn.ingest.sentry.io/**/*', route => { @@ -101,8 +100,7 @@ sentryTest( // Switches to session mode and then goes to background const req1 = await reqPromise1; - const req2 = await reqPromise2; - expect(callsToSentry).toBeGreaterThanOrEqual(5); + expect(callsToSentry).toBeGreaterThanOrEqual(4); const event0 = getReplayEvent(req0); const content0 = getReplayRecordingContent(req0); @@ -110,9 +108,6 @@ sentryTest( const event1 = getReplayEvent(req1); const content1 = getReplayRecordingContent(req1); - const event2 = getReplayEvent(req2); - const content2 = getReplayRecordingContent(req2); - expect(event0).toEqual( getExpectedReplayEvent({ error_ids: [errorEventId!], @@ -157,17 +152,7 @@ sentryTest( // From switching to session mode expect(content1.fullSnapshots).toHaveLength(1); - - expect(event2).toEqual( - getExpectedReplayEvent({ - replay_type: 'buffer', // although we're in session mode, we still send 'buffer' as replay_type - segment_id: 2, - urls: [], - }), - ); - - expect(content2.fullSnapshots).toHaveLength(0); - expect(content2.breadcrumbs).toEqual(expect.arrayContaining([expectedClickBreadcrumb])); + expect(content1.breadcrumbs).toEqual(expect.arrayContaining([expectedClickBreadcrumb])); }, ); diff --git a/packages/browser-integration-tests/suites/replay/fileInput/test.ts b/packages/browser-integration-tests/suites/replay/fileInput/test.ts index 685c626ec470..e0827538ba56 100644 --- a/packages/browser-integration-tests/suites/replay/fileInput/test.ts +++ b/packages/browser-integration-tests/suites/replay/fileInput/test.ts @@ -25,7 +25,6 @@ sentryTest( } const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise1 = waitForReplayRequest(page, 1); await page.route('https://dsn.ingest.sentry.io/**/*', route => { return route.fulfill({ @@ -39,7 +38,7 @@ sentryTest( await page.goto(url); - await reqPromise0; + const res = await reqPromise0; await page.setInputFiles('#file-input', { name: 'file.csv', @@ -49,9 +48,7 @@ sentryTest( await forceFlushReplay(); - const res1 = await reqPromise1; - - const snapshots = getIncrementalRecordingSnapshots(res1).filter(isInputMutation); + const snapshots = getIncrementalRecordingSnapshots(res).filter(isInputMutation); expect(snapshots).toEqual([]); }, diff --git a/packages/browser-integration-tests/suites/replay/largeMutations/defaultOptions/test.ts b/packages/browser-integration-tests/suites/replay/largeMutations/defaultOptions/test.ts index 29d0f3ada164..c0d8e8234da8 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/defaultOptions/test.ts +++ b/packages/browser-integration-tests/suites/replay/largeMutations/defaultOptions/test.ts @@ -11,7 +11,6 @@ sentryTest( } const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise0b = waitForReplayRequest(page, 1); await page.route('https://dsn.ingest.sentry.io/**/*', route => { return route.fulfill({ @@ -24,10 +23,7 @@ sentryTest( const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - await forceFlushReplay(); const res0 = await reqPromise0; - await reqPromise0b; - // A second request is sent right after initial snapshot, we want to wait for that to settle before we continue const reqPromise1 = waitForReplayRequest(page); 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 84f0113263d7..b826daafe6b4 100644 --- a/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts +++ b/packages/browser-integration-tests/suites/replay/largeMutations/mutationLimit/test.ts @@ -16,7 +16,6 @@ sentryTest( } const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise0b = waitForReplayRequest(page, 1); await page.route('https://dsn.ingest.sentry.io/**/*', route => { return route.fulfill({ @@ -30,8 +29,6 @@ sentryTest( await page.goto(url); const res0 = await reqPromise0; - await reqPromise0b; - // A second request is sent right after initial snapshot, we want to wait for that to settle before we continue const reqPromise1 = waitForReplayRequest(page); 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 deb394ebac2d..bab50e12938c 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -3,7 +3,7 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; -sentryTest('mutation after threshold results in slow click', async ({ getLocalTestUrl, page }) => { +sentryTest('mutation after threshold results in slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } @@ -21,6 +21,7 @@ sentryTest('mutation after threshold results in slow click', async ({ getLocalTe const url = await getLocalTestUrl({ testDir: __dirname }); await page.goto(url); + await forceFlushReplay(); await reqPromise0; const reqPromise1 = waitForReplayRequest(page, (event, res) => { @@ -125,59 +126,63 @@ sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => { expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100); }); -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(); - } - - const reqPromise0 = waitForReplayRequest(page, 0); - - await page.route('https://dsn.ingest.sentry.io/**/*', route => { - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ id: 'test-id' }), +sentryTest( + 'immediate mutation does not trigger slow click', + async ({ forceFlushReplay, browserName, getLocalTestUrl, page }) => { + // This test seems to only be flakey on firefox + if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + + 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 }); + const url = await getLocalTestUrl({ testDir: __dirname }); - await page.goto(url); - await reqPromise0; + await page.goto(url); + await forceFlushReplay(); + await reqPromise0; - const reqPromise1 = waitForReplayRequest(page, (event, res) => { - const { breadcrumbs } = getCustomRecordingEvents(res); + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); - }); - - await page.click('#mutationButtonImmediately'); - - const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); - expect(breadcrumbs).toEqual([ - { - category: 'ui.click', - data: { - node: { - attributes: { - id: 'mutationButtonImmediately', + await page.click('#mutationButtonImmediately'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mutationButtonImmediately', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******** ***********', }, - id: expect.any(Number), - tagName: 'button', - textContent: '******* ******** ***********', + nodeId: expect.any(Number), }, - nodeId: expect.any(Number), + message: 'body > button#mutationButtonImmediately', + timestamp: expect.any(Number), + type: 'default', }, - message: 'body > button#mutationButtonImmediately', - timestamp: expect.any(Number), - type: 'default', - }, - ]); -}); + ]); + }, +); -sentryTest('inline click handler does not trigger slow click', async ({ getLocalTestUrl, page }) => { +sentryTest('inline click handler does not trigger slow click', async ({ forceFlushReplay, getLocalTestUrl, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } @@ -195,6 +200,7 @@ sentryTest('inline click handler does not trigger slow click', async ({ getLocal const url = await getLocalTestUrl({ testDir: __dirname }); await page.goto(url); + await forceFlushReplay(); await reqPromise0; const reqPromise1 = waitForReplayRequest(page, (event, res) => { diff --git a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts index 17f4210624a0..e025c90a77e0 100644 --- a/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts +++ b/packages/browser-integration-tests/suites/replay/throttleBreadcrumbs/test.ts @@ -26,16 +26,19 @@ sentryTest( const url = await getLocalTestUrl({ testDir: __dirname }); await page.goto(url); - await reqPromise0; + await forceFlushReplay(); + const res0 = getCustomRecordingEvents(await reqPromise0); await page.click('[data-console]'); await forceFlushReplay(); - const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + const res1 = getCustomRecordingEvents(await reqPromise1); - // 1 click breadcrumb + 1 throttled breadcrumb is why console logs are less - // than throttle limit - expect(breadcrumbs.length).toBe(THROTTLE_LIMIT); + const breadcrumbs = [...res0.breadcrumbs, ...res1.breadcrumbs]; + const spans = [...res0.performanceSpans, ...res1.performanceSpans]; expect(breadcrumbs.filter(breadcrumb => breadcrumb.category === 'replay.throttled').length).toBe(1); + // replay.throttled breadcrumb does *not* use the throttledAddEvent as we + // alwants want that breadcrumb to be present in replay + expect(breadcrumbs.length + spans.length).toBe(THROTTLE_LIMIT + 1); }, ); diff --git a/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts b/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts index a22694a64304..0da2e1b2e327 100644 --- a/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts +++ b/packages/e2e-tests/test-applications/standard-frontend-react/tests/fixtures/ReplayRecordingData.ts @@ -95,26 +95,6 @@ export const ReplayRecordingData = [ }, timestamp: expect.any(Number), }, - { - type: 5, - timestamp: expect.any(Number), - data: { - tag: 'performanceSpan', - payload: { - op: 'memory', - description: 'memory', - startTimestamp: expect.any(Number), - endTimestamp: expect.any(Number), - data: { - memory: { - jsHeapSizeLimit: expect.any(Number), - totalJSHeapSize: expect.any(Number), - usedJSHeapSize: expect.any(Number), - }, - }, - }, - }, - }, { type: 3, data: { @@ -155,8 +135,6 @@ export const ReplayRecordingData = [ data: { source: 5, text: 'Capture Exception', isChecked: false, id: 16 }, timestamp: expect.any(Number), }, - ], - [ { type: 5, timestamp: expect.any(Number), diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index acb2980e608c..8fab410a0c34 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -547,6 +547,13 @@ export class ReplayContainer implements ReplayContainerInterface { return this.flushImmediate(); } + /** + * Flush using debounce flush + */ + public flush(): Promise { + return this._debouncedFlush() as Promise; + } + /** * Always flush via `_debouncedFlush` so that we do not have flushes triggered * from calling both `flush` and `_debouncedFlush`. Otherwise, there could be diff --git a/packages/replay/src/types/replay.ts b/packages/replay/src/types/replay.ts index f058b2c9011a..e2cfddd0f525 100644 --- a/packages/replay/src/types/replay.ts +++ b/packages/replay/src/types/replay.ts @@ -194,7 +194,6 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { _experiments: Partial<{ captureExceptions: boolean; traceInternals: boolean; - delayFlushOnCheckout: number; }>; } @@ -438,6 +437,7 @@ export interface ReplayContainer { stopRecording(): boolean; sendBufferedReplayOrFlush(options?: SendBufferedReplayOptions): Promise; conditionalFlush(): Promise; + flush(): Promise; flushImmediate(): Promise; cancelFlush(): void; triggerUserActivity(): void; diff --git a/packages/replay/src/util/handleRecordingEmit.ts b/packages/replay/src/util/handleRecordingEmit.ts index cc7c87afed48..987f589412ed 100644 --- a/packages/replay/src/util/handleRecordingEmit.ts +++ b/packages/replay/src/util/handleRecordingEmit.ts @@ -80,41 +80,12 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa } } - const options = replay.getOptions(); - - // TODO: We want this as an experiment so that we can test - // internally and create metrics before making this the default - if (options._experiments.delayFlushOnCheckout) { + if (replay.recordingMode === 'session') { // If the full snapshot is due to an initial load, we will not have // a previous session ID. In this case, we want to buffer events // for a set amount of time before flushing. This can help avoid // capturing replays of users that immediately close the window. - // TODO: We should check `recordingMode` here and do nothing if it's - // buffer, instead of checking inside of timeout, this will make our - // tests a bit cleaner as we will need to wait on the delay in order to - // do nothing. - setTimeout(() => replay.conditionalFlush(), options._experiments.delayFlushOnCheckout); - - // Cancel any previously debounced flushes to ensure there are no [near] - // simultaneous flushes happening. The latter request should be - // insignificant in this case, so wait for additional user interaction to - // trigger a new flush. - // - // This can happen because there's no guarantee that a recording event - // happens first. e.g. a mouse click can happen and trigger a debounced - // flush before the checkout. - replay.cancelFlush(); - - return true; - } - - // Flush immediately so that we do not miss the first segment, otherwise - // it can prevent loading on the UI. This will cause an increase in short - // replays (e.g. opening and closing a tab quickly), but these can be - // filtered on the UI. - if (replay.recordingMode === 'session') { - // We want to ensure the worker is ready, as otherwise we'd always send the first event uncompressed - void replay.flushImmediate(); + void replay.flush(); } return true; diff --git a/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts index 1e59a4f7eef0..cd367a2d04f5 100644 --- a/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts +++ b/packages/replay/test/integration/coreHandlers/handleAfterSendEvent.test.ts @@ -148,9 +148,13 @@ describe('Integration | coreHandlers | handleAfterSendEvent', () => { jest.runAllTimers(); await new Promise(process.nextTick); - // Send twice, one for the error & one right after for the session conversion + expect(mockSend).toHaveBeenCalledTimes(1); + + jest.runAllTimers(); + await new Promise(process.nextTick); expect(mockSend).toHaveBeenCalledTimes(2); + // This is removed now, because it has been converted to a "session" session expect(Array.from(replay.getContext().errorIds)).toEqual([]); expect(replay.isEnabled()).toBe(true); diff --git a/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts b/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts deleted file mode 100644 index f691d8e953c1..000000000000 --- a/packages/replay/test/integration/errorSampleRate-delayFlush.test.ts +++ /dev/null @@ -1,865 +0,0 @@ -import { captureException, getCurrentHub } from '@sentry/core'; - -import { - BUFFER_CHECKOUT_TIME, - DEFAULT_FLUSH_MIN_DELAY, - MAX_SESSION_LIFE, - REPLAY_SESSION_KEY, - SESSION_IDLE_EXPIRE_DURATION, - WINDOW, -} from '../../src/constants'; -import type { ReplayContainer } from '../../src/replay'; -import { clearSession } from '../../src/session/clearSession'; -import { addEvent } from '../../src/util/addEvent'; -import { createOptionsEvent } from '../../src/util/handleRecordingEmit'; -import { PerformanceEntryResource } from '../fixtures/performanceEntry/resource'; -import type { RecordMock } from '../index'; -import { BASE_TIMESTAMP } from '../index'; -import { resetSdkMock } from '../mocks/resetSdkMock'; -import type { DomHandler } from '../types'; -import { useFakeTimers } from '../utils/use-fake-timers'; - -useFakeTimers(); - -async function advanceTimers(time: number) { - jest.advanceTimersByTime(time); - await new Promise(process.nextTick); -} - -async function waitForBufferFlush() { - await new Promise(process.nextTick); - await new Promise(process.nextTick); -} - -async function waitForFlush() { - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); -} - -describe('Integration | errorSampleRate with delayed flush', () => { - let replay: ReplayContainer; - let mockRecord: RecordMock; - let domHandler: DomHandler; - - beforeEach(async () => { - ({ mockRecord, domHandler, replay } = await resetSdkMock({ - replayOptions: { - stickySession: true, - _experiments: { - delayFlushOnCheckout: DEFAULT_FLUSH_MIN_DELAY, - }, - }, - sentryOptions: { - replaysSessionSampleRate: 0.0, - replaysOnErrorSampleRate: 1.0, - }, - })); - }); - - afterEach(async () => { - clearSession(replay); - replay.stop(); - }); - - it('uploads a replay when `Sentry.captureException` is called and continues recording', async () => { - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - const optionsEvent = createOptionsEvent(replay); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - - // Does not capture on mouse click - domHandler({ - name: 'click', - }); - jest.runAllTimers(); - await new Promise(process.nextTick); - expect(replay).not.toHaveLastSentReplay(); - - captureException(new Error('testing')); - - await waitForBufferFlush(); - - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, - optionsEvent, - TEST_EVENT, - { - type: 5, - timestamp: BASE_TIMESTAMP, - data: { - tag: 'breadcrumb', - payload: { - timestamp: BASE_TIMESTAMP / 1000, - type: 'default', - category: 'ui.click', - message: '', - data: {}, - }, - }, - }, - ]), - }); - - await waitForFlush(); - - // This is from when we stop recording and start a session recording - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 1 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 40, type: 2 }]), - }); - - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - - // Check that click will get captured - domHandler({ - name: 'click', - }); - - await waitForFlush(); - - expect(replay).toHaveLastSentReplay({ - recordingData: JSON.stringify([ - { - type: 5, - timestamp: BASE_TIMESTAMP + 10000 + 80, - data: { - tag: 'breadcrumb', - payload: { - timestamp: (BASE_TIMESTAMP + 10000 + 80) / 1000, - type: 'default', - category: 'ui.click', - message: '', - data: {}, - }, - }, - }, - ]), - }); - }); - - it('manually flushes replay and does not continue to record', async () => { - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - const optionsEvent = createOptionsEvent(replay); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - - // Does not capture on mouse click - domHandler({ - name: 'click', - }); - jest.runAllTimers(); - await new Promise(process.nextTick); - expect(replay).not.toHaveLastSentReplay(); - - replay.sendBufferedReplayOrFlush({ continueRecording: false }); - - await waitForBufferFlush(); - - expect(replay).toHaveSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, - optionsEvent, - TEST_EVENT, - { - type: 5, - timestamp: BASE_TIMESTAMP, - data: { - tag: 'breadcrumb', - payload: { - timestamp: BASE_TIMESTAMP / 1000, - type: 'default', - category: 'ui.click', - message: '', - data: {}, - }, - }, - }, - ]), - }); - - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - // Check that click will not get captured - domHandler({ - name: 'click', - }); - - await waitForFlush(); - - // This is still the last replay sent since we passed `continueRecording: - // false`. - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, - optionsEvent, - TEST_EVENT, - { - type: 5, - timestamp: BASE_TIMESTAMP, - data: { - tag: 'breadcrumb', - payload: { - timestamp: BASE_TIMESTAMP / 1000, - type: 'default', - category: 'ui.click', - message: '', - data: {}, - }, - }, - }, - ]), - }); - }); - - // This tests a regression where we were calling flush indiscriminantly in `stop()` - it('does not upload a replay event if error is not sampled', async () => { - // We are trying to replicate the case where error rate is 0 and session - // rate is > 0, we can't set them both to 0 otherwise - // `_loadAndCheckSession` is not called when initializing the plugin. - replay.stop(); - replay['_options']['errorSampleRate'] = 0; - replay['_loadAndCheckSession'](); - - jest.runAllTimers(); - await new Promise(process.nextTick); - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - }); - - it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_EXPIRE_DURATION]ms', async () => { - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'visible'; - }, - }); - - jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1); - - document.dispatchEvent(new Event('visibilitychange')); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(replay).not.toHaveLastSentReplay(); - }); - - it('does not send a replay if user hides the tab and comes back within 60 seconds', async () => { - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'hidden'; - }, - }); - document.dispatchEvent(new Event('visibilitychange')); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(replay).not.toHaveLastSentReplay(); - - // User comes back before `SESSION_IDLE_EXPIRE_DURATION` elapses - jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION - 100); - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'visible'; - }, - }); - document.dispatchEvent(new Event('visibilitychange')); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - }); - - it('does not upload a replay event when document becomes hidden', async () => { - Object.defineProperty(document, 'visibilityState', { - configurable: true, - get: function () { - return 'hidden'; - }, - }); - - // Pretend 5 seconds have passed - const ELAPSED = 5000; - jest.advanceTimersByTime(ELAPSED); - - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; - addEvent(replay, TEST_EVENT); - - document.dispatchEvent(new Event('visibilitychange')); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - }); - - it('does not upload a replay event if 5 seconds have elapsed since the last replay event occurred', async () => { - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - // Pretend 5 seconds have passed - const ELAPSED = 5000; - await advanceTimers(ELAPSED); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(replay).not.toHaveLastSentReplay(); - }); - - it('does not upload a replay event if 15 seconds have elapsed since the last replay upload', async () => { - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - // Fire a new event every 4 seconds, 4 times - [...Array(4)].forEach(() => { - mockRecord._emitter(TEST_EVENT); - jest.advanceTimersByTime(4000); - }); - - // We are at time = +16seconds now (relative to BASE_TIMESTAMP) - // The next event should cause an upload immediately - mockRecord._emitter(TEST_EVENT); - await new Promise(process.nextTick); - - expect(replay).not.toHaveLastSentReplay(); - - // There should also not be another attempt at an upload 5 seconds after the last replay event - await waitForFlush(); - expect(replay).not.toHaveLastSentReplay(); - - // Let's make sure it continues to work - mockRecord._emitter(TEST_EVENT); - await waitForFlush(); - jest.runAllTimers(); - await new Promise(process.nextTick); - expect(replay).not.toHaveLastSentReplay(); - }); - - // When the error session records as a normal session, we want to stop - // recording after the session ends. Otherwise, we get into a state where the - // new session is a session type replay (this could conflict with the session - // sample rate of 0.0), or an error session that has no errors. Instead we - // simply stop the session replay completely and wait for a new page load to - // resample. - it.each([ - ['MAX_SESSION_LIFE', MAX_SESSION_LIFE], - ['SESSION_IDLE_DURATION', SESSION_IDLE_EXPIRE_DURATION], - ])( - 'stops replay if session had an error and exceeds %s and does not start a new session thereafter', - async (_label, waitTime) => { - expect(replay.session?.shouldRefresh).toBe(true); - - captureException(new Error('testing')); - - await waitForBufferFlush(); - - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - }); - - await waitForFlush(); - - // segment_id is 1 because it sends twice on error - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 1 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - }); - expect(replay.session?.shouldRefresh).toBe(false); - - // Idle for given time - jest.advanceTimersByTime(waitTime + 1); - await new Promise(process.nextTick); - - const TEST_EVENT = { - data: { name: 'lost event' }, - timestamp: BASE_TIMESTAMP, - type: 3, - }; - mockRecord._emitter(TEST_EVENT); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - // We stop recording after 15 minutes of inactivity in error mode - - // still no new replay sent - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 1 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - }); - - expect(replay.isEnabled()).toBe(false); - - domHandler({ - name: 'click', - }); - - // Remains disabled! - expect(replay.isEnabled()).toBe(false); - }, - ); - - it.each([ - ['MAX_SESSION_LIFE', MAX_SESSION_LIFE], - ['SESSION_IDLE_EXPIRE_DURATION', SESSION_IDLE_EXPIRE_DURATION], - ])('continues buffering replay if session had no error and exceeds %s', async (_label, waitTime) => { - expect(replay).not.toHaveLastSentReplay(); - - // Idle for given time - jest.advanceTimersByTime(waitTime + 1); - await new Promise(process.nextTick); - - const TEST_EVENT = { - data: { name: 'lost event' }, - timestamp: BASE_TIMESTAMP, - type: 3, - }; - mockRecord._emitter(TEST_EVENT); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - // still no new replay sent - expect(replay).not.toHaveLastSentReplay(); - - expect(replay.isEnabled()).toBe(true); - expect(replay.isPaused()).toBe(false); - expect(replay.recordingMode).toBe('buffer'); - - domHandler({ - name: 'click', - }); - - await waitForFlush(); - - expect(replay).not.toHaveLastSentReplay(); - expect(replay.isEnabled()).toBe(true); - expect(replay.isPaused()).toBe(false); - expect(replay.recordingMode).toBe('buffer'); - - // should still react to errors later on - captureException(new Error('testing')); - - await waitForBufferFlush(); - - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - }); - - expect(replay.isEnabled()).toBe(true); - expect(replay.isPaused()).toBe(false); - expect(replay.recordingMode).toBe('session'); - expect(replay.session?.sampled).toBe('buffer'); - expect(replay.session?.shouldRefresh).toBe(false); - }); - - // Should behave the same as above test - it('stops replay if user has been idle for more than SESSION_IDLE_EXPIRE_DURATION and does not start a new session thereafter', async () => { - // Idle for 15 minutes - jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1); - - const TEST_EVENT = { - data: { name: 'lost event' }, - timestamp: BASE_TIMESTAMP, - type: 3, - }; - mockRecord._emitter(TEST_EVENT); - expect(replay).not.toHaveLastSentReplay(); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - // We stop recording after SESSION_IDLE_EXPIRE_DURATION of inactivity in error mode - expect(replay).not.toHaveLastSentReplay(); - expect(replay.isEnabled()).toBe(true); - expect(replay.isPaused()).toBe(false); - expect(replay.recordingMode).toBe('buffer'); - - // should still react to errors later on - captureException(new Error('testing')); - - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); - - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - }); - - expect(replay.isEnabled()).toBe(true); - expect(replay.isPaused()).toBe(false); - expect(replay.recordingMode).toBe('session'); - expect(replay.session?.sampled).toBe('buffer'); - expect(replay.session?.shouldRefresh).toBe(false); - }); - - it('has the correct timestamps with deferred root event and last replay update', async () => { - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - const optionsEvent = createOptionsEvent(replay); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - - captureException(new Error('testing')); - - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); - - expect(replay).toHaveSentReplay({ - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, - optionsEvent, - TEST_EVENT, - ]), - replayEventPayload: expect.objectContaining({ - replay_start_timestamp: BASE_TIMESTAMP / 1000, - // the exception happens roughly 10 seconds after BASE_TIMESTAMP - // (advance timers + waiting for flush after the checkout) and - // extra time is likely due to async of `addMemoryEntry()` - - timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY + 40) / 1000, - error_ids: [expect.any(String)], - trace_ids: [], - urls: ['http://localhost/'], - replay_id: expect.any(String), - }), - recordingPayloadHeader: { segment_id: 0 }, - }); - }); - - it('has correct timestamps when error occurs much later than initial pageload/checkout', async () => { - const ELAPSED = BUFFER_CHECKOUT_TIME; - const TICK = 20; - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - - // add a mock performance event - replay.performanceEvents.push(PerformanceEntryResource()); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - - jest.advanceTimersByTime(ELAPSED); - - // in production, this happens at a time interval - // session started time should be updated to this current timestamp - mockRecord.takeFullSnapshot(true); - const optionsEvent = createOptionsEvent(replay); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(replay).not.toHaveLastSentReplay(); - - captureException(new Error('testing')); - - await waitForBufferFlush(); - - // See comments in `handleRecordingEmit.ts`, we perform a setTimeout into a - // noop when it can be skipped altogether - expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + DEFAULT_FLUSH_MIN_DELAY + TICK + TICK); - - // Does not capture mouse click - expect(replay).toHaveSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - // Make sure the old performance event is thrown out - replay_start_timestamp: (BASE_TIMESTAMP + ELAPSED + TICK) / 1000, - }), - recordingData: JSON.stringify([ - { - data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + ELAPSED + TICK, - type: 2, - }, - optionsEvent, - ]), - }); - }); - - it('stops replay when user goes idle', async () => { - jest.setSystemTime(BASE_TIMESTAMP); - - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - captureException(new Error('testing')); - - await waitForBufferFlush(); - - expect(replay).toHaveLastSentReplay(); - - // Flush from calling `stopRecording` - await waitForFlush(); - - // Now wait after session expires - should stop recording - mockRecord.takeFullSnapshot.mockClear(); - (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); - - expect(replay).not.toHaveLastSentReplay(); - - // Go idle - jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION + 1); - await new Promise(process.nextTick); - - mockRecord._emitter(TEST_EVENT); - - expect(replay).not.toHaveLastSentReplay(); - - await waitForFlush(); - - expect(replay).not.toHaveLastSentReplay(); - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); - expect(replay.isEnabled()).toBe(false); - }); - - it('stops replay when session exceeds max length after latest captured error', async () => { - const sessionId = replay.session?.id; - jest.setSystemTime(BASE_TIMESTAMP); - - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - jest.advanceTimersByTime(2 * MAX_SESSION_LIFE); - - captureException(new Error('testing')); - - // Flush due to exception - await new Promise(process.nextTick); - await waitForFlush(); - - expect(replay.session?.id).toBe(sessionId); - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - }); - - // This comes from `startRecording()` in `sendBufferedReplayOrFlush()` - await waitForFlush(); - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 1 }, - recordingData: JSON.stringify([ - { - data: { - isCheckout: true, - }, - timestamp: BASE_TIMESTAMP + 2 * MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 40, - type: 2, - }, - ]), - }); - - // Now wait after session expires - should stop recording - mockRecord.takeFullSnapshot.mockClear(); - (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); - - jest.advanceTimersByTime(MAX_SESSION_LIFE); - await new Promise(process.nextTick); - - mockRecord._emitter(TEST_EVENT); - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(replay).not.toHaveLastSentReplay(); - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); - expect(replay.isEnabled()).toBe(false); - - // Once the session is stopped after capturing a replay already - // (buffer-mode), another error will not trigger a new replay - captureException(new Error('testing')); - - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); - expect(replay).not.toHaveLastSentReplay(); - }); - - it('does not stop replay based on earliest event in buffer', async () => { - jest.setSystemTime(BASE_TIMESTAMP); - - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP - 60000, type: 3 }; - mockRecord._emitter(TEST_EVENT); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(replay).not.toHaveLastSentReplay(); - - jest.runAllTimers(); - await new Promise(process.nextTick); - - expect(replay).not.toHaveLastSentReplay(); - captureException(new Error('testing')); - - await waitForBufferFlush(); - - expect(replay).toHaveLastSentReplay(); - - // Flush from calling `stopRecording` - await waitForFlush(); - - // Now wait after session expires - should stop recording - mockRecord.takeFullSnapshot.mockClear(); - (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); - - expect(replay).not.toHaveLastSentReplay(); - - const TICKS = 80; - - // We advance time so that we are on the border of expiring, taking into - // account that TEST_EVENT timestamp is 60000 ms before BASE_TIMESTAMP. The - // 3 DEFAULT_FLUSH_MIN_DELAY is to account for the `waitForFlush` that has - // happened, and for the next two that will happen. The first following - // `waitForFlush` does not expire session, but the following one will. - jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION - 60000 - 3 * DEFAULT_FLUSH_MIN_DELAY - TICKS); - await new Promise(process.nextTick); - - mockRecord._emitter(TEST_EVENT); - expect(replay).not.toHaveLastSentReplay(); - await waitForFlush(); - - expect(replay).not.toHaveLastSentReplay(); - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); - expect(replay.isEnabled()).toBe(true); - - mockRecord._emitter(TEST_EVENT); - expect(replay).not.toHaveLastSentReplay(); - await waitForFlush(); - - expect(replay).not.toHaveLastSentReplay(); - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); - expect(replay.isEnabled()).toBe(true); - - // It's hard to test, but if we advance the below time less 1 ms, it should - // be enabled, but we can't trigger a session check via flush without - // incurring another DEFAULT_FLUSH_MIN_DELAY timeout. - jest.advanceTimersByTime(60000 - DEFAULT_FLUSH_MIN_DELAY); - mockRecord._emitter(TEST_EVENT); - expect(replay).not.toHaveLastSentReplay(); - await waitForFlush(); - - expect(replay).not.toHaveLastSentReplay(); - expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); - expect(replay.isEnabled()).toBe(false); - }); -}); - -/** - * This is testing a case that should only happen with error-only sessions. - * Previously we had assumed that loading a session from session storage meant - * that the session was not new. However, this is not the case with error-only - * sampling since we can load a saved session that did not have an error (and - * thus no replay was created). - */ -it('sends a replay after loading the session from storage', async () => { - // Pretend that a session is already saved before loading replay - WINDOW.sessionStorage.setItem( - REPLAY_SESSION_KEY, - `{"segmentId":0,"id":"fd09adfc4117477abc8de643e5a5798a","sampled":"buffer","started":${BASE_TIMESTAMP},"lastActivity":${BASE_TIMESTAMP}}`, - ); - const { mockRecord, replay, integration } = await resetSdkMock({ - replayOptions: { - stickySession: true, - _experiments: { - delayFlushOnCheckout: DEFAULT_FLUSH_MIN_DELAY, - }, - }, - sentryOptions: { - replaysOnErrorSampleRate: 1.0, - }, - autoStart: false, - }); - integration['_initialize'](); - const optionsEvent = createOptionsEvent(replay); - - jest.runAllTimers(); - - await new Promise(process.nextTick); - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; - mockRecord._emitter(TEST_EVENT); - - expect(replay).not.toHaveLastSentReplay(); - - captureException(new Error('testing')); - - // 2 ticks to send replay from an error - await waitForBufferFlush(); - - // Buffered events before error - expect(replay).toHaveSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, - optionsEvent, - TEST_EVENT, - ]), - }); - - // `startRecording()` after switching to session mode to continue recording - await waitForFlush(); - - // Latest checkout when we call `startRecording` again after uploading segment - // after an error occurs (e.g. when we switch to session replay recording) - expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 1 }, - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40, type: 2 }, - ]), - }); -}); diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index ea1825dd8429..fe3049f9704f 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -26,6 +26,15 @@ async function advanceTimers(time: number) { await new Promise(process.nextTick); } +async function waitForBufferFlush() { + await new Promise(process.nextTick); + await new Promise(process.nextTick); +} + +async function waitForFlush() { + await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); +} + describe('Integration | errorSampleRate', () => { let replay: ReplayContainer; let mockRecord: RecordMock; @@ -66,11 +75,9 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForBufferFlush(); - expect(replay).toHaveSentReplay({ + expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', @@ -96,48 +103,35 @@ describe('Integration | errorSampleRate', () => { ]), }); + await waitForFlush(); + // This is from when we stop recording and start a session recording expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 1 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', }), - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40, type: 2 }, - ]), + recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 40, type: 2 }]), }); jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - // New checkout when we call `startRecording` again after uploading segment - // after an error occurs - expect(replay).toHaveLastSentReplay({ - recordingData: JSON.stringify([ - { - data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 40, - type: 2, - }, - ]), - }); - // Check that click will get captured domHandler({ name: 'click', }); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForFlush(); expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([ { type: 5, - timestamp: BASE_TIMESTAMP + 10000 + 60, + timestamp: BASE_TIMESTAMP + 10000 + 80, data: { tag: 'breadcrumb', payload: { - timestamp: (BASE_TIMESTAMP + 10000 + 60) / 1000, + timestamp: (BASE_TIMESTAMP + 10000 + 80) / 1000, type: 'default', category: 'ui.click', message: '', @@ -167,9 +161,7 @@ describe('Integration | errorSampleRate', () => { replay.sendBufferedReplayOrFlush({ continueRecording: false }); - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForBufferFlush(); expect(replay).toHaveSentReplay({ recordingPayloadHeader: { segment_id: 0 }, @@ -202,8 +194,8 @@ describe('Integration | errorSampleRate', () => { domHandler({ name: 'click', }); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + + await waitForFlush(); // This is still the last replay sent since we passed `continueRecording: // false`. @@ -353,12 +345,12 @@ describe('Integration | errorSampleRate', () => { expect(replay).not.toHaveLastSentReplay(); // There should also not be another attempt at an upload 5 seconds after the last replay event - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); + await waitForFlush(); expect(replay).not.toHaveLastSentReplay(); // Let's make sure it continues to work mockRecord._emitter(TEST_EVENT); - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); + await waitForFlush(); jest.runAllTimers(); await new Promise(process.nextTick); expect(replay).not.toHaveLastSentReplay(); @@ -380,9 +372,16 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForBufferFlush(); + + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + replayEventPayload: expect.objectContaining({ + replay_type: 'buffer', + }), + }); + + await waitForFlush(); // segment_id is 1 because it sends twice on error expect(replay).toHaveLastSentReplay({ @@ -462,9 +461,7 @@ describe('Integration | errorSampleRate', () => { name: 'click', }); - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForFlush(); expect(replay).not.toHaveLastSentReplay(); expect(replay.isEnabled()).toBe(true); @@ -474,23 +471,12 @@ describe('Integration | errorSampleRate', () => { // should still react to errors later on captureException(new Error('testing')); - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForBufferFlush(); expect(replay.session?.id).toBe(oldSessionId); - // Flush of buffered events - expect(replay).toHaveSentReplay({ - recordingPayloadHeader: { segment_id: 0 }, - replayEventPayload: expect.objectContaining({ - replay_type: 'buffer', - }), - }); - - // Checkout from `startRecording` expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 1 }, + recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', }), @@ -546,7 +532,7 @@ describe('Integration | errorSampleRate', () => { // `startRecording` full checkout expect(replay).toHaveLastSentReplay({ - recordingPayloadHeader: { segment_id: 1 }, + recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ replay_type: 'buffer', }), @@ -602,6 +588,7 @@ describe('Integration | errorSampleRate', () => { it('has correct timestamps when error occurs much later than initial pageload/checkout', async () => { const ELAPSED = BUFFER_CHECKOUT_TIME; + const TICK = 20; const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 3 }; mockRecord._emitter(TEST_EVENT); @@ -624,25 +611,25 @@ describe('Integration | errorSampleRate', () => { jest.runAllTimers(); await new Promise(process.nextTick); + expect(replay).not.toHaveLastSentReplay(); + captureException(new Error('testing')); - await new Promise(process.nextTick); - jest.runAllTimers(); - await new Promise(process.nextTick); + await waitForBufferFlush(); - expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 40); + expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + TICK + TICK); // Does not capture mouse click expect(replay).toHaveSentReplay({ recordingPayloadHeader: { segment_id: 0 }, replayEventPayload: expect.objectContaining({ // Make sure the old performance event is thrown out - replay_start_timestamp: (BASE_TIMESTAMP + ELAPSED + 20) / 1000, + replay_start_timestamp: (BASE_TIMESTAMP + ELAPSED + TICK) / 1000, }), recordingData: JSON.stringify([ { data: { isCheckout: true }, - timestamp: BASE_TIMESTAMP + ELAPSED + 20, + timestamp: BASE_TIMESTAMP + ELAPSED + TICK, type: 2, }, optionsEvent, @@ -664,12 +651,13 @@ describe('Integration | errorSampleRate', () => { captureException(new Error('testing')); - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForBufferFlush(); expect(replay).toHaveLastSentReplay(); + // Flush from calling `stopRecording` + await waitForFlush(); + // Now wait after session expires - should stop recording mockRecord.takeFullSnapshot.mockClear(); (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); @@ -684,8 +672,7 @@ describe('Integration | errorSampleRate', () => { expect(replay).not.toHaveLastSentReplay(); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForFlush(); expect(replay).not.toHaveLastSentReplay(); expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); @@ -711,12 +698,29 @@ describe('Integration | errorSampleRate', () => { // Flush due to exception await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + await waitForFlush(); + expect(replay.session?.id).toBe(sessionId); - expect(replay).toHaveLastSentReplay(); + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 0 }, + }); + + // This comes from `startRecording()` in `sendBufferedReplayOrFlush()` + await waitForFlush(); + expect(replay).toHaveLastSentReplay({ + recordingPayloadHeader: { segment_id: 1 }, + recordingData: JSON.stringify([ + { + data: { + isCheckout: true, + }, + timestamp: BASE_TIMESTAMP + 2 * MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 40, + type: 2, + }, + ]), + }); - // Now wait after session expires - should re-start into buffering mode + // Now wait after session expires - should stop recording mockRecord.takeFullSnapshot.mockClear(); (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); @@ -732,7 +736,7 @@ describe('Integration | errorSampleRate', () => { expect(replay.isEnabled()).toBe(false); // Once the session is stopped after capturing a replay already - // (buffer-mode), another error should trigger a new replay + // (buffer-mode), another error will not trigger a new replay captureException(new Error('testing')); await new Promise(process.nextTick); @@ -740,6 +744,73 @@ describe('Integration | errorSampleRate', () => { await new Promise(process.nextTick); expect(replay).not.toHaveLastSentReplay(); }); + + it('does not stop replay based on earliest event in buffer', async () => { + jest.setSystemTime(BASE_TIMESTAMP); + + const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP - 60000, type: 3 }; + mockRecord._emitter(TEST_EVENT); + + expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(replay).not.toHaveLastSentReplay(); + + jest.runAllTimers(); + await new Promise(process.nextTick); + + expect(replay).not.toHaveLastSentReplay(); + captureException(new Error('testing')); + + await waitForBufferFlush(); + + expect(replay).toHaveLastSentReplay(); + + // Flush from calling `stopRecording` + await waitForFlush(); + + // Now wait after session expires - should stop recording + mockRecord.takeFullSnapshot.mockClear(); + (getCurrentHub().getClient()!.getTransport()!.send as unknown as jest.SpyInstance).mockClear(); + + expect(replay).not.toHaveLastSentReplay(); + + const TICKS = 80; + + // We advance time so that we are on the border of expiring, taking into + // account that TEST_EVENT timestamp is 60000 ms before BASE_TIMESTAMP. The + // 3 DEFAULT_FLUSH_MIN_DELAY is to account for the `waitForFlush` that has + // happened, and for the next two that will happen. The first following + // `waitForFlush` does not expire session, but the following one will. + jest.advanceTimersByTime(SESSION_IDLE_EXPIRE_DURATION - 60000 - 3 * DEFAULT_FLUSH_MIN_DELAY - TICKS); + await new Promise(process.nextTick); + + mockRecord._emitter(TEST_EVENT); + expect(replay).not.toHaveLastSentReplay(); + await waitForFlush(); + + expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(true); + + mockRecord._emitter(TEST_EVENT); + expect(replay).not.toHaveLastSentReplay(); + await waitForFlush(); + + expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(true); + + // It's hard to test, but if we advance the below time less 1 ms, it should + // be enabled, but we can't trigger a session check via flush without + // incurring another DEFAULT_FLUSH_MIN_DELAY timeout. + jest.advanceTimersByTime(60000 - DEFAULT_FLUSH_MIN_DELAY); + mockRecord._emitter(TEST_EVENT); + expect(replay).not.toHaveLastSentReplay(); + await waitForFlush(); + + expect(replay).not.toHaveLastSentReplay(); + expect(mockRecord.takeFullSnapshot).toHaveBeenCalledTimes(0); + expect(replay.isEnabled()).toBe(false); + }); }); /** @@ -749,7 +820,7 @@ describe('Integration | errorSampleRate', () => { * sampling since we can load a saved session that did not have an error (and * thus no replay was created). */ -it('sends a replay after loading the session multiple times', async () => { +it('sends a replay after loading the session from storage', async () => { // Pretend that a session is already saved before loading replay WINDOW.sessionStorage.setItem( REPLAY_SESSION_KEY, @@ -765,7 +836,6 @@ it('sends a replay after loading the session multiple times', async () => { autoStart: false, }); integration['_initialize'](); - const optionsEvent = createOptionsEvent(replay); jest.runAllTimers(); @@ -778,10 +848,10 @@ it('sends a replay after loading the session multiple times', async () => { captureException(new Error('testing')); - await new Promise(process.nextTick); - jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY); - await new Promise(process.nextTick); + // 2 ticks to send replay from an error + await waitForBufferFlush(); + // Buffered events before error expect(replay).toHaveSentReplay({ recordingPayloadHeader: { segment_id: 0 }, recordingData: JSON.stringify([ @@ -791,10 +861,13 @@ it('sends a replay after loading the session multiple times', async () => { ]), }); + // `startRecording()` after switching to session mode to continue recording + await waitForFlush(); + // Latest checkout when we call `startRecording` again after uploading segment // after an error occurs (e.g. when we switch to session replay recording) expect(replay).toHaveLastSentReplay({ recordingPayloadHeader: { segment_id: 1 }, - recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5040, type: 2 }]), + recordingData: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 40, type: 2 }]), }); }); From 50ead2493dcc64f1a54fd54a6b3a6909e3005c43 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 19 Jun 2023 16:54:36 +0200 Subject: [PATCH 32/38] test(e2e): Add test for Next.js middleware (#8350) --- .../nextjs-app-dir/middleware.ts | 15 +++++ .../nextjs-app-dir/pages/api/edge-endpoint.ts | 18 +++++- .../pages/api/endpoint-behind-middleware.ts | 9 +++ .../pages/api/error-edge-endpoint.ts | 5 ++ .../nextjs-app-dir/tests/edge-route.test.ts | 56 +++++++++++++++++++ .../nextjs-app-dir/tests/middleware.test.ts | 53 ++++++++++++++++++ 6 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/endpoint-behind-middleware.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts create mode 100644 packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts new file mode 100644 index 000000000000..bb9db27b50d7 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/middleware.ts @@ -0,0 +1,15 @@ +import { NextResponse } from 'next/server'; +import type { NextRequest } from 'next/server'; + +export function middleware(request: NextRequest) { + if (request.headers.has('x-should-throw')) { + throw new Error('Middleware Error'); + } + + return NextResponse.next(); +} + +// See "Matching Paths" below to learn more +export const config = { + matcher: ['/api/endpoint-behind-middleware'], +}; diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts index d8af89f2e9d5..b2b2dfdf4fc3 100644 --- a/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/edge-endpoint.ts @@ -1,3 +1,17 @@ -export const config = { runtime: 'edge' }; +export const config = { + runtime: 'edge', +}; -export default () => new Response('Hello world!'); +export default async function handler() { + return new Response( + JSON.stringify({ + name: 'Jim Halpert', + }), + { + status: 200, + headers: { + 'content-type': 'application/json', + }, + }, + ); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/endpoint-behind-middleware.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/endpoint-behind-middleware.ts new file mode 100644 index 000000000000..2ca75a33ba7e --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/endpoint-behind-middleware.ts @@ -0,0 +1,9 @@ +import type { NextApiRequest, NextApiResponse } from 'next'; + +type Data = { + name: string; +}; + +export default function handler(req: NextApiRequest, res: NextApiResponse) { + res.status(200).json({ name: 'John Doe' }); +} diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts new file mode 100644 index 000000000000..043112494c23 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/pages/api/error-edge-endpoint.ts @@ -0,0 +1,5 @@ +export const config = { runtime: 'edge' }; + +export default () => { + throw new Error('Edge Route Error'); +}; diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts new file mode 100644 index 000000000000..8f73764a919f --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/edge-route.test.ts @@ -0,0 +1,56 @@ +import { test, expect } from '@playwright/test'; +import { waitForTransaction, waitForError } from '../../../test-utils/event-proxy-server'; + +test('Should create a transaction for edge routes', async ({ request }) => { + test.skip(process.env.TEST_ENV === 'development', "Doesn't work in dev mode."); + + const edgerouteTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'GET /api/edge-endpoint' && transactionEvent?.contexts?.trace?.status === 'ok' + ); + }); + + const response = await request.get('/api/edge-endpoint'); + expect(await response.json()).toStrictEqual({ name: 'Jim Halpert' }); + + const edgerouteTransaction = await edgerouteTransactionPromise; + + expect(edgerouteTransaction.contexts?.trace?.status).toBe('ok'); + expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server'); + expect(edgerouteTransaction.contexts?.runtime?.name).toBe('edge'); +}); + +test('Should create a transaction with error status for faulty edge routes', async ({ request }) => { + test.skip(process.env.TEST_ENV === 'development', "Doesn't work in dev mode."); + + const edgerouteTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'GET /api/error-edge-endpoint' && + transactionEvent?.contexts?.trace?.status === 'internal_error' + ); + }); + + request.get('/api/error-edge-endpoint').catch(() => { + // Noop + }); + + const edgerouteTransaction = await edgerouteTransactionPromise; + + expect(edgerouteTransaction.contexts?.trace?.status).toBe('internal_error'); + expect(edgerouteTransaction.contexts?.trace?.op).toBe('http.server'); + expect(edgerouteTransaction.contexts?.runtime?.name).toBe('edge'); +}); + +test('Should record exceptions for faulty edge routes', async ({ request }) => { + test.skip(process.env.TEST_ENV === 'development', "Doesn't work in dev mode."); + + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Edge Route Error'; + }); + + request.get('/api/error-edge-endpoint').catch(() => { + // Noop + }); + + expect(await errorEventPromise).toBeDefined(); +}); diff --git a/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts new file mode 100644 index 000000000000..268a55f1f481 --- /dev/null +++ b/packages/e2e-tests/test-applications/nextjs-app-dir/tests/middleware.test.ts @@ -0,0 +1,53 @@ +import { test, expect } from '@playwright/test'; +import { waitForTransaction, waitForError } from '../../../test-utils/event-proxy-server'; + +test('Should create a transaction for middleware', async ({ request }) => { + test.skip(process.env.TEST_ENV === 'development', "Doesn't work in dev mode."); + + const middlewareTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return transactionEvent?.transaction === 'middleware' && transactionEvent?.contexts?.trace?.status === 'ok'; + }); + + const response = await request.get('/api/endpoint-behind-middleware'); + expect(await response.json()).toStrictEqual({ name: 'John Doe' }); + + const middlewareTransaction = await middlewareTransactionPromise; + + expect(middlewareTransaction.contexts?.trace?.status).toBe('ok'); + expect(middlewareTransaction.contexts?.trace?.op).toBe('middleware.nextjs'); + expect(middlewareTransaction.contexts?.runtime?.name).toBe('edge'); +}); + +test('Should create a transaction with error status for faulty middleware', async ({ request }) => { + test.skip(process.env.TEST_ENV === 'development', "Doesn't work in dev mode."); + + const middlewareTransactionPromise = waitForTransaction('nextjs-13-app-dir', async transactionEvent => { + return ( + transactionEvent?.transaction === 'middleware' && transactionEvent?.contexts?.trace?.status === 'internal_error' + ); + }); + + request.get('/api/endpoint-behind-middleware', { headers: { 'x-should-throw': '1' } }).catch(() => { + // Noop + }); + + const middlewareTransaction = await middlewareTransactionPromise; + + expect(middlewareTransaction.contexts?.trace?.status).toBe('internal_error'); + expect(middlewareTransaction.contexts?.trace?.op).toBe('middleware.nextjs'); + expect(middlewareTransaction.contexts?.runtime?.name).toBe('edge'); +}); + +test('Records exceptions happening in middleware', async ({ request }) => { + test.skip(process.env.TEST_ENV === 'development', "Doesn't work in dev mode."); + + const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => { + return errorEvent?.exception?.values?.[0]?.value === 'Middleware Error'; + }); + + request.get('/api/endpoint-behind-middleware', { headers: { 'x-should-throw': '1' } }).catch(() => { + // Noop + }); + + expect(await errorEventPromise).toBeDefined(); +}); From 9d8464b0a6b8d72782d7b74a1a4ad7355abf2d2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Tue, 20 Jun 2023 11:19:18 +0200 Subject: [PATCH 33/38] chore(profiling): Move profiling types to types and cache to utils (#8270) --- packages/browser/src/profiling/cache.ts | 72 +----------------- .../browser/src/profiling/jsSelfProfiling.ts | 60 --------------- packages/browser/src/profiling/utils.ts | 10 +-- packages/hub/package.json | 3 +- packages/types/src/index.ts | 10 +++ packages/types/src/profiling.ts | 76 +++++++++++++++++++ packages/utils/src/cache.ts | 68 +++++++++++++++++ packages/utils/src/index.ts | 1 + 8 files changed, 162 insertions(+), 138 deletions(-) create mode 100644 packages/types/src/profiling.ts create mode 100644 packages/utils/src/cache.ts diff --git a/packages/browser/src/profiling/cache.ts b/packages/browser/src/profiling/cache.ts index ee62538e60cb..34b5da5d12fa 100644 --- a/packages/browser/src/profiling/cache.ts +++ b/packages/browser/src/profiling/cache.ts @@ -1,72 +1,4 @@ import type { Event } from '@sentry/types'; +import { makeFifoCache } from '@sentry/utils'; -/** - * Creates a cache that evicts keys in fifo order - * @param size {Number} - */ -export function makeProfilingCache( - size: number, -): { - get: (key: Key) => Value | undefined; - add: (key: Key, value: Value) => void; - delete: (key: Key) => boolean; - clear: () => void; - size: () => number; -} { - // Maintain a fifo queue of keys, we cannot rely on Object.keys as the browser may not support it. - let evictionOrder: Key[] = []; - let cache: Record = {}; - - return { - add(key: Key, value: Value) { - while (evictionOrder.length >= size) { - // shift is O(n) but this is small size and only happens if we are - // exceeding the cache size so it should be fine. - const evictCandidate = evictionOrder.shift(); - - if (evictCandidate !== undefined) { - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete cache[evictCandidate]; - } - } - - // in case we have a collision, delete the old key. - if (cache[key]) { - this.delete(key); - } - - evictionOrder.push(key); - cache[key] = value; - }, - clear() { - cache = {}; - evictionOrder = []; - }, - get(key: Key): Value | undefined { - return cache[key]; - }, - size() { - return evictionOrder.length; - }, - // Delete cache key and return true if it existed, false otherwise. - delete(key: Key): boolean { - if (!cache[key]) { - return false; - } - - // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete cache[key]; - - for (let i = 0; i < evictionOrder.length; i++) { - if (evictionOrder[i] === key) { - evictionOrder.splice(i, 1); - break; - } - } - - return true; - }, - }; -} - -export const PROFILING_EVENT_CACHE = makeProfilingCache(20); +export const PROFILING_EVENT_CACHE = makeFifoCache(20); diff --git a/packages/browser/src/profiling/jsSelfProfiling.ts b/packages/browser/src/profiling/jsSelfProfiling.ts index af9ebada8cbf..efa4a0a0a0bc 100644 --- a/packages/browser/src/profiling/jsSelfProfiling.ts +++ b/packages/browser/src/profiling/jsSelfProfiling.ts @@ -53,63 +53,3 @@ declare global { export interface RawThreadCpuProfile extends JSSelfProfile { profile_id: string; } -export interface ThreadCpuProfile { - samples: { - stack_id: number; - thread_id: string; - elapsed_since_start_ns: string; - }[]; - stacks: number[][]; - frames: { - function: string; - file: string | undefined; - line: number | undefined; - column: number | undefined; - }[]; - thread_metadata: Record; - queue_metadata?: Record; -} - -export interface SentryProfile { - event_id: string; - version: string; - os: { - name: string; - version: string; - build_number: string; - }; - runtime: { - name: string; - version: string; - }; - device: { - architecture: string; - is_emulator: boolean; - locale: string; - manufacturer: string; - model: string; - }; - timestamp: string; - release: string; - environment: string; - platform: string; - profile: ThreadCpuProfile; - debug_meta?: { - images: { - debug_id: string; - image_addr: string; - code_file: string; - type: string; - image_size: number; - image_vmaddr: string; - }[]; - }; - transactions: { - name: string; - trace_id: string; - id: string; - active_thread_id: string; - relative_start_ns: string; - relative_end_ns: string; - }[]; -} diff --git a/packages/browser/src/profiling/utils.ts b/packages/browser/src/profiling/utils.ts index 9fc068f7c0ee..7b2e1b60e848 100644 --- a/packages/browser/src/profiling/utils.ts +++ b/packages/browser/src/profiling/utils.ts @@ -6,19 +6,15 @@ import type { EventEnvelope, EventEnvelopeHeaders, EventItem, + Profile as SentryProfile, SdkInfo, SdkMetadata, + ThreadCpuProfile, } from '@sentry/types'; import { createEnvelope, dropUndefinedKeys, dsnToString, logger, uuid4 } from '@sentry/utils'; import { WINDOW } from '../helpers'; -import type { - JSSelfProfile, - JSSelfProfileStack, - RawThreadCpuProfile, - SentryProfile, - ThreadCpuProfile, -} from './jsSelfProfiling'; +import type { JSSelfProfile, JSSelfProfileStack, RawThreadCpuProfile } from './jsSelfProfiling'; const MS_TO_NS = 1e6; // Use 0 as main thread id which is identical to threadId in node:worker_threads diff --git a/packages/hub/package.json b/packages/hub/package.json index ca34143369db..bc795aa6bee5 100644 --- a/packages/hub/package.json +++ b/packages/hub/package.json @@ -40,7 +40,8 @@ "lint:eslint": "eslint . --format stylish", "lint:prettier": "prettier --check \"{src,test,scripts}/**/**.ts\"", "test": "jest", - "test:watch": "jest --watch" + "test:watch": "jest --watch", + "yalc:publish": "ts-node ../../scripts/prepack.ts && yalc publish build --push" }, "volta": { "extends": "../../package.json" diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 1da1778d2012..ccabae59a995 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -49,6 +49,16 @@ export type { ExtractedNodeRequestData, HttpHeaderValue, Primitive, WorkerLocati export type { ClientOptions, Options } from './options'; export type { Package } from './package'; export type { PolymorphicEvent, PolymorphicRequest } from './polymorphics'; +export type { + ThreadId, + FrameId, + StackId, + ThreadCpuSample, + ThreadCpuStack, + ThreadCpuFrame, + ThreadCpuProfile, + Profile, +} from './profiling'; export type { ReplayEvent, ReplayRecordingData, ReplayRecordingMode } from './replay'; export type { QueryParams, Request, SanitizedRequestData } from './request'; export type { Runtime } from './runtime'; diff --git a/packages/types/src/profiling.ts b/packages/types/src/profiling.ts new file mode 100644 index 000000000000..1b6dac566901 --- /dev/null +++ b/packages/types/src/profiling.ts @@ -0,0 +1,76 @@ +export type ThreadId = string; +export type FrameId = number; +export type StackId = number; + +export interface ThreadCpuSample { + stack_id: StackId; + thread_id: ThreadId; + elapsed_since_start_ns: string; +} + +export type ThreadCpuStack = FrameId[]; + +export type ThreadCpuFrame = { + function: string; + file?: string; + line?: number; + column?: number; +}; + +export interface ThreadCpuProfile { + samples: ThreadCpuSample[]; + stacks: ThreadCpuStack[]; + frames: ThreadCpuFrame[]; + thread_metadata: Record; + queue_metadata?: Record; +} + +export interface Profile { + event_id: string; + version: string; + os: { + name: string; + version: string; + build_number?: string; + }; + runtime: { + name: string; + version: string; + }; + device: { + architecture: string; + is_emulator: boolean; + locale: string; + manufacturer: string; + model: string; + }; + timestamp: string; + release: string; + environment: string; + platform: string; + profile: ThreadCpuProfile; + debug_meta?: { + images: { + debug_id: string; + image_addr: string; + code_file: string; + type: string; + image_size: number; + image_vmaddr: string; + }[]; + }; + transaction?: { + name: string; + id: string; + trace_id: string; + active_thread_id: string; + }; + transactions?: { + name: string; + id: string; + trace_id: string; + active_thread_id: string; + relative_start_ns: string; + relative_end_ns: string; + }[]; +} diff --git a/packages/utils/src/cache.ts b/packages/utils/src/cache.ts new file mode 100644 index 000000000000..412970e77c76 --- /dev/null +++ b/packages/utils/src/cache.ts @@ -0,0 +1,68 @@ +/** + * Creates a cache that evicts keys in fifo order + * @param size {Number} + */ +export function makeFifoCache( + size: number, +): { + get: (key: Key) => Value | undefined; + add: (key: Key, value: Value) => void; + delete: (key: Key) => boolean; + clear: () => void; + size: () => number; +} { + // Maintain a fifo queue of keys, we cannot rely on Object.keys as the browser may not support it. + let evictionOrder: Key[] = []; + let cache: Record = {}; + + return { + add(key: Key, value: Value) { + while (evictionOrder.length >= size) { + // shift is O(n) but this is small size and only happens if we are + // exceeding the cache size so it should be fine. + const evictCandidate = evictionOrder.shift(); + + if (evictCandidate !== undefined) { + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete cache[evictCandidate]; + } + } + + // in case we have a collision, delete the old key. + if (cache[key]) { + this.delete(key); + } + + evictionOrder.push(key); + cache[key] = value; + }, + clear() { + cache = {}; + evictionOrder = []; + }, + get(key: Key): Value | undefined { + return cache[key]; + }, + size() { + return evictionOrder.length; + }, + // Delete cache key and return true if it existed, false otherwise. + delete(key: Key): boolean { + if (!cache[key]) { + return false; + } + + // eslint-disable-next-line @typescript-eslint/no-dynamic-delete + delete cache[key]; + + for (let i = 0; i < evictionOrder.length; i++) { + if (evictionOrder[i] === key) { + evictionOrder.splice(i, 1); + break; + } + } + + return true; + }, + }; +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index c5631559a9aa..6b9426c22149 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -28,3 +28,4 @@ export * from './ratelimit'; export * from './baggage'; export * from './url'; export * from './userIntegrations'; +export * from './cache'; From c8686ffaee0844d86d6684df742dd2bf45792aae Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 20 Jun 2023 14:22:36 +0200 Subject: [PATCH 34/38] fix(core): Only start spans in `trace` if tracing is enabled (#8357) In our (still internal) `trace` function, we don't yet check if `hasTracingEnabled` returns `true` before trying to start a span or a transaction. This leads to the following problematic UX path that was discovered [here](https://github.com/getsentry/sentry-javascript/discussions/5838#discussioncomment-6217855): 1. Users don't set `tracesSampleRate` (or `tracesSampler`), causing some SDKs (NextJS, SvelteKit) to **not** add the `BrowserTracing` integration, which in turn means that tracing extension methods are not added/registered. 2. Users or, more likely, other parts of our SDK (e.g. SvelteKit's wrappers/handlers) call `trace` which will still try to start a span/txn. 3. Users will get a console warning that tracing extension methods were not installed and they should manually call `addExtensionMethods` which is completely misleading in this case. This fix makes `trace` check for `hasTracingEnabled()` in which case it will not start a span but just invoke the error callback if an error occurred. --- packages/core/src/tracing/trace.ts | 16 ++++++++--- packages/core/test/lib/tracing/trace.test.ts | 28 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 8e7844d23988..2864377bfc04 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -2,13 +2,15 @@ import type { TransactionContext } from '@sentry/types'; import { isThenable } from '@sentry/utils'; import { getCurrentHub } from '../hub'; +import { hasTracingEnabled } from '../utils/hasTracingEnabled'; import type { Span } from './span'; /** * Wraps a function with a transaction/span and finishes the span after the function is done. * - * Note that if you have not enabled tracing extensions via `addTracingExtensions`, this function - * will not generate spans, and the `span` returned from the callback may be undefined. + * Note that if you have not enabled tracing extensions via `addTracingExtensions` + * or you didn't set `tracesSampleRate`, this function will not generate spans + * and the `span` returned from the callback will be undefined. * * This function is meant to be used internally and may break at any time. Use at your own risk. * @@ -31,7 +33,15 @@ export function trace( const scope = hub.getScope(); const parentSpan = scope.getSpan(); - const activeSpan = parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx); + + function getActiveSpan(): Span | undefined { + if (!hasTracingEnabled()) { + return undefined; + } + return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx); + } + + const activeSpan = getActiveSpan(); scope.setSpan(activeSpan); function finishAndSetSpan(): void { diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 064c41dc123a..bff1c425c2a0 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -185,5 +185,33 @@ describe('trace', () => { } expect(onError).toHaveBeenCalledTimes(isError ? 1 : 0); }); + + it("doesn't create spans but calls onError if tracing is disabled", async () => { + const options = getDefaultTestClientOptions({ + /* we don't set tracesSampleRate or tracesSampler */ + }); + client = new TestClient(options); + hub = new Hub(client); + makeMain(hub); + + const startTxnSpy = jest.spyOn(hub, 'startTransaction'); + + const onError = jest.fn(); + try { + await trace( + { name: 'GET users/[id]' }, + () => { + return callback(); + }, + onError, + ); + } catch (e) { + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith(e); + } + expect(onError).toHaveBeenCalledTimes(isError ? 1 : 0); + + expect(startTxnSpy).not.toHaveBeenCalled(); + }); }); }); From 365c75085e85af615958d28c0202c587fce129bb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 20 Jun 2023 16:26:55 +0200 Subject: [PATCH 35/38] fix(angular): Stop routing spans on navigation cancel and error events (#8369) Previously, in our Angular routing instrumentation, we only stopped routing spans when a navigation ended successfully. This was because we only listened to [`NavigationEnd`](https://angular.io/api/router/NavigationEnd) router events. However, the Angular router emits other events in unsuccessful routing attempts: * a routing error (e.g. unknown route) emits [`NavigationError`](https://angular.io/api/router/NavigationCancel) * a cancelled navigation (e.g. due to a route guard that rejected/returned false) emits [`NavigationCancel`](https://angular.io/api/router/NavigationCancel) This fix adjusts our instrumentation to also listen to these events and to stop the span accordingly. --- packages/angular/src/tracing.ts | 6 ++- packages/angular/test/tracing.test.ts | 62 ++++++++++++++++++++++++++- packages/angular/test/utils/index.ts | 22 +++++++--- 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index b206d7fe429d..f2e79adfe9b0 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -5,7 +5,7 @@ import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router // Duplicated import to work around a TypeScript bug where it'd complain that `Router` isn't imported as a type. // We need to import it as a value to satisfy Angular dependency injection. So: // eslint-disable-next-line @typescript-eslint/consistent-type-imports, import/no-duplicates -import { Router } from '@angular/router'; +import { NavigationCancel, NavigationError, Router } from '@angular/router'; // eslint-disable-next-line import/no-duplicates import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router'; import { getCurrentHub, WINDOW } from '@sentry/browser'; @@ -131,7 +131,9 @@ export class TraceService implements OnDestroy { ); public navEnd$: Observable = this._router.events.pipe( - filter(event => event instanceof NavigationEnd), + filter( + event => event instanceof NavigationEnd || event instanceof NavigationCancel || event instanceof NavigationError, + ), tap(() => { if (this._routingSpan) { runOutsideAngular(() => { diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts index 0afef2771add..a3375518466a 100644 --- a/packages/angular/test/tracing.test.ts +++ b/packages/angular/test/tracing.test.ts @@ -1,5 +1,5 @@ import { Component } from '@angular/core'; -import type { ActivatedRouteSnapshot } from '@angular/router'; +import type { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot } from '@angular/router'; import type { Hub } from '@sentry/types'; import { instrumentAngularRouting, TraceClassDecorator, TraceDirective, TraceMethodDecorator } from '../src'; @@ -185,6 +185,66 @@ describe('Angular Tracing', () => { env.destroy(); }); + it('finishes routing span on navigation error', async () => { + const customStartTransaction = jest.fn(defaultStartTransaction); + + const env = await TestEnv.setup({ + customStartTransaction, + routes: [ + { + path: '', + component: AppComponent, + }, + ], + useTraceService: true, + }); + + const finishMock = jest.fn(); + transaction.startChild = jest.fn(() => ({ + finish: finishMock, + })); + + await env.navigateInAngular('/somewhere'); + + expect(finishMock).toHaveBeenCalledTimes(1); + + env.destroy(); + }); + + it('finishes routing span on navigation cancel', async () => { + const customStartTransaction = jest.fn(defaultStartTransaction); + + class CanActivateGuard implements CanActivate { + canActivate(_route: ActivatedRouteSnapshot, _state: RouterStateSnapshot): boolean { + return false; + } + } + + const env = await TestEnv.setup({ + customStartTransaction, + routes: [ + { + path: 'cancel', + component: AppComponent, + canActivate: [CanActivateGuard], + }, + ], + useTraceService: true, + additionalProviders: [{ provide: CanActivateGuard, useClass: CanActivateGuard }], + }); + + const finishMock = jest.fn(); + transaction.startChild = jest.fn(() => ({ + finish: finishMock, + })); + + await env.navigateInAngular('/cancel'); + + expect(finishMock).toHaveBeenCalledTimes(1); + + env.destroy(); + }); + describe('URL parameterization', () => { it.each([ [ diff --git a/packages/angular/test/utils/index.ts b/packages/angular/test/utils/index.ts index b15ad2028560..daa23155d931 100644 --- a/packages/angular/test/utils/index.ts +++ b/packages/angular/test/utils/index.ts @@ -1,3 +1,4 @@ +import type { Provider } from '@angular/core'; import { Component, NgModule } from '@angular/core'; import type { ComponentFixture } from '@angular/core/testing'; import { TestBed } from '@angular/core/testing'; @@ -47,6 +48,7 @@ export class TestEnv { startTransactionOnPageLoad?: boolean; startTransactionOnNavigation?: boolean; useTraceService?: boolean; + additionalProviders?: Provider[]; }): Promise { instrumentAngularRouting( conf.customStartTransaction || jest.fn(), @@ -60,14 +62,16 @@ export class TestEnv { TestBed.configureTestingModule({ imports: [AppModule, RouterTestingModule.withRoutes(routes)], declarations: [...(conf.components || []), AppComponent], - providers: useTraceService + providers: (useTraceService ? [ { provide: TraceService, deps: [Router], }, + ...(conf.additionalProviders || []), ] - : [], + : [] + ).concat(...(conf.additionalProviders || [])), }); const router: Router = TestBed.inject(Router); @@ -80,10 +84,16 @@ export class TestEnv { public async navigateInAngular(url: string): Promise { return new Promise(resolve => { return this.fixture.ngZone?.run(() => { - void this.router.navigateByUrl(url).then(() => { - this.fixture.detectChanges(); - resolve(); - }); + void this.router + .navigateByUrl(url) + .then(() => { + this.fixture.detectChanges(); + resolve(); + }) + .catch(() => { + this.fixture.detectChanges(); + resolve(); + }); }); }); } From e5e6a6bd8ab2bbec59879971595a7248fa132826 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 20 Jun 2023 16:27:53 +0200 Subject: [PATCH 36/38] fix(angular): Filter out `TryCatch` integration by default (#8367) The `TryCatch` default integration interferes with the `SentryErrorHander` error handler of the Angular(-Ivy) SDKs by catching certain errors too early, before the Angular SDK-specific error handler can catch them. This caused missing data on the event in some or duplicated errors in other cases. This fix filters out the `TryCatch` by default, as long as users didn't set `defaultIntegrations` in their SDK init. Therefore, it makes the previously provided [workaround](https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097) obsolete. --- packages/angular-ivy/src/sdk.ts | 14 +++++++++++++- packages/angular/src/sdk.ts | 14 +++++++++++++- packages/angular/test/sdk.test.ts | 31 ++++++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/angular-ivy/src/sdk.ts b/packages/angular-ivy/src/sdk.ts index d16d16009ca0..fcbbbce399d0 100644 --- a/packages/angular-ivy/src/sdk.ts +++ b/packages/angular-ivy/src/sdk.ts @@ -1,6 +1,6 @@ import { VERSION } from '@angular/core'; import type { BrowserOptions } from '@sentry/browser'; -import { init as browserInit, SDK_VERSION, setContext } from '@sentry/browser'; +import { defaultIntegrations, init as browserInit, SDK_VERSION, setContext } from '@sentry/browser'; import { logger } from '@sentry/utils'; import { IS_DEBUG_BUILD } from './flags'; @@ -21,6 +21,18 @@ export function init(options: BrowserOptions): void { version: SDK_VERSION, }; + // Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`: + // TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a + // lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide. + // see: + // - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097 + // - https://github.com/getsentry/sentry-javascript/issues/2744 + if (options.defaultIntegrations === undefined) { + options.defaultIntegrations = defaultIntegrations.filter(integration => { + return integration.name !== 'TryCatch'; + }); + } + checkAndSetAngularVersion(); browserInit(options); } diff --git a/packages/angular/src/sdk.ts b/packages/angular/src/sdk.ts index 4afce6259c2b..e50cece043d0 100755 --- a/packages/angular/src/sdk.ts +++ b/packages/angular/src/sdk.ts @@ -1,6 +1,6 @@ import { VERSION } from '@angular/core'; import type { BrowserOptions } from '@sentry/browser'; -import { init as browserInit, SDK_VERSION, setContext } from '@sentry/browser'; +import { defaultIntegrations, init as browserInit, SDK_VERSION, setContext } from '@sentry/browser'; import { logger } from '@sentry/utils'; import { IS_DEBUG_BUILD } from './flags'; @@ -21,6 +21,18 @@ export function init(options: BrowserOptions): void { version: SDK_VERSION, }; + // Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`: + // TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a + // lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide. + // see: + // - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097 + // - https://github.com/getsentry/sentry-javascript/issues/2744 + if (options.defaultIntegrations === undefined) { + options.defaultIntegrations = defaultIntegrations.filter(integration => { + return integration.name !== 'TryCatch'; + }); + } + checkAndSetAngularVersion(); browserInit(options); } diff --git a/packages/angular/test/sdk.test.ts b/packages/angular/test/sdk.test.ts index bf5ecabb0ac5..0a7244d424f7 100644 --- a/packages/angular/test/sdk.test.ts +++ b/packages/angular/test/sdk.test.ts @@ -1,6 +1,6 @@ import * as SentryBrowser from '@sentry/browser'; -import { init } from '../src/sdk'; +import { defaultIntegrations, init } from '../src/index'; describe('init', () => { it('sets the Angular version (if available) in the global scope', () => { @@ -13,4 +13,33 @@ describe('init', () => { expect(setContextSpy).toHaveBeenCalledTimes(1); expect(setContextSpy).toHaveBeenCalledWith('angular', { version: 10 }); }); + + describe('filtering out the `TryCatch` integration', () => { + const browserInitSpy = jest.spyOn(SentryBrowser, 'init'); + + beforeEach(() => { + browserInitSpy.mockClear(); + }); + + it('filters if `defaultIntegrations` is not set', () => { + init({}); + + expect(browserInitSpy).toHaveBeenCalledTimes(1); + + const options = browserInitSpy.mock.calls[0][0] || {}; + expect(options.defaultIntegrations).not.toContainEqual(expect.objectContaining({ name: 'TryCatch' })); + }); + + it.each([false as const, defaultIntegrations])( + "doesn't filter if `defaultIntegrations` is set to %s", + defaultIntegrations => { + init({ defaultIntegrations }); + + expect(browserInitSpy).toHaveBeenCalledTimes(1); + + const options = browserInitSpy.mock.calls[0][0] || {}; + expect(options.defaultIntegrations).toEqual(defaultIntegrations); + }, + ); + }); }); From b84c23f6a43014396a8d89de827b1c1ad17bd737 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 21 Jun 2023 09:16:54 +0200 Subject: [PATCH 37/38] test(e2e): Ensure sveltekit E2E test work with prereleases (#8372) Since @sentry/sveltekit depends on @sentry/vite-plugin, which in turn depens on `@sentry/node@^7.19.0` & `@sentry/tracing@^7.19.0`, this fails to install in E2E tests for pre-release versions (e.g. `7.57.0-beta.0`), as the prerelease does not satisfy the range `^7.19.0`. So we override this to `*` to ensure this works as expected. --- packages/e2e-tests/test-applications/sveltekit/package.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/e2e-tests/test-applications/sveltekit/package.json b/packages/e2e-tests/test-applications/sveltekit/package.json index ab1a2c9cac2e..003868af5ab8 100644 --- a/packages/e2e-tests/test-applications/sveltekit/package.json +++ b/packages/e2e-tests/test-applications/sveltekit/package.json @@ -27,5 +27,11 @@ "vite": "^4.2.0", "wait-port": "1.0.4" }, + "pnpm": { + "overrides": { + "@sentry/node": "*", + "@sentry/tracing": "*" + } + }, "type": "module" } From 4fb043e875a50611f77ff7c1904b8e0c0fce0e2c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 21 Jun 2023 09:49:14 +0200 Subject: [PATCH 38/38] fix(nextjs): Inject init calls via loader instead of via entrypoints (#8368) --- .github/workflows/build.yml | 2 +- .../src/config/loaders/wrappingLoader.ts | 18 +-- packages/nextjs/src/config/webpack.ts | 42 +---- .../webpack/constructWebpackConfig.test.ts | 149 ------------------ 4 files changed, 18 insertions(+), 193 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 990361d6ae7c..3bc95b171d94 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -424,7 +424,7 @@ jobs: name: Nextjs (Node ${{ matrix.node }}) Tests needs: [job_get_metadata, job_build] if: needs.job_get_metadata.outputs.changed_nextjs == 'true' || github.event_name != 'pull_request' - timeout-minutes: 15 + timeout-minutes: 25 runs-on: ubuntu-20.04 strategy: fail-fast: false diff --git a/packages/nextjs/src/config/loaders/wrappingLoader.ts b/packages/nextjs/src/config/loaders/wrappingLoader.ts index 3157d41df71f..e4d58c579420 100644 --- a/packages/nextjs/src/config/loaders/wrappingLoader.ts +++ b/packages/nextjs/src/config/loaders/wrappingLoader.ts @@ -201,21 +201,21 @@ export default function wrappingLoader( } else { templateCode = templateCode.replace(/__COMPONENT_TYPE__/g, 'Unknown'); } - - // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, - // however we can only create relative paths to the sentry config from absolute paths.Examples where this could possibly be non - absolute are virtual modules. - if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { - const sentryConfigImportPath = path - .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 - .replace(/\\/g, '/'); - templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); - } } else if (wrappingTargetKind === 'middleware') { templateCode = middlewareWrapperTemplateCode; } else { throw new Error(`Invariant: Could not get template code of unknown kind "${wrappingTargetKind}"`); } + // We check whether `this.resourcePath` is absolute because there is no contract by webpack that says it is absolute, + // however we can only create relative paths to the sentry config from absolute paths.Examples where this could possibly be non - absolute are virtual modules. + if (sentryConfigFilePath && path.isAbsolute(this.resourcePath)) { + const sentryConfigImportPath = path + .relative(path.dirname(this.resourcePath), sentryConfigFilePath) // Absolute paths do not work with Windows: https://github.com/getsentry/sentry-javascript/issues/8133 + .replace(/\\/g, '/'); + templateCode = `import "${sentryConfigImportPath}";\n`.concat(templateCode); + } + // Replace the import path of the wrapping target in the template with a path that the `wrapUserCode` function will understand. templateCode = templateCode.replace(/__SENTRY_WRAPPING_TARGET_FILE__/g, WRAPPING_TARGET_MODULE_NAME); diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts index 0bb42f98b7ec..7237d8ce24be 100644 --- a/packages/nextjs/src/config/webpack.ts +++ b/packages/nextjs/src/config/webpack.ts @@ -1,7 +1,7 @@ /* eslint-disable complexity */ /* eslint-disable max-lines */ import { getSentryRelease } from '@sentry/node'; -import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger, stringMatchesSomePattern } from '@sentry/utils'; +import { arrayify, dropUndefinedKeys, escapeStringForRegex, logger } from '@sentry/utils'; import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin'; import * as chalk from 'chalk'; import * as fs from 'fs'; @@ -441,7 +441,7 @@ async function addSentryToEntryProperty( // inject into all entry points which might contain user's code for (const entryPointName in newEntryProperty) { - if (shouldAddSentryToEntryPoint(entryPointName, runtime, userSentryOptions.excludeServerRoutes ?? [])) { + if (shouldAddSentryToEntryPoint(entryPointName, runtime)) { addFilesToExistingEntryPoint(newEntryProperty, entryPointName, filesToInject); } else { if ( @@ -589,39 +589,13 @@ function checkWebpackPluginOverrides( * @param excludeServerRoutes A list of excluded serverside entrypoints provided by the user * @returns `true` if sentry code should be injected, and `false` otherwise */ -function shouldAddSentryToEntryPoint( - entryPointName: string, - runtime: 'node' | 'browser' | 'edge', - excludeServerRoutes: Array, -): boolean { - // On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions). - if (runtime === 'node') { - // User-specified pages to skip. (Note: For ease of use, `excludeServerRoutes` is specified in terms of routes, - // which don't have the `pages` prefix.) - const entryPointRoute = entryPointName.replace(/^pages/, ''); - if (stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true)) { - return false; - } - - // This expression will implicitly include `pages/_app` which is called for all serverside routes and pages - // regardless whether or not the user has a`_app` file. - return entryPointName.startsWith('pages/'); - } else if (runtime === 'browser') { - return ( - // entrypoint for `/pages` pages - this is included on all clientside pages - // It's important that we inject the SDK into this file and not into 'main' because in 'main' - // some important Next.js code (like the setup code for getCongig()) is located and some users - // may need this code inside their Sentry configs - entryPointName === 'pages/_app' || +function shouldAddSentryToEntryPoint(entryPointName: string, runtime: 'node' | 'browser' | 'edge'): boolean { + return ( + runtime === 'browser' && + (entryPointName === 'pages/_app' || // entrypoint for `/app` pages - entryPointName === 'main-app' - ); - } else { - // User-specified pages to skip. (Note: For ease of use, `excludeServerRoutes` is specified in terms of routes, - // which don't have the `pages` prefix.) - const entryPointRoute = entryPointName.replace(/^pages/, ''); - return !stringMatchesSomePattern(entryPointRoute, excludeServerRoutes, true); - } + entryPointName === 'main-app') + ); } /** diff --git a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts index 01f89bed1077..86bb2d03d3fd 100644 --- a/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts +++ b/packages/nextjs/test/config/webpack/constructWebpackConfig.test.ts @@ -7,10 +7,7 @@ import { CLIENT_SDK_CONFIG_FILE, clientBuildContext, clientWebpackConfig, - EDGE_SDK_CONFIG_FILE, - edgeBuildContext, exportedNextConfig, - SERVER_SDK_CONFIG_FILE, serverBuildContext, serverWebpackConfig, userNextConfig, @@ -88,74 +85,15 @@ describe('constructWebpackConfigFunction()', () => { }); describe('webpack `entry` property config', () => { - const serverConfigFilePath = `./${SERVER_SDK_CONFIG_FILE}`; const clientConfigFilePath = `./${CLIENT_SDK_CONFIG_FILE}`; - const edgeConfigFilePath = `./${EDGE_SDK_CONFIG_FILE}`; - - it('handles various entrypoint shapes', async () => { - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - // original entrypoint value is a string - // (was 'private-next-pages/_error.js') - 'pages/_error': [serverConfigFilePath, 'private-next-pages/_error.js'], - - // original entrypoint value is a string array - // (was ['./node_modules/smellOVision/index.js', 'private-next-pages/sniffTour.js']) - 'pages/sniffTour': [ - serverConfigFilePath, - './node_modules/smellOVision/index.js', - 'private-next-pages/sniffTour.js', - ], - - // original entrypoint value is an object containing a string `import` value - // (was { import: 'private-next-pages/api/simulator/dogStats/[name].js' }) - 'pages/api/simulator/dogStats/[name]': { - import: [serverConfigFilePath, 'private-next-pages/api/simulator/dogStats/[name].js'], - }, - - // original entrypoint value is an object containing a string array `import` value - // (was { import: ['./node_modules/dogPoints/converter.js', 'private-next-pages/simulator/leaderboard.js'] }) - 'pages/simulator/leaderboard': { - import: [ - serverConfigFilePath, - './node_modules/dogPoints/converter.js', - 'private-next-pages/simulator/leaderboard.js', - ], - }, - - // original entrypoint value is an object containg properties besides `import` - // (was { import: 'private-next-pages/api/tricks/[trickName].js', dependOn: 'treats', }) - 'pages/api/tricks/[trickName]': { - import: [serverConfigFilePath, 'private-next-pages/api/tricks/[trickName].js'], - dependOn: 'treats', // untouched - }, - }), - ); - }); it('injects user config file into `_app` in server bundle and in the client bundle', async () => { - const finalServerWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); const finalClientWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, incomingWebpackConfig: clientWebpackConfig, incomingWebpackBuildContext: clientBuildContext, }); - expect(finalServerWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/_app': expect.arrayContaining([serverConfigFilePath]), - }), - ); expect(finalClientWebpackConfig.entry).toEqual( expect.objectContaining({ 'pages/_app': expect.arrayContaining([clientConfigFilePath]), @@ -163,68 +101,6 @@ describe('constructWebpackConfigFunction()', () => { ); }); - it('injects user config file into `_error` in server bundle but not client bundle', async () => { - const finalServerWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - const finalClientWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: clientWebpackConfig, - incomingWebpackBuildContext: clientBuildContext, - }); - - expect(finalServerWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/_error': expect.arrayContaining([serverConfigFilePath]), - }), - ); - expect(finalClientWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/_error': expect.not.arrayContaining([clientConfigFilePath]), - }), - ); - }); - - it('injects user config file into both API routes and non-API routes', async () => { - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/api/simulator/dogStats/[name]': { - import: expect.arrayContaining([serverConfigFilePath]), - }, - - 'pages/api/tricks/[trickName]': expect.objectContaining({ - import: expect.arrayContaining([serverConfigFilePath]), - }), - - 'pages/simulator/leaderboard': { - import: expect.arrayContaining([serverConfigFilePath]), - }, - }), - ); - }); - - it('injects user config file into API middleware', async () => { - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: edgeBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - middleware: [edgeConfigFilePath, 'private-next-pages/middleware.js'], - }), - ); - }); - it('does not inject anything into non-_app pages during client build', async () => { const finalWebpackConfig = await materializeFinalWebpackConfig({ exportedNextConfig, @@ -244,30 +120,5 @@ describe('constructWebpackConfigFunction()', () => { simulatorBundle: './src/simulator/index.ts', }); }); - - it('does not inject into routes included in `excludeServerRoutes`', async () => { - const nextConfigWithExcludedRoutes = { - ...exportedNextConfig, - sentry: { - excludeServerRoutes: [/simulator/], - }, - }; - const finalWebpackConfig = await materializeFinalWebpackConfig({ - exportedNextConfig: nextConfigWithExcludedRoutes, - incomingWebpackConfig: serverWebpackConfig, - incomingWebpackBuildContext: serverBuildContext, - }); - - expect(finalWebpackConfig.entry).toEqual( - expect.objectContaining({ - 'pages/simulator/leaderboard': { - import: expect.not.arrayContaining([serverConfigFilePath]), - }, - 'pages/api/simulator/dogStats/[name]': { - import: expect.not.arrayContaining([serverConfigFilePath]), - }, - }), - ); - }); }); });