Skip to content

Commit a445083

Browse files
onurtemizkanonurtemizkan
committed
ref(remix): Rework captureRemixServerException 
1 parent 8744898 commit a445083

File tree

12 files changed

+217
-22
lines changed

12 files changed

+217
-22
lines changed

‎packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ Sentry.init({
2424
tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
2525
});
2626

27-
export function handleError(error: unknown, { request }: DataFunctionArgs): void {
28-
Sentry.captureRemixServerException(error, 'remix.server', request);
29-
}
27+
export const handleError = Sentry.wrapRemixHandleError;
3028

3129
export default function handleRequest(
3230
request: Request,

‎packages/e2e-tests/test-applications/create-remix-app/app/entry.server.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ Sentry.init({
2121
tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
2222
});
2323

24-
export function handleError(error: unknown, { request }: DataFunctionArgs): void {
25-
Sentry.captureRemixServerException(error, 'remix.server', request);
26-
}
24+
export const handleError = Sentry.wrapRemixHandleError;
2725

2826
export default function handleRequest(
2927
request: Request,

‎packages/remix/src/index.server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export {
5959
// Keeping the `*` exports for backwards compatibility and types
6060
export * from '@sentry/node';
6161

62-
export { captureRemixServerException } from './utils/instrumentServer';
62+
export { captureRemixServerException, wrapRemixHandleError } from './utils/instrumentServer';
6363
export { ErrorBoundary, withErrorBoundary } from '@sentry/react';
6464
export { remixRouterInstrumentation, withSentry } from './client/performance';
6565
export { captureRemixErrorBoundaryError } from './client/errors';

‎packages/remix/src/utils/instrumentServer.ts

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
isNodeEnv,
1111
loadModule,
1212
logger,
13+
objectify,
1314
tracingContextFromHeaders,
1415
} from '@sentry/utils';
1516

@@ -32,6 +33,7 @@ import type {
3233
EntryContext,
3334
FutureConfig,
3435
HandleDocumentRequestFunction,
36+
HandleErrorFunction,
3537
ReactRouterDomPkg,
3638
RemixRequest,
3739
RequestHandler,
@@ -70,6 +72,44 @@ async function extractResponseError(response: Response): Promise<unknown> {
7072
return responseData;
7173
}
7274

75+
/**
76+
*
77+
*/
78+
export function wrapRemixHandleError(this: unknown, err: unknown, { request }: DataFunctionArgs): void {
79+
// We are skipping thrown responses here as they are either handled:
80+
// - Remix v1: by captureRemixServerException at loader / action
81+
// - Remix v2: by ErrorBoundary
82+
// We don't want to capture them twice.
83+
// https://remix.run/docs/en/main/file-conventions/entry.server#thrown-responses
84+
// https://remix.run/docs/en/v1/api/conventions#throwing-responses-in-loaders
85+
if (isResponse(err) || isRouteErrorResponse(err)) {
86+
return;
87+
}
88+
89+
if (err && typeof err === 'object' && 'error' in err) {
90+
// No need to await here as we're skipping the error responses
91+
void captureRemixServerException(err, 'remix.server.handleError', request);
92+
} else {
93+
const objectifiedErr = objectify(err);
94+
95+
captureException(objectifiedErr, scope => {
96+
scope.addEventProcessor(event => {
97+
addExceptionMechanism(event, {
98+
type: 'instrument',
99+
handled: false,
100+
data: {
101+
function: 'remix.server.handleError',
102+
},
103+
});
104+
105+
return event;
106+
});
107+
108+
return scope;
109+
});
110+
}
111+
}
112+
73113
/**
74114
* Captures an exception happened in the Remix server.
75115
*
@@ -107,7 +147,9 @@ export async function captureRemixServerException(err: unknown, name: string, re
107147
DEBUG_BUILD && logger.warn('Failed to normalize Remix request');
108148
}
109149

110-
captureException(isResponse(err) ? await extractResponseError(err) : err, scope => {
150+
const objectifiedErr = objectify(err);
151+
152+
captureException(isResponse(objectifiedErr) ? await extractResponseError(objectifiedErr) : objectifiedErr, scope => {
111153
const activeTransactionName = getActiveTransaction()?.name;
112154

113155
scope.setSDKProcessingMetadata({
@@ -240,7 +282,10 @@ const makeWrappedLoader =
240282
return makeWrappedDataFunction(origLoader, id, 'loader', remixVersion);
241283
};
242284

243-
function getTraceAndBaggage(): { sentryTrace?: string; sentryBaggage?: string } {
285+
function getTraceAndBaggage(): {
286+
sentryTrace?: string;
287+
sentryBaggage?: string;
288+
} {
244289
const transaction = getActiveTransaction();
245290
const currentScope = getCurrentHub().getScope();
246291

@@ -287,7 +332,11 @@ function makeWrappedRootLoader(remixVersion: number) {
287332
if (typeof data === 'object') {
288333
return json(
289334
{ ...data, ...traceAndBaggage, remixVersion },
290-
{ headers: res.headers, statusText: res.statusText, status: res.status },
335+
{
336+
headers: res.headers,
337+
statusText: res.statusText,
338+
status: res.status,
339+
},
291340
);
292341
} else {
293342
DEBUG_BUILD && logger.warn('Skipping injection of trace and baggage as the response body is not an object');
@@ -498,7 +547,9 @@ function makeWrappedCreateRequestHandler(
498547
* which Remix Adapters (https://remix.run/docs/en/v1/api/remix) use underneath.
499548
*/
500549
export function instrumentServer(): void {
501-
const pkg = loadModule<{ createRequestHandler: CreateRequestHandlerFunction }>('@remix-run/server-runtime');
550+
const pkg = loadModule<{
551+
createRequestHandler: CreateRequestHandlerFunction;
552+
}>('@remix-run/server-runtime');
502553

503554
if (!pkg) {
504555
DEBUG_BUILD && logger.warn('Remix SDK was unable to require `@remix-run/server-runtime` package.');

‎packages/remix/src/utils/vendor/types.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,12 @@ export type DeferredData = {
108108
};
109109

110110
export interface MetaFunction {
111-
(args: { data: AppData; parentsData: RouteData; params: Params; location: Location }): HtmlMetaDescriptor;
111+
(args: {
112+
data: AppData;
113+
parentsData: RouteData;
114+
params: Params;
115+
location: Location;
116+
}): HtmlMetaDescriptor;
112117
}
113118

114119
export interface HtmlMetaDescriptor {
@@ -143,7 +148,11 @@ export interface LoaderFunction {
143148
}
144149

145150
export interface HeadersFunction {
146-
(args: { loaderHeaders: Headers; parentHeaders: Headers; actionHeaders: Headers }): Headers | HeadersInit;
151+
(args: {
152+
loaderHeaders: Headers;
153+
parentHeaders: Headers;
154+
actionHeaders: Headers;
155+
}): Headers | HeadersInit;
147156
}
148157

149158
export interface ServerRouteModule extends EntryRouteModule {
@@ -197,6 +206,10 @@ export interface DataFunctionArgs {
197206
params: Params;
198207
}
199208

209+
export interface HandleErrorFunction {
210+
(error: unknown, args: DataFunctionArgs): void;
211+
}
212+
200213
export interface DataFunction {
201214
(args: DataFunctionArgs): Promise<Response> | Response | Promise<AppData> | AppData;
202215
}

‎packages/remix/test/integration/app_v1/routes/server-side-unexpected-errors/$id.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export * from '../../../common/routes/server-side-unexpected-errors.$id';
2+
export { default } from '../../../common/routes/server-side-unexpected-errors.$id';

‎packages/remix/test/integration/app_v2/entry.server.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,7 @@ Sentry.init({
1111
autoSessionTracking: false,
1212
});
1313

14-
export function handleError(error: unknown, { request }: DataFunctionArgs): void {
15-
Sentry.captureRemixServerException(error, 'remix.server', request);
16-
}
14+
export const handleError = Sentry.wrapRemixHandleError;
1715

1816
export default function handleRequest(
1917
request: Request,

‎packages/remix/test/integration/app_v2/routes/server-side-unexpected-errors.$id.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export * from '../../common/routes/server-side-unexpected-errors.$id';
2+
export { default } from '../../common/routes/server-side-unexpected-errors.$id';

‎packages/remix/test/integration/common/routes/action-json-response.$id.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ActionFunction, LoaderFunction, json, redirect } from '@remix-run/node'
22
import { useActionData } from '@remix-run/react';
33

44
export const loader: LoaderFunction = async ({ params: { id } }) => {
5-
if (id === '-1') {
5+
if (id === '-100') {
66
throw new Error('Unexpected Server Error');
77
}
88

@@ -16,7 +16,7 @@ export const action: ActionFunction = async ({ params: { id } }) => {
1616

1717
if (id === '-2') {
1818
// Note: This GET request triggers the `Loader` of the URL, not the `Action`.
19-
throw redirect('/action-json-response/-1');
19+
throw redirect('/action-json-response/-100');
2020
}
2121

2222
if (id === '-3') {

‎packages/remix/test/integration/common/routes/server-side-unexpected-errors.$id.tsx

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { ActionFunction, LoaderFunction, json, redirect } from '@remix-run/node';
2+
import { useActionData } from '@remix-run/react';
3+
4+
export const action: ActionFunction = async ({ params: { id } }) => {
5+
// Throw string
6+
if (id === '-1') {
7+
throw 'Thrown String Error';
8+
}
9+
10+
// Throw object
11+
if (id === '-2') {
12+
throw {
13+
message: 'Thrown Object Error',
14+
statusCode: 500,
15+
};
16+
}
17+
};
18+
19+
export default function ActionJSONResponse() {
20+
const data = useActionData();
21+
22+
return (
23+
<div>
24+
<h1>{data && data.test ? data.test : 'Not Found'}</h1>
25+
</div>
26+
);
27+
}

‎packages/remix/test/integration/test/server/action.test.ts

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada
8181
stacktrace: expect.any(Object),
8282
mechanism: {
8383
data: {
84-
function: useV2 ? 'remix.server' : 'action',
84+
function: useV2 ? 'remix.server.handleError' : 'action',
8585
},
8686
handled: false,
8787
type: 'instrument',
@@ -201,7 +201,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada
201201
stacktrace: expect.any(Object),
202202
mechanism: {
203203
data: {
204-
function: useV2 ? 'remix.server' : 'loader',
204+
function: useV2 ? 'remix.server.handleError' : 'loader',
205205
},
206206
handled: false,
207207
type: 'instrument',
@@ -427,4 +427,110 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada
427427
},
428428
});
429429
});
430+
431+
it('handles thrown string from an action', async () => {
432+
const env = await RemixTestEnv.init(adapter);
433+
const url = `${env.url}/server-side-unexpected-errors/-1`;
434+
435+
const envelopes = await env.getMultipleEnvelopeRequest({
436+
url,
437+
count: 2,
438+
method: 'post',
439+
envelopeType: ['event', 'transaction'],
440+
});
441+
442+
const [transaction] = envelopes.filter(envelope => envelope[1].type === 'transaction');
443+
const [event] = envelopes.filter(envelope => envelope[1].type === 'event');
444+
445+
assertSentryTransaction(transaction[2], {
446+
contexts: {
447+
trace: {
448+
op: 'http.server',
449+
status: 'internal_error',
450+
tags: {
451+
method: 'POST',
452+
'http.status_code': '500',
453+
},
454+
data: {
455+
'http.response.status_code': 500,
456+
},
457+
},
458+
},
459+
tags: {
460+
transaction: `routes/server-side-unexpected-errors${useV2 ? '.' : '/'}$id`,
461+
},
462+
});
463+
464+
assertSentryEvent(event[2], {
465+
exception: {
466+
values: [
467+
{
468+
type: 'Error',
469+
value: 'Thrown String Error',
470+
stacktrace: expect.any(Object),
471+
mechanism: {
472+
data: {
473+
function: useV2 ? 'remix.server.handleError' : 'action',
474+
},
475+
handled: false,
476+
type: 'instrument',
477+
},
478+
},
479+
],
480+
},
481+
});
482+
});
483+
484+
it('handles thrown object from an action', async () => {
485+
const env = await RemixTestEnv.init(adapter);
486+
const url = `${env.url}/server-side-unexpected-errors/-2`;
487+
488+
const envelopes = await env.getMultipleEnvelopeRequest({
489+
url,
490+
count: 2,
491+
method: 'post',
492+
envelopeType: ['event', 'transaction'],
493+
});
494+
495+
const [transaction] = envelopes.filter(envelope => envelope[1].type === 'transaction');
496+
const [event] = envelopes.filter(envelope => envelope[1].type === 'event');
497+
498+
assertSentryTransaction(transaction[2], {
499+
contexts: {
500+
trace: {
501+
op: 'http.server',
502+
status: 'internal_error',
503+
tags: {
504+
method: 'POST',
505+
'http.status_code': '500',
506+
},
507+
data: {
508+
'http.response.status_code': 500,
509+
},
510+
},
511+
},
512+
tags: {
513+
transaction: `routes/server-side-unexpected-errors${useV2 ? '.' : '/'}$id`,
514+
},
515+
});
516+
517+
assertSentryEvent(event[2], {
518+
exception: {
519+
values: [
520+
{
521+
type: 'Error',
522+
value: 'Thrown Object Error',
523+
stacktrace: expect.any(Object),
524+
mechanism: {
525+
data: {
526+
function: useV2 ? 'remix.server.handleError' : 'action',
527+
},
528+
handled: false,
529+
type: 'instrument',
530+
},
531+
},
532+
],
533+
},
534+
});
535+
});
430536
});

‎packages/remix/test/integration/test/server/loader.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada
3939
stacktrace: expect.any(Object),
4040
mechanism: {
4141
data: {
42-
function: useV2 ? 'remix.server' : 'loader',
42+
function: useV2 ? 'remix.server.handleError' : 'loader',
4343
},
4444
handled: false,
4545
type: 'instrument',
@@ -138,7 +138,7 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada
138138
stacktrace: expect.any(Object),
139139
mechanism: {
140140
data: {
141-
function: useV2 ? 'remix.server' : 'loader',
141+
function: useV2 ? 'remix.server.handleError' : 'loader',
142142
},
143143
handled: false,
144144
type: 'instrument',

0 commit comments

Comments
 (0)