Skip to content

Commit f252474

Browse files
chargomeclaude
chargome
and
claude
authored
feat(react-router): Use sentryOnError (#1255)
* use new error handler * pr feedback * no double logging * fix: prettier formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * . * add version guard * emit warning if hydrated router is not found --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 367c797 commit f252474

24 files changed

Lines changed: 297 additions & 987 deletions

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
### Features
6+
7+
- feat(react-router): Use `sentryOnError` on `HydratedRouter` instead of mutating `root.tsx` ErrorBoundary
8+
39
## 6.12.0
410

511
### Features

‎e2e-tests/tests/react-router.test.ts‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,9 @@ describe('React Router', () => {
161161
]);
162162
});
163163

164-
test('root file contains Sentry ErrorBoundary', () => {
165-
checkFileContents(`${projectDir}/app/root.tsx`, [
166-
'import * as Sentry from',
167-
'@sentry/react-router',
168-
'export function ErrorBoundary',
169-
'Sentry.captureException(error)',
164+
test('entry.client file contains onError prop on HydratedRouter', () => {
165+
checkFileContents(`${projectDir}/app/entry.client.tsx`, [
166+
'onError={Sentry.sentryOnError}',
170167
]);
171168
});
172169

‎src/react-router/codemods/client.entry.ts‎

Lines changed: 118 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
33

44
import * as recast from 'recast';
5-
import * as path from 'path';
65
import type { namedTypes as t } from 'ast-types';
6+
import type { ExpressionKind } from 'ast-types/lib/gen/kinds';
77

88
// @ts-expect-error - clack is ESM and TS complains about that. It works though
99
import clack from '@clack/prompts';
@@ -21,30 +21,28 @@ export async function instrumentClientEntry(
2121
enableReplay: boolean,
2222
enableLogs: boolean,
2323
useInstrumentationAPI = false,
24+
useOnError = false,
2425
): Promise<void> {
2526
const clientEntryAst = await loadFile(clientEntryPath);
2627

27-
if (hasSentryContent(clientEntryAst.$ast as t.Program)) {
28-
const filename = path.basename(clientEntryPath);
29-
clack.log.info(`Sentry initialization found in ${chalk.cyan(filename)}`);
30-
return;
31-
}
28+
const alreadyHasSentry = hasSentryContent(clientEntryAst.$ast as t.Program);
3229

33-
clientEntryAst.imports.$add({
34-
from: '@sentry/react-router',
35-
imported: '*',
36-
local: 'Sentry',
37-
});
30+
if (!alreadyHasSentry) {
31+
clientEntryAst.imports.$add({
32+
from: '@sentry/react-router',
33+
imported: '*',
34+
local: 'Sentry',
35+
});
3836

39-
let initContent: string;
37+
let initContent: string;
4038

41-
if (useInstrumentationAPI && enableTracing) {
42-
const integrations = ['tracing'];
43-
if (enableReplay) {
44-
integrations.push('Sentry.replayIntegration()');
45-
}
39+
if (useInstrumentationAPI && enableTracing) {
40+
const integrations = ['tracing'];
41+
if (enableReplay) {
42+
integrations.push('Sentry.replayIntegration()');
43+
}
4644

47-
initContent = `
45+
initContent = `
4846
const tracing = Sentry.reactRouterTracingIntegration({ useInstrumentationAPI: true });
4947
5048
Sentry.init({
@@ -59,63 +57,95 @@ Sentry.init({
5957
: ''
6058
}
6159
});`;
62-
} else {
63-
const integrations = [];
64-
if (enableTracing) {
65-
integrations.push('Sentry.reactRouterTracingIntegration()');
66-
}
67-
if (enableReplay) {
68-
integrations.push('Sentry.replayIntegration()');
69-
}
60+
} else {
61+
const integrations = [];
62+
if (enableTracing) {
63+
integrations.push('Sentry.reactRouterTracingIntegration()');
64+
}
65+
if (enableReplay) {
66+
integrations.push('Sentry.replayIntegration()');
67+
}
7068

71-
initContent = `
69+
initContent = `
7270
Sentry.init({
7371
dsn: "${dsn}",
7472
sendDefaultPii: true,
7573
integrations: [${integrations.join(', ')}],
7674
${enableLogs ? 'enableLogs: true,' : ''}
7775
tracesSampleRate: ${enableTracing ? '1.0' : '0'},${
78-
enableTracing
79-
? '\n tracePropagationTargets: [/^\\//, /^https:\\/\\/yourserver\\.io\\/api/],'
80-
: ''
81-
}${
82-
enableReplay
83-
? '\n replaysSessionSampleRate: 0.1,\n replaysOnErrorSampleRate: 1.0,'
84-
: ''
85-
}
76+
enableTracing
77+
? '\n tracePropagationTargets: [/^\\//, /^https:\\/\\/yourserver\\.io\\/api/],'
78+
: ''
79+
}${
80+
enableReplay
81+
? '\n replaysSessionSampleRate: 0.1,\n replaysOnErrorSampleRate: 1.0,'
82+
: ''
83+
}
8684
});`;
85+
}
86+
87+
(clientEntryAst.$ast as t.Program).body.splice(
88+
getAfterImportsInsertionIndex(clientEntryAst.$ast as t.Program),
89+
0,
90+
...recast.parse(initContent).program.body,
91+
);
8792
}
8893

89-
(clientEntryAst.$ast as t.Program).body.splice(
90-
getAfterImportsInsertionIndex(clientEntryAst.$ast as t.Program),
91-
0,
92-
...recast.parse(initContent).program.body,
93-
);
94+
const useInstrAPI = useInstrumentationAPI && enableTracing;
95+
const addInstrProp = useInstrAPI && !alreadyHasSentry;
9496

95-
if (useInstrumentationAPI && enableTracing) {
96-
const hydratedRouterFound = addInstrumentationPropsToHydratedRouter(
97-
clientEntryAst.$ast as t.Program,
98-
);
97+
if (addInstrProp) {
98+
addInstrumentationPropsToHydratedRouter(clientEntryAst.$ast as t.Program);
99+
}
100+
101+
if (useOnError) {
102+
addOnErrorToHydratedRouter(clientEntryAst.$ast as t.Program);
103+
}
99104

100-
if (!hydratedRouterFound) {
101-
clack.log.warn(
102-
`Could not find ${chalk.cyan(
103-
'HydratedRouter',
104-
)} component in your client entry file.\n` +
105-
`To use the Instrumentation API, manually add the ${chalk.cyan(
106-
'unstable_instrumentations',
107-
)} prop:\n` +
108-
` ${chalk.green(
109-
'<HydratedRouter unstable_instrumentations={[tracing.clientInstrumentation]} />',
110-
)}`,
111-
);
105+
// Emit a single warning if HydratedRouter wasn't found for any prop we tried to add
106+
if (
107+
(addInstrProp || useOnError) &&
108+
!hasHydratedRouter(clientEntryAst.$ast as t.Program)
109+
) {
110+
const props: string[] = [];
111+
if (useOnError) {
112+
props.push('onError={Sentry.sentryOnError}');
112113
}
114+
if (addInstrProp) {
115+
props.push('unstable_instrumentations={[tracing.clientInstrumentation]}');
116+
}
117+
clack.log.warn(
118+
`Could not find ${chalk.cyan(
119+
'HydratedRouter',
120+
)} component in your client entry file.\n` +
121+
`Manually add the following props:\n` +
122+
` ${chalk.green(`<HydratedRouter ${props.join(' ')} />`)}`,
123+
);
113124
}
114125

115126
await writeFile(clientEntryAst.$ast, clientEntryPath);
116127
}
117128

118-
function addInstrumentationPropsToHydratedRouter(ast: t.Program): boolean {
129+
function hasHydratedRouter(ast: t.Program): boolean {
130+
let found = false;
131+
recast.visit(ast, {
132+
visitJSXElement(path) {
133+
const name = path.node.openingElement.name;
134+
if (name.type === 'JSXIdentifier' && name.name === 'HydratedRouter') {
135+
found = true;
136+
return false;
137+
}
138+
this.traverse(path);
139+
},
140+
});
141+
return found;
142+
}
143+
144+
function addPropToHydratedRouter(
145+
ast: t.Program,
146+
propName: string,
147+
propValue: ExpressionKind,
148+
): boolean {
119149
let found = false;
120150

121151
recast.visit(ast, {
@@ -128,30 +158,23 @@ function addInstrumentationPropsToHydratedRouter(ast: t.Program): boolean {
128158
) {
129159
found = true;
130160

131-
const hasInstrumentationsProp = openingElement.attributes?.some(
161+
const hasProp = openingElement.attributes?.some(
132162
(attr) =>
133163
attr.type === 'JSXAttribute' &&
134164
attr.name.type === 'JSXIdentifier' &&
135-
attr.name.name === 'unstable_instrumentations',
165+
attr.name.name === propName,
136166
);
137167

138-
if (!hasInstrumentationsProp) {
139-
const instrumentationsProp = recast.types.builders.jsxAttribute(
140-
recast.types.builders.jsxIdentifier('unstable_instrumentations'),
141-
recast.types.builders.jsxExpressionContainer(
142-
recast.types.builders.arrayExpression([
143-
recast.types.builders.memberExpression(
144-
recast.types.builders.identifier('tracing'),
145-
recast.types.builders.identifier('clientInstrumentation'),
146-
),
147-
]),
148-
),
168+
if (!hasProp) {
169+
const prop = recast.types.builders.jsxAttribute(
170+
recast.types.builders.jsxIdentifier(propName),
171+
recast.types.builders.jsxExpressionContainer(propValue),
149172
);
150173

151174
if (!openingElement.attributes) {
152175
openingElement.attributes = [];
153176
}
154-
openingElement.attributes.push(instrumentationsProp);
177+
openingElement.attributes.push(prop);
155178
}
156179

157180
return false;
@@ -163,3 +186,27 @@ function addInstrumentationPropsToHydratedRouter(ast: t.Program): boolean {
163186

164187
return found;
165188
}
189+
190+
function addOnErrorToHydratedRouter(ast: t.Program): boolean {
191+
return addPropToHydratedRouter(
192+
ast,
193+
'onError',
194+
recast.types.builders.memberExpression(
195+
recast.types.builders.identifier('Sentry'),
196+
recast.types.builders.identifier('sentryOnError'),
197+
),
198+
);
199+
}
200+
201+
function addInstrumentationPropsToHydratedRouter(ast: t.Program): boolean {
202+
return addPropToHydratedRouter(
203+
ast,
204+
'unstable_instrumentations',
205+
recast.types.builders.arrayExpression([
206+
recast.types.builders.memberExpression(
207+
recast.types.builders.identifier('tracing'),
208+
recast.types.builders.identifier('clientInstrumentation'),
209+
),
210+
]),
211+
);
212+
}

0 commit comments

Comments
 (0)