Skip to content

Commit 992af87

Browse files
mydeamydea
committed
ref(replay): Use beforeAddBreadcrumb hook instead of scope listener
1 parent a7097d9 commit 992af87

File tree

7 files changed

+179
-228
lines changed

7 files changed

+179
-228
lines changed

‎packages/react/src/redux.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
2-
import { getClient, getCurrentScope, getGlobalScope } from '@sentry/core';
2+
import { addBreadcrumb, getClient, getCurrentScope, getGlobalScope } from '@sentry/core';
33
import type { Scope } from '@sentry/types';
44
import { addNonEnumerableProperty } from '@sentry/utils';
55

@@ -121,7 +121,7 @@ function createReduxEnhancer(enhancerOptions?: Partial<SentryEnhancerOptions>):
121121
/* Action breadcrumbs */
122122
const transformedAction = options.actionTransformer(action);
123123
if (typeof transformedAction !== 'undefined' && transformedAction !== null) {
124-
scope.addBreadcrumb({
124+
addBreadcrumb({
125125
category: ACTION_BREADCRUMB_CATEGORY,
126126
data: transformedAction,
127127
type: ACTION_BREADCRUMB_TYPE,

‎packages/replay/src/coreHandlers/handleScope.ts renamed to ‎packages/replay/src/coreHandlers/handleBreadcrumbs.ts

Lines changed: 39 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { Breadcrumb, Scope } from '@sentry/types';
1+
import { getClient } from '@sentry/core';
2+
import type { Breadcrumb } from '@sentry/types';
23
import { normalize } from '@sentry/utils';
34

45
import { CONSOLE_ARG_MAX_SIZE } from '../constants';
@@ -7,61 +8,55 @@ import type { ReplayFrame } from '../types/replayFrame';
78
import { createBreadcrumb } from '../util/createBreadcrumb';
89
import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
910

10-
let _LAST_BREADCRUMB: null | Breadcrumb = null;
11-
1211
type BreadcrumbWithCategory = Required<Pick<Breadcrumb, 'category'>>;
1312

14-
function isBreadcrumbWithCategory(breadcrumb: Breadcrumb): breadcrumb is BreadcrumbWithCategory {
15-
return !!breadcrumb.category;
16-
}
13+
/**
14+
* Handle breadcrumbs that Sentry captures, and make sure to capture relevant breadcrumbs to Replay as well.
15+
*/
16+
export function handleBreadcrumbs(replay: ReplayContainer): void {
17+
const client = getClient();
1718

18-
export const handleScopeListener: (replay: ReplayContainer) => (scope: Scope) => void =
19-
(replay: ReplayContainer) =>
20-
(scope: Scope): void => {
21-
if (!replay.isEnabled()) {
22-
return;
23-
}
19+
if (!client || !client.on) {
20+
return;
21+
}
2422

25-
const result = handleScope(scope);
23+
client.on('beforeAddBreadcrumb', breadcrumb => beforeAddBreadcrumb(replay, breadcrumb));
24+
}
2625

27-
if (!result) {
28-
return;
29-
}
26+
function beforeAddBreadcrumb(replay: ReplayContainer, breadcrumb: Breadcrumb): void {
27+
if (!replay.isEnabled() || !isBreadcrumbWithCategory(breadcrumb)) {
28+
return;
29+
}
3030

31+
const result = normalizeBreadcrumb(breadcrumb);
32+
if (result) {
3133
addBreadcrumbEvent(replay, result);
32-
};
33-
34-
/**
35-
* An event handler to handle scope changes.
36-
*/
37-
export function handleScope(scope: Scope): Breadcrumb | null {
38-
// TODO (v8): Remove this guard. This was put in place because we introduced
39-
// Scope.getLastBreadcrumb mid-v7 which caused incompatibilities with older SDKs.
40-
// For now, we'll just return null if the method doesn't exist but we should eventually
41-
// get rid of this guard.
42-
const newBreadcrumb = scope.getLastBreadcrumb && scope.getLastBreadcrumb();
43-
44-
// Listener can be called when breadcrumbs have not changed, so we store the
45-
// reference to the last crumb and only return a crumb if it has changed
46-
if (_LAST_BREADCRUMB === newBreadcrumb || !newBreadcrumb) {
47-
return null;
4834
}
35+
}
4936

50-
_LAST_BREADCRUMB = newBreadcrumb;
51-
37+
/** Exported only for tests. */
38+
export function normalizeBreadcrumb(breadcrumb: Breadcrumb): Breadcrumb | null {
5239
if (
53-
!isBreadcrumbWithCategory(newBreadcrumb) ||
54-
['fetch', 'xhr', 'sentry.event', 'sentry.transaction'].includes(newBreadcrumb.category) ||
55-
newBreadcrumb.category.startsWith('ui.')
40+
!isBreadcrumbWithCategory(breadcrumb) ||
41+
[
42+
// fetch & xhr are handled separately,in handleNetworkBreadcrumbs
43+
'fetch',
44+
'xhr',
45+
// These two are breadcrumbs for emitted sentry events, we don't care about them
46+
'sentry.event',
47+
'sentry.transaction',
48+
].includes(breadcrumb.category) ||
49+
// We capture UI breadcrumbs separately
50+
breadcrumb.category.startsWith('ui.')
5651
) {
5752
return null;
5853
}
5954

60-
if (newBreadcrumb.category === 'console') {
61-
return normalizeConsoleBreadcrumb(newBreadcrumb);
55+
if (breadcrumb.category === 'console') {
56+
return normalizeConsoleBreadcrumb(breadcrumb);
6257
}
6358

64-
return createBreadcrumb(newBreadcrumb);
59+
return createBreadcrumb(breadcrumb);
6560
}
6661

6762
/** exported for tests only */
@@ -116,3 +111,7 @@ export function normalizeConsoleBreadcrumb(
116111
},
117112
});
118113
}
114+
115+
function isBreadcrumbWithCategory(breadcrumb: Breadcrumb): breadcrumb is BreadcrumbWithCategory {
116+
return !!breadcrumb.category;
117+
}

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,27 @@
11
import type { BaseClient } from '@sentry/core';
2-
import { getCurrentScope } from '@sentry/core';
32
import { addEventProcessor, getClient } from '@sentry/core';
43
import type { Client, DynamicSamplingContext } from '@sentry/types';
54
import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler } from '@sentry/utils';
65

76
import { handleAfterSendEvent } from '../coreHandlers/handleAfterSendEvent';
87
import { handleBeforeSendEvent } from '../coreHandlers/handleBeforeSendEvent';
8+
import { handleBreadcrumbs } from '../coreHandlers/handleBreadcrumbs';
99
import { handleDomListener } from '../coreHandlers/handleDom';
1010
import { handleGlobalEventListener } from '../coreHandlers/handleGlobalEvent';
1111
import { handleHistorySpanListener } from '../coreHandlers/handleHistory';
1212
import { handleNetworkBreadcrumbs } from '../coreHandlers/handleNetworkBreadcrumbs';
13-
import { handleScopeListener } from '../coreHandlers/handleScope';
1413
import type { ReplayContainer } from '../types';
1514

1615
/**
1716
* Add global listeners that cannot be removed.
1817
*/
1918
export function addGlobalListeners(replay: ReplayContainer): void {
2019
// Listeners from core SDK //
21-
const scope = getCurrentScope();
2220
const client = getClient();
2321

24-
scope.addScopeListener(handleScopeListener(replay));
2522
addClickKeypressInstrumentationHandler(handleDomListener(replay));
2623
addHistoryInstrumentationHandler(handleHistorySpanListener(replay));
24+
handleBreadcrumbs(replay);
2725
handleNetworkBreadcrumbs(replay);
2826

2927
// Tag all (non replay) events that get sent to Sentry with the current

‎packages/replay/test/integration/coreHandlers/handleScope.test.ts

Lines changed: 0 additions & 38 deletions
This file was deleted.

‎packages/replay/test/unit/coreHandlers/handleBreadcrumbs.test.ts

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { CONSOLE_ARG_MAX_SIZE } from '../../../src/constants';
2+
import { normalizeBreadcrumb, normalizeConsoleBreadcrumb } from '../../../src/coreHandlers/handleBreadcrumbs';
3+
4+
describe('Unit | coreHandlers | handleBreadcrumbs', () => {
5+
describe('normalizeBreadcrumb', () => {
6+
it.each([undefined, 'ui.click', 'ui.scroll', 'fetch', 'xhr', 'sentry.event', 'sentry.transaction'])(
7+
'returns null if breadcrumb has category=%p',
8+
category => {
9+
const actual = normalizeBreadcrumb({ category });
10+
expect(actual).toBeNull();
11+
},
12+
);
13+
14+
it('returns breadcrumb when category is valid', () => {
15+
const breadcrumb = { category: 'other' };
16+
const actual = normalizeBreadcrumb(breadcrumb);
17+
expect(actual).toEqual({
18+
timestamp: expect.any(Number),
19+
category: 'other',
20+
type: 'default',
21+
});
22+
});
23+
24+
it('timestamp takes precedence', () => {
25+
const breadcrumb = { category: 'other', timestamp: 123456 };
26+
const actual = normalizeBreadcrumb(breadcrumb);
27+
expect(actual).toEqual({
28+
timestamp: 123456,
29+
category: 'other',
30+
type: 'default',
31+
});
32+
});
33+
34+
it('handles console breadcrumb', () => {
35+
const breadcrumb = {
36+
category: 'console',
37+
message: 'test',
38+
data: {
39+
arguments: ['a'.repeat(CONSOLE_ARG_MAX_SIZE + 10), 'b'.repeat(CONSOLE_ARG_MAX_SIZE + 10)],
40+
},
41+
};
42+
const actual = normalizeBreadcrumb(breadcrumb);
43+
expect(actual).toEqual({
44+
timestamp: expect.any(Number),
45+
category: 'console',
46+
message: 'test',
47+
type: 'default',
48+
data: {
49+
arguments: [`${'a'.repeat(CONSOLE_ARG_MAX_SIZE)}…`, `${'b'.repeat(CONSOLE_ARG_MAX_SIZE)}…`],
50+
_meta: { warnings: ['CONSOLE_ARG_TRUNCATED'] },
51+
},
52+
});
53+
});
54+
});
55+
56+
describe('normalizeConsoleBreadcrumb', () => {
57+
it('handles console messages with no arguments', () => {
58+
const breadcrumb = { category: 'console', message: 'test' };
59+
const actual = normalizeConsoleBreadcrumb(breadcrumb);
60+
61+
expect(actual).toMatchObject({ category: 'console', message: 'test' });
62+
});
63+
64+
it('handles console messages with empty arguments', () => {
65+
const breadcrumb = { category: 'console', message: 'test', data: { arguments: [] } };
66+
const actual = normalizeConsoleBreadcrumb(breadcrumb);
67+
68+
expect(actual).toMatchObject({ category: 'console', message: 'test', data: { arguments: [] } });
69+
});
70+
71+
it('handles console messages with simple arguments', () => {
72+
const breadcrumb = {
73+
category: 'console',
74+
message: 'test',
75+
data: { arguments: [1, 'a', true, null, undefined] },
76+
};
77+
const actual = normalizeConsoleBreadcrumb(breadcrumb);
78+
79+
expect(actual).toMatchObject({
80+
category: 'console',
81+
message: 'test',
82+
data: {
83+
arguments: [1, 'a', true, null, undefined],
84+
},
85+
});
86+
});
87+
88+
it('truncates large strings', () => {
89+
const breadcrumb = {
90+
category: 'console',
91+
message: 'test',
92+
data: {
93+
arguments: ['a'.repeat(CONSOLE_ARG_MAX_SIZE + 10), 'b'.repeat(CONSOLE_ARG_MAX_SIZE + 10)],
94+
},
95+
};
96+
const actual = normalizeConsoleBreadcrumb(breadcrumb);
97+
98+
expect(actual).toMatchObject({
99+
category: 'console',
100+
message: 'test',
101+
data: {
102+
arguments: [`${'a'.repeat(CONSOLE_ARG_MAX_SIZE)}…`, `${'b'.repeat(CONSOLE_ARG_MAX_SIZE)}…`],
103+
_meta: { warnings: ['CONSOLE_ARG_TRUNCATED'] },
104+
},
105+
});
106+
});
107+
108+
it('truncates large JSON objects', () => {
109+
const bb = { bb: 'b'.repeat(CONSOLE_ARG_MAX_SIZE + 10) };
110+
const c = { c: 'c'.repeat(CONSOLE_ARG_MAX_SIZE + 10) };
111+
112+
const breadcrumb = {
113+
category: 'console',
114+
message: 'test',
115+
data: {
116+
arguments: [{ aa: 'yes' }, bb, c],
117+
},
118+
};
119+
const actual = normalizeConsoleBreadcrumb(breadcrumb);
120+
121+
expect(actual).toMatchObject({
122+
category: 'console',
123+
message: 'test',
124+
data: {
125+
arguments: [
126+
{ aa: 'yes' },
127+
`${JSON.stringify(bb, null, 2).slice(0, CONSOLE_ARG_MAX_SIZE)}…`,
128+
`${JSON.stringify(c, null, 2).slice(0, CONSOLE_ARG_MAX_SIZE)}…`,
129+
],
130+
_meta: { warnings: ['CONSOLE_ARG_TRUNCATED'] },
131+
},
132+
});
133+
});
134+
});
135+
});

0 commit comments

Comments
 (0)