From 48156d85455f394574b1b95660087309a56df9f3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Dec 2022 09:42:43 +0100 Subject: [PATCH 1/3] ref(replay): Avoid using private `hub._withClient` This is brittle, and can actually be refactored quite easily. --- packages/replay/src/constants.ts | 5 + packages/replay/src/replay.ts | 101 ++++++------------ .../replay/src/util/createReplayEnvelope.ts | 31 ++++++ packages/replay/src/util/getReplayEvent.ts | 33 ++++++ 4 files changed, 102 insertions(+), 68 deletions(-) create mode 100644 packages/replay/src/util/createReplayEnvelope.ts create mode 100644 packages/replay/src/util/getReplayEvent.ts diff --git a/packages/replay/src/constants.ts b/packages/replay/src/constants.ts index 47da014c0e78..46b3d04bbd45 100644 --- a/packages/replay/src/constants.ts +++ b/packages/replay/src/constants.ts @@ -25,3 +25,8 @@ export const MAX_SESSION_LIFE = 1_800_000; // 30 minutes */ export const DEFAULT_SESSION_SAMPLE_RATE = 0.1; export const DEFAULT_ERROR_SAMPLE_RATE = 1.0; + +export const REPLAY_SDK_INFO = { + name: 'sentry.javascript.integration.replay', + version: __SENTRY_REPLAY_VERSION__, +}; diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index fd1f9a7dc97a..35235b42185b 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 { addGlobalEventProcessor, captureException, getCurrentHub, Scope, setContext } from '@sentry/core'; -import { Breadcrumb, Client, Event } from '@sentry/types'; -import { addInstrumentationHandler, createEnvelope, logger } from '@sentry/utils'; +import { addGlobalEventProcessor, captureException, getCurrentHub, setContext } from '@sentry/core'; +import { Breadcrumb, Event } from '@sentry/types'; +import { addInstrumentationHandler, logger } from '@sentry/utils'; import debounce from 'lodash.debounce'; import { EventType, record } from 'rrweb'; @@ -44,6 +44,8 @@ import { addMemoryEntry } from './util/addMemoryEntry'; import { createBreadcrumb } from './util/createBreadcrumb'; import { createPayload } from './util/createPayload'; import { createPerformanceSpans } from './util/createPerformanceSpans'; +import { createReplayEnvelope } from './util/createReplayEnvelope'; +import { getReplayEvent } from './util/getReplayEvent'; import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; @@ -906,7 +908,7 @@ export class ReplayContainer implements ReplayContainerInterface { */ async sendReplayRequest({ events, - replayId: event_id, + replayId, segmentId: segment_id, includeReplayStartTimestamp, eventContext, @@ -922,77 +924,40 @@ export class ReplayContainer implements ReplayContainerInterface { const currentTimestamp = new Date().getTime(); - const sdkInfo = { - name: 'sentry.javascript.integration.replay', - version: __SENTRY_REPLAY_VERSION__, - }; + const hub = getCurrentHub(); + const client = hub.getClient(); + const scope = hub.getScope(); + const transport = client?.getTransport(); - const replayEvent = await new Promise(resolve => { - getCurrentHub() - // @ts-ignore private api - ?._withClient(async (client: Client, scope: Scope) => { - // XXX: This event does not trigger `beforeSend` in SDK - // @ts-ignore private api - const preparedEvent: Event = await client._prepareEvent( - { - type: REPLAY_EVENT_NAME, - ...(includeReplayStartTimestamp ? { replay_start_timestamp: initialTimestamp / 1000 } : {}), - timestamp: currentTimestamp / 1000, - error_ids: errorIds, - trace_ids: traceIds, - urls, - replay_id: event_id, - segment_id, - }, - { event_id }, - scope, - ); - const session = scope && scope.getSession(); - if (session) { - // @ts-ignore private api - client._updateSessionFromEvent(session, preparedEvent); - } + if (!client || !scope || !transport) { + return; + } - preparedEvent.sdk = { - ...preparedEvent.sdk, - ...sdkInfo, - }; + const baseEvent: Event = { + // @ts-ignore private api + type: REPLAY_EVENT_NAME, + ...(includeReplayStartTimestamp ? { replay_start_timestamp: initialTimestamp / 1000 } : {}), + timestamp: currentTimestamp / 1000, + error_ids: errorIds, + trace_ids: traceIds, + urls, + replay_id: replayId, + segment_id, + }; - preparedEvent.tags = { - ...preparedEvent.tags, - sessionSampleRate: this._options.sessionSampleRate, - errorSampleRate: this._options.errorSampleRate, - replayType: this.session?.sampled, - }; + const replayEvent = await getReplayEvent({ scope, client, replayId, event: baseEvent }); - resolve(preparedEvent); - }); - }); + replayEvent.tags = { + ...replayEvent.tags, + sessionSampleRate: this._options.sessionSampleRate, + errorSampleRate: this._options.errorSampleRate, + replayType: this.session?.sampled, + }; - const envelope = createEnvelope( - { - event_id, - sent_at: new Date().toISOString(), - sdk: sdkInfo, - }, - [ - // @ts-ignore New types - [{ type: 'replay_event' }, replayEvent], - [ - { - // @ts-ignore setting envelope - type: 'replay_recording', - length: payloadWithSequence.length, - }, - // @ts-ignore: Type 'string' is not assignable to type 'ClientReport'.ts(2322) - payloadWithSequence, - ], - ], - ); + const envelope = createReplayEnvelope(replayId, replayEvent, payloadWithSequence); - const client = getCurrentHub().getClient(); try { - return client?.getTransport()?.send(envelope); + return transport.send(envelope); } catch { throw new Error(UNABLE_TO_SEND_REPLAY); } diff --git a/packages/replay/src/util/createReplayEnvelope.ts b/packages/replay/src/util/createReplayEnvelope.ts new file mode 100644 index 000000000000..91cb35044211 --- /dev/null +++ b/packages/replay/src/util/createReplayEnvelope.ts @@ -0,0 +1,31 @@ +import { Envelope, Event } from '@sentry/types'; +import { createEnvelope } from '@sentry/utils'; + +import { REPLAY_SDK_INFO } from '../constants'; + +export function createReplayEnvelope( + replayId: string, + replayEvent: Event, + payloadWithSequence: string | Uint8Array, +): Envelope { + return createEnvelope( + { + event_id: replayId, + sent_at: new Date().toISOString(), + sdk: REPLAY_SDK_INFO, + }, + [ + // @ts-ignore New types + [{ type: 'replay_event' }, replayEvent], + [ + { + // @ts-ignore setting envelope + type: 'replay_recording', + length: payloadWithSequence.length, + }, + // @ts-ignore: Type 'string' is not assignable to type 'ClientReport'.ts(2322) + payloadWithSequence, + ], + ], + ); +} diff --git a/packages/replay/src/util/getReplayEvent.ts b/packages/replay/src/util/getReplayEvent.ts new file mode 100644 index 000000000000..156e2a42970f --- /dev/null +++ b/packages/replay/src/util/getReplayEvent.ts @@ -0,0 +1,33 @@ +import { Scope } from '@sentry/core'; +import { Client, Event } from '@sentry/types'; + +import { REPLAY_SDK_INFO } from '../constants'; + +export async function getReplayEvent({ + client, + scope, + replayId: event_id, + event, +}: { + client: Client; + scope: Scope; + replayId: string; + event: Event; +}): Promise { + // XXX: This event does not trigger `beforeSend` in SDK + // @ts-ignore private api + const preparedEvent: Event = await client._prepareEvent(event, { event_id }, scope); + + const session = scope && scope.getSession(); + if (session) { + // @ts-ignore private api + client._updateSessionFromEvent(session, preparedEvent); + } + + preparedEvent.sdk = { + ...preparedEvent.sdk, + ...REPLAY_SDK_INFO, + }; + + return preparedEvent; +} From 78f62e338d97cb2bf74db74644c481a6e3b65e87 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Dec 2022 10:39:07 +0100 Subject: [PATCH 2/3] test(replay): Add test & comment for replay event creation --- packages/replay/src/replay.ts | 36 +++++++++++ .../test/unit/util/getReplayEvent.test.ts | 60 +++++++++++++++++++ .../utils/getDefaultBrowserClientOptions.ts | 12 ++++ 3 files changed, 108 insertions(+) create mode 100644 packages/replay/test/unit/util/getReplayEvent.test.ts create mode 100644 packages/replay/test/utils/getDefaultBrowserClientOptions.ts diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 35235b42185b..c016a1d71977 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -954,6 +954,42 @@ export class ReplayContainer implements ReplayContainerInterface { replayType: this.session?.sampled, }; + /* + For reference, the fully built event looks something like this: + { + "type": "replay_event", + "timestamp": 1670837008.634, + "error_ids": [ + "errorId" + ], + "trace_ids": [ + "traceId" + ], + "urls": [ + "https://example.com" + ], + "replay_id": "eventId", + "segment_id": 3, + "platform": "javascript", + "event_id": "eventId", + "environment": "production", + "sdk": { + "integrations": [ + "BrowserTracing", + "Replay" + ], + "name": "sentry.javascript.integration.replay", + "version": "7.24.2" + }, + "sdkProcessingMetadata": {}, + "tags": { + "sessionSampleRate": 1, + "errorSampleRate": 0, + "replayType": "error" + } + } + */ + const envelope = createReplayEnvelope(replayId, replayEvent, payloadWithSequence); try { diff --git a/packages/replay/test/unit/util/getReplayEvent.test.ts b/packages/replay/test/unit/util/getReplayEvent.test.ts new file mode 100644 index 000000000000..a8b8129c0af8 --- /dev/null +++ b/packages/replay/test/unit/util/getReplayEvent.test.ts @@ -0,0 +1,60 @@ +import { BrowserClient } from '@sentry/browser'; +import { getCurrentHub, Hub, Scope } from '@sentry/core'; +import { Client, Event } from '@sentry/types'; + +import { REPLAY_EVENT_NAME } from '../../../src/constants'; +import { getReplayEvent } from '../../../src/util/getReplayEvent'; +import { getDefaultBrowserClientOptions } from '../../utils/getDefaultBrowserClientOptions'; + +describe('getReplayEvent', () => { + let hub: Hub; + let client: Client; + let scope: Scope; + + beforeEach(() => { + hub = getCurrentHub(); + client = new BrowserClient(getDefaultBrowserClientOptions()); + hub.bindClient(client); + + client = hub.getClient()!; + scope = hub.getScope()!; + }); + + it('works', async () => { + expect(client).toBeDefined(); + expect(scope).toBeDefined(); + + const replayId = 'replay-ID'; + const event: Event = { + // @ts-ignore private api + type: REPLAY_EVENT_NAME, + timestamp: 1670837008.634, + error_ids: ['error-ID'], + trace_ids: ['trace-ID'], + urls: ['https://sentry.io/'], + replay_id: replayId, + segment_id: 3, + }; + + const replayEvent = await getReplayEvent({ scope, client, replayId, event }); + + expect(replayEvent).toEqual({ + type: 'replay_event', + timestamp: 1670837008.634, + error_ids: ['error-ID'], + trace_ids: ['trace-ID'], + urls: ['https://sentry.io/'], + replay_id: 'replay-ID', + segment_id: 3, + platform: 'javascript', + event_id: 'replay-ID', + environment: 'production', + sdk: { + name: 'sentry.javascript.integration.replay', + version: 'version:Test', + }, + sdkProcessingMetadata: {}, + breadcrumbs: undefined, + }); + }); +}); diff --git a/packages/replay/test/utils/getDefaultBrowserClientOptions.ts b/packages/replay/test/utils/getDefaultBrowserClientOptions.ts new file mode 100644 index 000000000000..8f99ad3a8938 --- /dev/null +++ b/packages/replay/test/utils/getDefaultBrowserClientOptions.ts @@ -0,0 +1,12 @@ +import { createTransport } from '@sentry/core'; +import { ClientOptions } from '@sentry/types'; +import { resolvedSyncPromise } from '@sentry/utils'; + +export function getDefaultBrowserClientOptions(options: Partial = {}): ClientOptions { + return { + integrations: [], + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})), + stackParser: () => [], + ...options, + }; +} From c9e24bfbdb3b699c84e0933b8ddc45f1f03cba54 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 12 Dec 2022 10:42:14 +0100 Subject: [PATCH 3/3] ref(replay): Avoid optional chaining --- packages/replay/src/replay.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index c016a1d71977..576ae1181a46 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -927,7 +927,7 @@ export class ReplayContainer implements ReplayContainerInterface { const hub = getCurrentHub(); const client = hub.getClient(); const scope = hub.getScope(); - const transport = client?.getTransport(); + const transport = client && client.getTransport(); if (!client || !scope || !transport) { return;