Skip to content

Commit 439b9d3

Browse files
fix(remix): Wrap handleDocumentRequest functions. (#5387)
We're currently wrapping `action` and `loader` functions of Remix routes for tracing and error capturing. When testing the case in #5362, I realized the `render` function of a SSR component in Remix has another handler [`handleDocumentRequest`](https://github.com/remix-run/remix/blob/b928040061890a6ef0270abdb5b1201638f0dd00/packages/remix-server-runtime/server.ts#L174) which [doesn't re-throw internal errors](https://github.com/remix-run/remix/blob/main/packages/remix-server-runtime/server.ts#L502-L507) so we can't catch them in `wrapRequestHandler`. Also added a tracing span for `handleDocumentRequest`. Co-authored-by: Abhijeet Prasad <[email protected]>
1 parent ae086f9 commit 439b9d3

File tree

1 file changed

+66
-19
lines changed

1 file changed

+66
-19
lines changed

packages/remix/src/utils/instrumentServer.ts

+66-19
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { captureException, configureScope, getCurrentHub, startTransaction } from '@sentry/node';
1+
import { captureException, getCurrentHub, startTransaction } from '@sentry/node';
22
import { getActiveTransaction } from '@sentry/tracing';
33
import { addExceptionMechanism, fill, loadModule, logger, stripUrlQueryAndFragment } from '@sentry/utils';
44

@@ -69,6 +69,65 @@ interface DataFunction {
6969
(args: DataFunctionArgs): Promise<Response> | Response | Promise<AppData> | AppData;
7070
}
7171

72+
function captureRemixServerException(err: Error, name: string): void {
73+
captureException(err, scope => {
74+
scope.addEventProcessor(event => {
75+
addExceptionMechanism(event, {
76+
type: 'instrument',
77+
handled: true,
78+
data: {
79+
function: name,
80+
},
81+
});
82+
83+
return event;
84+
});
85+
86+
return scope;
87+
});
88+
}
89+
90+
function makeWrappedDocumentRequestFunction(
91+
origDocumentRequestFunction: HandleDocumentRequestFunction,
92+
): HandleDocumentRequestFunction {
93+
return async function (
94+
this: unknown,
95+
request: Request,
96+
responseStatusCode: number,
97+
responseHeaders: Headers,
98+
context: Record<symbol, unknown>,
99+
): Promise<Response> {
100+
let res: Response;
101+
102+
const activeTransaction = getActiveTransaction();
103+
const currentScope = getCurrentHub().getScope();
104+
105+
if (!activeTransaction || !currentScope) {
106+
return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context);
107+
}
108+
109+
try {
110+
const span = activeTransaction.startChild({
111+
op: 'remix.server.documentRequest',
112+
description: activeTransaction.name,
113+
tags: {
114+
method: request.method,
115+
url: request.url,
116+
},
117+
});
118+
119+
res = await origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context);
120+
121+
span.finish();
122+
} catch (err) {
123+
captureRemixServerException(err, name);
124+
throw err;
125+
}
126+
127+
return res;
128+
};
129+
}
130+
72131
function makeWrappedDataFunction(origFn: DataFunction, name: 'action' | 'loader'): DataFunction {
73132
return async function (this: unknown, args: DataFunctionArgs): Promise<Response | AppData> {
74133
let res: Response | AppData;
@@ -98,23 +157,7 @@ function makeWrappedDataFunction(origFn: DataFunction, name: 'action' | 'loader'
98157
currentScope.setSpan(activeTransaction);
99158
span.finish();
100159
} catch (err) {
101-
configureScope(scope => {
102-
scope.addEventProcessor(event => {
103-
addExceptionMechanism(event, {
104-
type: 'instrument',
105-
handled: true,
106-
data: {
107-
function: name,
108-
},
109-
});
110-
111-
return event;
112-
});
113-
});
114-
115-
captureException(err);
116-
117-
// Rethrow for other handlers
160+
captureRemixServerException(err, name);
118161
throw err;
119162
}
120163

@@ -160,6 +203,10 @@ function makeWrappedCreateRequestHandler(
160203
return function (this: unknown, build: ServerBuild, mode: string | undefined): RequestHandler {
161204
const routes: ServerRouteManifest = {};
162205

206+
const wrappedEntry = { ...build.entry, module: { ...build.entry.module } };
207+
208+
fill(wrappedEntry.module, 'default', makeWrappedDocumentRequestFunction);
209+
163210
for (const [id, route] of Object.entries(build.routes)) {
164211
const wrappedRoute = { ...route, module: { ...route.module } };
165212

@@ -174,7 +221,7 @@ function makeWrappedCreateRequestHandler(
174221
routes[id] = wrappedRoute;
175222
}
176223

177-
const requestHandler = origCreateRequestHandler.call(this, { ...build, routes }, mode);
224+
const requestHandler = origCreateRequestHandler.call(this, { ...build, routes, entry: wrappedEntry }, mode);
178225

179226
return wrapRequestHandler(requestHandler);
180227
};

0 commit comments

Comments
 (0)