Skip to content

Commit 13778da

Browse files
mydeamydea
authored
ref(replay): Avoid using private hub._withClient (#6494)
This is brittle, and can actually be refactored quite easily.
1 parent b4efa21 commit 13778da

File tree

6 files changed

+209
-67
lines changed

6 files changed

+209
-67
lines changed

‎packages/replay/src/constants.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,8 @@ export const MAX_SESSION_LIFE = 1_800_000; // 30 minutes
2525
*/
2626
export const DEFAULT_SESSION_SAMPLE_RATE = 0.1;
2727
export const DEFAULT_ERROR_SAMPLE_RATE = 1.0;
28+
29+
export const REPLAY_SDK_INFO = {
30+
name: 'sentry.javascript.integration.replay',
31+
version: __SENTRY_REPLAY_VERSION__,
32+
};

‎packages/replay/src/replay.ts‎

Lines changed: 68 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable max-lines */ // TODO: We might want to split this file up
2-
import { addGlobalEventProcessor, captureException, getCurrentHub, Scope, setContext } from '@sentry/core';
3-
import { Breadcrumb, Client, Event } from '@sentry/types';
4-
import { addInstrumentationHandler, createEnvelope, logger } from '@sentry/utils';
2+
import { addGlobalEventProcessor, captureException, getCurrentHub, setContext } from '@sentry/core';
3+
import { Breadcrumb, Event } from '@sentry/types';
4+
import { addInstrumentationHandler, logger } from '@sentry/utils';
55
import debounce from 'lodash.debounce';
66
import { EventType, record } from 'rrweb';
77

@@ -44,6 +44,8 @@ import { addMemoryEntry } from './util/addMemoryEntry';
4444
import { createBreadcrumb } from './util/createBreadcrumb';
4545
import { createPayload } from './util/createPayload';
4646
import { createPerformanceSpans } from './util/createPerformanceSpans';
47+
import { createReplayEnvelope } from './util/createReplayEnvelope';
48+
import { getReplayEvent } from './util/getReplayEvent';
4749
import { isExpired } from './util/isExpired';
4850
import { isSessionExpired } from './util/isSessionExpired';
4951
import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent';
@@ -906,7 +908,7 @@ export class ReplayContainer implements ReplayContainerInterface {
906908
*/
907909
async sendReplayRequest({
908910
events,
909-
replayId: event_id,
911+
replayId,
910912
segmentId: segment_id,
911913
includeReplayStartTimestamp,
912914
eventContext,
@@ -922,77 +924,76 @@ export class ReplayContainer implements ReplayContainerInterface {
922924

923925
const currentTimestamp = new Date().getTime();
924926

925-
const sdkInfo = {
926-
name: 'sentry.javascript.integration.replay',
927-
version: __SENTRY_REPLAY_VERSION__,
928-
};
927+
const hub = getCurrentHub();
928+
const client = hub.getClient();
929+
const scope = hub.getScope();
930+
const transport = client && client.getTransport();
929931

930-
const replayEvent = await new Promise(resolve => {
931-
getCurrentHub()
932-
// @ts-ignore private api
933-
?._withClient(async (client: Client, scope: Scope) => {
934-
// XXX: This event does not trigger `beforeSend` in SDK
935-
// @ts-ignore private api
936-
const preparedEvent: Event = await client._prepareEvent(
937-
{
938-
type: REPLAY_EVENT_NAME,
939-
...(includeReplayStartTimestamp ? { replay_start_timestamp: initialTimestamp / 1000 } : {}),
940-
timestamp: currentTimestamp / 1000,
941-
error_ids: errorIds,
942-
trace_ids: traceIds,
943-
urls,
944-
replay_id: event_id,
945-
segment_id,
946-
},
947-
{ event_id },
948-
scope,
949-
);
950-
const session = scope && scope.getSession();
951-
if (session) {
952-
// @ts-ignore private api
953-
client._updateSessionFromEvent(session, preparedEvent);
954-
}
932+
if (!client || !scope || !transport) {
933+
return;
934+
}
955935

956-
preparedEvent.sdk = {
957-
...preparedEvent.sdk,
958-
...sdkInfo,
959-
};
936+
const baseEvent: Event = {
937+
// @ts-ignore private api
938+
type: REPLAY_EVENT_NAME,
939+
...(includeReplayStartTimestamp ? { replay_start_timestamp: initialTimestamp / 1000 } : {}),
940+
timestamp: currentTimestamp / 1000,
941+
error_ids: errorIds,
942+
trace_ids: traceIds,
943+
urls,
944+
replay_id: replayId,
945+
segment_id,
946+
};
960947

961-
preparedEvent.tags = {
962-
...preparedEvent.tags,
963-
sessionSampleRate: this._options.sessionSampleRate,
964-
errorSampleRate: this._options.errorSampleRate,
965-
replayType: this.session?.sampled,
966-
};
948+
const replayEvent = await getReplayEvent({ scope, client, replayId, event: baseEvent });
967949

968-
resolve(preparedEvent);
969-
});
970-
});
950+
replayEvent.tags = {
951+
...replayEvent.tags,
952+
sessionSampleRate: this._options.sessionSampleRate,
953+
errorSampleRate: this._options.errorSampleRate,
954+
replayType: this.session?.sampled,
955+
};
971956

972-
const envelope = createEnvelope(
973-
{
974-
event_id,
975-
sent_at: new Date().toISOString(),
976-
sdk: sdkInfo,
977-
},
978-
[
979-
// @ts-ignore New types
980-
[{ type: 'replay_event' }, replayEvent],
981-
[
982-
{
983-
// @ts-ignore setting envelope
984-
type: 'replay_recording',
985-
length: payloadWithSequence.length,
986-
},
987-
// @ts-ignore: Type 'string' is not assignable to type 'ClientReport'.ts(2322)
988-
payloadWithSequence,
957+
/*
958+
For reference, the fully built event looks something like this:
959+
{
960+
"type": "replay_event",
961+
"timestamp": 1670837008.634,
962+
"error_ids": [
963+
"errorId"
964+
],
965+
"trace_ids": [
966+
"traceId"
989967
],
990-
],
991-
);
968+
"urls": [
969+
"https://example.com"
970+
],
971+
"replay_id": "eventId",
972+
"segment_id": 3,
973+
"platform": "javascript",
974+
"event_id": "eventId",
975+
"environment": "production",
976+
"sdk": {
977+
"integrations": [
978+
"BrowserTracing",
979+
"Replay"
980+
],
981+
"name": "sentry.javascript.integration.replay",
982+
"version": "7.24.2"
983+
},
984+
"sdkProcessingMetadata": {},
985+
"tags": {
986+
"sessionSampleRate": 1,
987+
"errorSampleRate": 0,
988+
"replayType": "error"
989+
}
990+
}
991+
*/
992+
993+
const envelope = createReplayEnvelope(replayId, replayEvent, payloadWithSequence);
992994

993-
const client = getCurrentHub().getClient();
994995
try {
995-
return client?.getTransport()?.send(envelope);
996+
return transport.send(envelope);
996997
} catch {
997998
throw new Error(UNABLE_TO_SEND_REPLAY);
998999
}

‎packages/replay/src/util/createReplayEnvelope.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { Envelope, Event } from '@sentry/types';
2+
import { createEnvelope } from '@sentry/utils';
3+
4+
import { REPLAY_SDK_INFO } from '../constants';
5+
6+
export function createReplayEnvelope(
7+
replayId: string,
8+
replayEvent: Event,
9+
payloadWithSequence: string | Uint8Array,
10+
): Envelope {
11+
return createEnvelope(
12+
{
13+
event_id: replayId,
14+
sent_at: new Date().toISOString(),
15+
sdk: REPLAY_SDK_INFO,
16+
},
17+
[
18+
// @ts-ignore New types
19+
[{ type: 'replay_event' }, replayEvent],
20+
[
21+
{
22+
// @ts-ignore setting envelope
23+
type: 'replay_recording',
24+
length: payloadWithSequence.length,
25+
},
26+
// @ts-ignore: Type 'string' is not assignable to type 'ClientReport'.ts(2322)
27+
payloadWithSequence,
28+
],
29+
],
30+
);
31+
}

‎packages/replay/src/util/getReplayEvent.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { Scope } from '@sentry/core';
2+
import { Client, Event } from '@sentry/types';
3+
4+
import { REPLAY_SDK_INFO } from '../constants';
5+
6+
export async function getReplayEvent({
7+
client,
8+
scope,
9+
replayId: event_id,
10+
event,
11+
}: {
12+
client: Client;
13+
scope: Scope;
14+
replayId: string;
15+
event: Event;
16+
}): Promise<Event> {
17+
// XXX: This event does not trigger `beforeSend` in SDK
18+
// @ts-ignore private api
19+
const preparedEvent: Event = await client._prepareEvent(event, { event_id }, scope);
20+
21+
const session = scope && scope.getSession();
22+
if (session) {
23+
// @ts-ignore private api
24+
client._updateSessionFromEvent(session, preparedEvent);
25+
}
26+
27+
preparedEvent.sdk = {
28+
...preparedEvent.sdk,
29+
...REPLAY_SDK_INFO,
30+
};
31+
32+
return preparedEvent;
33+
}

‎packages/replay/test/unit/util/getReplayEvent.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { BrowserClient } from '@sentry/browser';
2+
import { getCurrentHub, Hub, Scope } from '@sentry/core';
3+
import { Client, Event } from '@sentry/types';
4+
5+
import { REPLAY_EVENT_NAME } from '../../../src/constants';
6+
import { getReplayEvent } from '../../../src/util/getReplayEvent';
7+
import { getDefaultBrowserClientOptions } from '../../utils/getDefaultBrowserClientOptions';
8+
9+
describe('getReplayEvent', () => {
10+
let hub: Hub;
11+
let client: Client;
12+
let scope: Scope;
13+
14+
beforeEach(() => {
15+
hub = getCurrentHub();
16+
client = new BrowserClient(getDefaultBrowserClientOptions());
17+
hub.bindClient(client);
18+
19+
client = hub.getClient()!;
20+
scope = hub.getScope()!;
21+
});
22+
23+
it('works', async () => {
24+
expect(client).toBeDefined();
25+
expect(scope).toBeDefined();
26+
27+
const replayId = 'replay-ID';
28+
const event: Event = {
29+
// @ts-ignore private api
30+
type: REPLAY_EVENT_NAME,
31+
timestamp: 1670837008.634,
32+
error_ids: ['error-ID'],
33+
trace_ids: ['trace-ID'],
34+
urls: ['https://sentry.io/'],
35+
replay_id: replayId,
36+
segment_id: 3,
37+
};
38+
39+
const replayEvent = await getReplayEvent({ scope, client, replayId, event });
40+
41+
expect(replayEvent).toEqual({
42+
type: 'replay_event',
43+
timestamp: 1670837008.634,
44+
error_ids: ['error-ID'],
45+
trace_ids: ['trace-ID'],
46+
urls: ['https://sentry.io/'],
47+
replay_id: 'replay-ID',
48+
segment_id: 3,
49+
platform: 'javascript',
50+
event_id: 'replay-ID',
51+
environment: 'production',
52+
sdk: {
53+
name: 'sentry.javascript.integration.replay',
54+
version: 'version:Test',
55+
},
56+
sdkProcessingMetadata: {},
57+
breadcrumbs: undefined,
58+
});
59+
});
60+
});

‎packages/replay/test/utils/getDefaultBrowserClientOptions.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { createTransport } from '@sentry/core';
2+
import { ClientOptions } from '@sentry/types';
3+
import { resolvedSyncPromise } from '@sentry/utils';
4+
5+
export function getDefaultBrowserClientOptions(options: Partial<ClientOptions> = {}): ClientOptions {
6+
return {
7+
integrations: [],
8+
transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})),
9+
stackParser: () => [],
10+
...options,
11+
};
12+
}

0 commit comments

Comments
 (0)