Skip to content

Commit 9969dbe

Browse files
onurtemizkanonurtemizkan
committed
Add profiling and finalize
1 parent 3e100cc commit 9969dbe

File tree

16 files changed

+1664
-355
lines changed

16 files changed

+1664
-355
lines changed

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

Lines changed: 293 additions & 55 deletions
Large diffs are not rendered by default.

‎e2e-tests/utils/index.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,16 @@ export function cleanupGit(projectDir: string): void {
260260
*/
261261
export function revertLocalChanges(projectDir: string): void {
262262
try {
263-
// Revert tracked files
264-
execSync('git restore .', { cwd: projectDir });
265-
// Revert untracked files
266-
execSync('git clean -fd .', { cwd: projectDir });
263+
// Check if this is a git repository first
264+
const isGitRepo = fs.existsSync(path.join(projectDir, '.git'));
265+
266+
if (isGitRepo) {
267+
// Revert tracked files
268+
execSync('git restore .', { cwd: projectDir });
269+
// Revert untracked files
270+
execSync('git clean -fd .', { cwd: projectDir });
271+
}
272+
267273
// Remove node_modules and dist (.gitignore'd and therefore not removed via git clean)
268274
execSync('rm -rf node_modules', { cwd: projectDir });
269275
execSync('rm -rf dist', { cwd: projectDir });

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import clack from '@clack/prompts';
1010
// @ts-expect-error - magicast is ESM and TS complains about that. It works though
1111
import { loadFile, writeFile } from 'magicast';
1212
import { hasSentryContent } from '../../utils/ast-utils';
13-
import { getSentryInitClientContent } from '../templates';
1413
import { getAfterImportsInsertionIndex } from './utils';
1514

1615
export async function instrumentClientEntry(
@@ -33,12 +32,30 @@ export async function instrumentClientEntry(
3332
local: 'Sentry',
3433
});
3534

36-
const initContent = getSentryInitClientContent(
37-
dsn,
38-
enableTracing,
39-
enableReplay,
40-
enableLogs,
41-
);
35+
const integrations = [];
36+
if (enableTracing) {
37+
integrations.push('Sentry.reactRouterTracingIntegration()');
38+
}
39+
if (enableReplay) {
40+
integrations.push('Sentry.replayIntegration()');
41+
}
42+
43+
const initContent = `
44+
Sentry.init({
45+
dsn: "${dsn}",
46+
sendDefaultPii: true,
47+
integrations: [${integrations.join(', ')}],
48+
${enableLogs ? 'enableLogs: true,' : ''}
49+
tracesSampleRate: ${enableTracing ? '1.0' : '0'},${
50+
enableTracing
51+
? '\n tracePropagationTargets: [/^\\//, /^https:\\/\\/yourserver\\.io\\/api/],'
52+
: ''
53+
}${
54+
enableReplay
55+
? '\n replaysSessionSampleRate: 0.1,\n replaysOnErrorSampleRate: 1.0,'
56+
: ''
57+
}
58+
});`;
4259

4360
(clientEntryAst.$ast as t.Program).body.splice(
4461
getAfterImportsInsertionIndex(clientEntryAst.$ast as t.Program),

‎src/react-router/codemods/root.ts‎

Lines changed: 64 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,16 @@ import {
1515
} from 'magicast';
1616

1717
import { ERROR_BOUNDARY_TEMPLATE } from '../templates';
18-
import { hasSentryContent } from '../../utils/ast-utils';
18+
import {
19+
hasSentryContent,
20+
safeGetFunctionBody,
21+
safeInsertBeforeReturn,
22+
} from '../../utils/ast-utils';
23+
import { debug } from '../../utils/debug';
1924

2025
export async function instrumentRoot(rootFileName: string): Promise<void> {
21-
const rootRouteAst = await loadFile(
22-
path.join(process.cwd(), 'app', rootFileName),
23-
);
26+
const filePath = path.join(process.cwd(), 'app', rootFileName);
27+
const rootRouteAst = await loadFile(filePath);
2428

2529
const exportsAst = rootRouteAst.exports.$ast as t.Program;
2630

@@ -97,21 +101,45 @@ export async function instrumentRoot(rootFileName: string): Promise<void> {
97101

98102
recast.visit(rootRouteAst.$ast, {
99103
visitExportNamedDeclaration(path) {
100-
// Find ErrorBoundary export
104+
// Find ErrorBoundary export with proper null checks
101105
if (
102-
path.value.declaration?.declarations?.[0].id?.name === 'ErrorBoundary'
106+
path.value.declaration?.type === 'VariableDeclaration' &&
107+
path.value.declaration?.declarations &&
108+
path.value.declaration.declarations.length > 0 &&
109+
path.value.declaration.declarations[0].id?.name === 'ErrorBoundary'
103110
) {
104111
hasBlockStatementBody = true;
105112
}
106113

107-
if (path.value.declaration?.id?.name === 'ErrorBoundary') {
114+
if (
115+
path.value.declaration?.type === 'FunctionDeclaration' &&
116+
path.value.declaration?.id?.name === 'ErrorBoundary'
117+
) {
108118
hasFunctionDeclarationBody = true;
109119
}
110120

111121
if (hasBlockStatementBody || hasFunctionDeclarationBody) {
112-
const errorBoundaryExport = hasBlockStatementBody
113-
? path.value.declaration?.declarations?.[0].init
114-
: path.value.declaration;
122+
let errorBoundaryExport = null;
123+
124+
if (
125+
hasBlockStatementBody &&
126+
path.value.declaration?.type === 'VariableDeclaration' &&
127+
path.value.declaration?.declarations &&
128+
path.value.declaration.declarations.length > 0
129+
) {
130+
errorBoundaryExport = path.value.declaration.declarations[0].init;
131+
} else if (
132+
hasFunctionDeclarationBody &&
133+
path.value.declaration?.type === 'FunctionDeclaration'
134+
) {
135+
errorBoundaryExport = path.value.declaration;
136+
}
137+
138+
// Skip if we couldn't safely extract the ErrorBoundary export
139+
if (!errorBoundaryExport) {
140+
this.traverse(path);
141+
return;
142+
}
115143

116144
let alreadyHasCaptureException = false;
117145

@@ -121,7 +149,9 @@ export async function instrumentRoot(rootFileName: string): Promise<void> {
121149
const callee = callPath.value.callee;
122150
if (
123151
(callee.type === 'MemberExpression' &&
152+
callee.object &&
124153
callee.object.name === 'Sentry' &&
154+
callee.property &&
125155
callee.property.name === 'captureException') ||
126156
(callee.type === 'Identifier' &&
127157
callee.name === 'captureException')
@@ -147,11 +177,18 @@ export async function instrumentRoot(rootFileName: string): Promise<void> {
147177

148178
if (isFunctionDeclaration) {
149179
// If it's a function declaration, we can insert the call directly
150-
errorBoundaryExport.body.body.splice(
151-
errorBoundaryExport.body.body.length - 1,
152-
0,
153-
captureExceptionCall,
154-
);
180+
const functionBody = safeGetFunctionBody(errorBoundaryExport);
181+
if (functionBody) {
182+
if (
183+
!safeInsertBeforeReturn(functionBody, captureExceptionCall)
184+
) {
185+
// Fallback: append to the end if insertion fails
186+
functionBody.push(captureExceptionCall);
187+
}
188+
} else {
189+
// Log warning if we can't safely access function body
190+
debug('Could not safely access ErrorBoundary function body');
191+
}
155192
} else if (isVariableDeclaration) {
156193
// If it's a variable declaration, we need to find the right place to insert the call
157194
const init = errorBoundaryExport.init;
@@ -160,11 +197,17 @@ export async function instrumentRoot(rootFileName: string): Promise<void> {
160197
(init.type === 'ArrowFunctionExpression' ||
161198
init.type === 'FunctionExpression')
162199
) {
163-
init.body.body.splice(
164-
init.body.body.length - 1,
165-
0,
166-
captureExceptionCall,
167-
);
200+
const initBody = safeGetFunctionBody(init);
201+
if (initBody) {
202+
if (!safeInsertBeforeReturn(initBody, captureExceptionCall)) {
203+
// Fallback: append to the end if insertion fails
204+
initBody.push(captureExceptionCall);
205+
}
206+
} else {
207+
debug(
208+
'Could not safely access ErrorBoundary function expression body',
209+
);
210+
}
168211
}
169212
}
170213
}
@@ -175,8 +218,5 @@ export async function instrumentRoot(rootFileName: string): Promise<void> {
175218
});
176219
}
177220

178-
await writeFile(
179-
rootRouteAst.$ast,
180-
path.join(process.cwd(), 'app', rootFileName),
181-
);
221+
await writeFile(rootRouteAst.$ast, filePath);
182222
}

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

Lines changed: 80 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,12 @@ import chalk from 'chalk';
1717

1818
// @ts-expect-error - magicast is ESM and TS complains about that. It works though
1919
import { generateCode, loadFile, writeFile } from 'magicast';
20-
import { debug } from 'console';
21-
import { hasSentryContent } from '../../utils/ast-utils';
20+
import { debug } from '../../utils/debug';
21+
import {
22+
hasSentryContent,
23+
safeCalleeIdentifierMatch,
24+
safeGetIdentifierName,
25+
} from '../../utils/ast-utils';
2226
import { getAfterImportsInsertionIndex } from './utils';
2327

2428
export async function instrumentServerEntry(
@@ -41,7 +45,6 @@ export async function instrumentServerEntry(
4145
}
4246

4347
export function instrumentHandleRequest(
44-
// MagicAst returns `ProxifiedModule<any>` so therefore we have to use `any` here
4548
originalEntryServerMod: ProxifiedModule<any>,
4649
): void {
4750
const originalEntryServerModAST = originalEntryServerMod.$ast as t.Program;
@@ -59,7 +62,6 @@ export function instrumentHandleRequest(
5962
)} in your server entry file. Creating one for you.`,
6063
);
6164

62-
// Add imports required by handleRequest [ServerRouter, renderToPipeableStream, createReadableStreamFromReadable] if not present
6365
let foundServerRouterImport = false;
6466
let foundRenderToPipeableStreamImport = false;
6567
let foundCreateReadableStreamFromReadableImport = false;
@@ -141,7 +143,6 @@ export function instrumentHandleRequest(
141143
} else {
142144
let defaultExportNode: recast.types.namedTypes.ExportDefaultDeclaration | null =
143145
null;
144-
// Replace the existing default export with the wrapped one
145146
const defaultExportIndex = originalEntryServerModAST.body.findIndex(
146147
(node) => {
147148
const found = node.type === 'ExportDefaultDeclaration';
@@ -155,16 +156,14 @@ export function instrumentHandleRequest(
155156
);
156157

157158
if (defaultExportIndex !== -1 && defaultExportNode !== null) {
158-
// Try to find `pipe(body)` so we can wrap the body with `Sentry.getMetaTagTransformer`
159159
recast.visit(defaultExportNode, {
160160
visitCallExpression(path) {
161161
if (
162-
path.value.callee.name === 'pipe' &&
162+
safeCalleeIdentifierMatch(path.value.callee, 'pipe') &&
163163
path.value.arguments.length &&
164164
path.value.arguments[0].type === 'Identifier' &&
165-
path.value.arguments[0].name === 'body'
165+
safeGetIdentifierName(path.value.arguments[0]) === 'body'
166166
) {
167-
// // Wrap the call expression with `Sentry.getMetaTagTransformer`
168167
const wrapped = recast.types.builders.callExpression(
169168
recast.types.builders.memberExpression(
170169
recast.types.builders.identifier('Sentry'),
@@ -220,13 +219,27 @@ export function instrumentHandleError(
220219
);
221220

222221
const handleErrorFunctionVariableDeclarationExport =
223-
originalEntryServerModAST.body.find(
224-
(node) =>
225-
node.type === 'ExportNamedDeclaration' &&
226-
node.declaration?.type === 'VariableDeclaration' &&
227-
// @ts-expect-error - id should always have a name in this case
228-
node.declaration.declarations[0].id.name === 'handleError',
229-
);
222+
originalEntryServerModAST.body.find((node) => {
223+
if (
224+
node.type !== 'ExportNamedDeclaration' ||
225+
node.declaration?.type !== 'VariableDeclaration'
226+
) {
227+
return false;
228+
}
229+
230+
const declarations = node.declaration.declarations;
231+
if (!declarations || declarations.length === 0) {
232+
return false;
233+
}
234+
235+
const firstDeclaration = declarations[0];
236+
if (!firstDeclaration || firstDeclaration.type !== 'VariableDeclarator') {
237+
return false;
238+
}
239+
240+
const id = firstDeclaration.id;
241+
return id && id.type === 'Identifier' && id.name === 'handleError';
242+
});
230243

231244
if (
232245
!handleErrorFunctionExport &&
@@ -277,25 +290,61 @@ export function instrumentHandleError(
277290
) {
278291
debug('createSentryHandleError is already used, skipping adding it again');
279292
} else if (handleErrorFunctionExport) {
280-
const implementation = recast.parse(`if (!request.signal.aborted) {
293+
// Create the Sentry captureException call as an IfStatement
294+
const sentryCall = recast.parse(`if (!request.signal.aborted) {
281295
Sentry.captureException(error);
282296
}`).program.body[0];
283-
// If the current handleError function has a body, we need to merge the new implementation with the existing one
284-
implementation.declarations[0].init.arguments[0].body.body.unshift(
285-
// @ts-expect-error - declaration works here because the AST is proxified by magicast
286-
...handleErrorFunctionExport.declaration.body.body,
287-
);
288297

298+
// Safely insert the Sentry call at the beginning of the handleError function body
289299
// @ts-expect-error - declaration works here because the AST is proxified by magicast
290-
handleErrorFunctionExport.declaration = implementation;
300+
const declaration = handleErrorFunctionExport.declaration;
301+
if (
302+
declaration &&
303+
declaration.body &&
304+
declaration.body.body &&
305+
Array.isArray(declaration.body.body)
306+
) {
307+
declaration.body.body.unshift(sentryCall);
308+
} else {
309+
debug(
310+
'Cannot safely access handleError function body, skipping instrumentation',
311+
);
312+
}
291313
} else if (handleErrorFunctionVariableDeclarationExport) {
292-
const implementation = recast.parse(`if (!request.signal.aborted) {
314+
// Create the Sentry captureException call as an IfStatement
315+
const sentryCall = recast.parse(`if (!request.signal.aborted) {
293316
Sentry.captureException(error);
294317
}`).program.body[0];
295-
const existingHandleErrorImplementation =
296-
// @ts-expect-error - declaration works here because the AST is proxified by magicast
297-
handleErrorFunctionVariableDeclarationExport.declaration.declarations[0]
298-
.init;
318+
319+
// Safe access to existing handle error implementation with proper null checks
320+
// We know this is ExportNamedDeclaration with VariableDeclaration from the earlier find
321+
const exportDeclaration =
322+
handleErrorFunctionVariableDeclarationExport as any;
323+
if (
324+
!exportDeclaration.declaration ||
325+
exportDeclaration.declaration.type !== 'VariableDeclaration' ||
326+
!exportDeclaration.declaration.declarations ||
327+
exportDeclaration.declaration.declarations.length === 0
328+
) {
329+
debug(
330+
'Cannot safely access handleError variable declaration, skipping instrumentation',
331+
);
332+
return;
333+
}
334+
335+
const firstDeclaration = exportDeclaration.declaration.declarations[0];
336+
if (
337+
!firstDeclaration ||
338+
firstDeclaration.type !== 'VariableDeclarator' ||
339+
!firstDeclaration.init
340+
) {
341+
debug(
342+
'Cannot safely access handleError variable declarator init, skipping instrumentation',
343+
);
344+
return;
345+
}
346+
347+
const existingHandleErrorImplementation = firstDeclaration.init;
299348
const existingParams = existingHandleErrorImplementation.params;
300349
const existingBody = existingHandleErrorImplementation.body;
301350

@@ -322,12 +371,13 @@ export function instrumentHandleError(
322371
existingParams[1].type === 'ObjectPattern' &&
323372
!existingParams[1].properties.some(
324373
(prop: t.ObjectProperty) =>
325-
prop.key.type === 'Identifier' && prop.key.name === 'request',
374+
safeGetIdentifierName(prop.key) === 'request',
326375
)
327376
) {
328377
existingParams[1].properties.push(requestParam);
329378
}
330379

331-
existingBody.body.push(implementation);
380+
// Add the Sentry call to the function body
381+
existingBody.body.push(sentryCall);
332382
}
333383
}

0 commit comments

Comments
 (0)