Skip to content

Commit

Permalink
fix(remix): Paramaterize server side transactions (#5491)
Browse files Browse the repository at this point in the history
This makes sure server side transactions are parameterized correctly in Remix.

We do this by matching the server routes the same way Remix does under the hood, by reconciling the routes and URL pathname - https://github.com/remix-run/remix/blob/97999d02493e8114c39d48b76944069d58526e8d/packages/remix-server-runtime/server.ts#L36
  • Loading branch information
AbhiPrasad committed Aug 2, 2022
1 parent cb925e3 commit 6f4ad56
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 15 deletions.
103 changes: 90 additions & 13 deletions packages/remix/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
/* eslint-disable max-lines */
import { captureException, getCurrentHub } from '@sentry/node';
import { getActiveTransaction } from '@sentry/tracing';
import {
addExceptionMechanism,
fill,
loadModule,
logger,
serializeBaggage,
stripUrlQueryAndFragment,
} from '@sentry/utils';
import { addExceptionMechanism, fill, loadModule, logger, serializeBaggage } from '@sentry/utils';

// Types vendored from @remix-run/server-runtime@1.6.0:
// https://github.com/remix-run/remix/blob/f3691d51027b93caa3fd2cdfe146d7b62a6eb8f2/packages/remix-server-runtime/server.ts
Expand Down Expand Up @@ -92,6 +85,18 @@ interface DataFunction {
(args: DataFunctionArgs): Promise<Response> | Response | Promise<AppData> | AppData;
}

interface ReactRouterDomPkg {
matchRoutes: (routes: ServerRoute[], pathname: string) => RouteMatch<ServerRoute>[] | null;
}

// Taken from Remix Implementation
// https://github.com/remix-run/remix/blob/97999d02493e8114c39d48b76944069d58526e8d/packages/remix-server-runtime/routeMatching.ts#L6-L10
export interface RouteMatch<Route> {
params: Params;
pathname: string;
route: Route;
}

// Taken from Remix Implementation
// https://github.com/remix-run/remix/blob/7688da5c75190a2e29496c78721456d6e12e3abe/packages/remix-server-runtime/responses.ts#L54-L62
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -262,18 +267,61 @@ function makeWrappedMeta(origMeta: MetaFunction | HtmlMetaDescriptor = {}): Meta
};
}

function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler {
function createRoutes(manifest: ServerRouteManifest, parentId?: string): ServerRoute[] {
return Object.entries(manifest)
.filter(([, route]) => route.parentId === parentId)
.map(([id, route]) => ({
...route,
children: createRoutes(manifest, id),
}));
}

// Remix Implementation:
// https://github.com/remix-run/remix/blob/38e127b1d97485900b9c220d93503de0deb1fc81/packages/remix-server-runtime/routeMatching.ts#L12-L24
//
// Changed so that `matchRoutes` function is passed in.
function matchServerRoutes(
routes: ServerRoute[],
pathname: string,
pkg?: ReactRouterDomPkg,
): RouteMatch<ServerRoute>[] | null {
if (!pkg) {
return null;
}

const matches = pkg.matchRoutes(routes, pathname);
if (!matches) {
return null;
}

return matches.map(match => ({
params: match.params,
pathname: match.pathname,
route: match.route,
}));
}

function wrapRequestHandler(origRequestHandler: RequestHandler, build: ServerBuild): RequestHandler {
const routes = createRoutes(build.routes);
const pkg = loadModule<ReactRouterDomPkg>('react-router-dom');
return async function (this: unknown, request: Request, loadContext?: unknown): Promise<Response> {
const hub = getCurrentHub();
const currentScope = hub.getScope();

const url = new URL(request.url);
const matches = matchServerRoutes(routes, url.pathname, pkg);

const match = matches && getRequestMatch(url, matches);
const name = match === null ? url.pathname : match.route.id;
const source = match === null ? 'url' : 'route';
const transaction = hub.startTransaction({
name: stripUrlQueryAndFragment(request.url),
name,
op: 'http.server',
tags: {
method: request.method,
},
metadata: {
source: 'url',
source,
},
});

Expand All @@ -290,6 +338,33 @@ function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler
};
}

// https://github.com/remix-run/remix/blob/97999d02493e8114c39d48b76944069d58526e8d/packages/remix-server-runtime/server.ts#L573-L586
function isIndexRequestUrl(url: URL): boolean {
for (const param of url.searchParams.getAll('index')) {
// only use bare `?index` params without a value
// ✅ /foo?index
// ✅ /foo?index&index=123
// ✅ /foo?index=123&index
// ❌ /foo?index=123
if (param === '') {
return true;
}
}

return false;
}

// https://github.com/remix-run/remix/blob/97999d02493e8114c39d48b76944069d58526e8d/packages/remix-server-runtime/server.ts#L588-L596
function getRequestMatch(url: URL, matches: RouteMatch<ServerRoute>[]): RouteMatch<ServerRoute> {
const match = matches.slice(-1)[0];

if (!isIndexRequestUrl(url) && match.route.id.endsWith('/index')) {
return matches.slice(-2)[0];
}

return match;
}

function makeWrappedCreateRequestHandler(
origCreateRequestHandler: CreateRequestHandlerFunction,
): CreateRequestHandlerFunction {
Expand All @@ -316,9 +391,11 @@ function makeWrappedCreateRequestHandler(
routes[id] = wrappedRoute;
}

const requestHandler = origCreateRequestHandler.call(this, { ...build, routes, entry: wrappedEntry }, mode);
const newBuild = { ...build, routes, entry: wrappedEntry };

const requestHandler = origCreateRequestHandler.call(this, newBuild, mode);

return wrapRequestHandler(requestHandler);
return wrapRequestHandler(requestHandler, newBuild);
};
}

Expand Down
8 changes: 6 additions & 2 deletions packages/remix/test/integration/test/server/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,17 @@ describe('Remix API Loaders', () => {
const transaction = envelope[2];

assertSentryTransaction(transaction, {
transaction: 'routes/loader-json-response/$id',
transaction_info: {
source: 'route',
},
spans: [
{
description: url,
description: 'routes/loader-json-response/$id',
op: 'remix.server.loader',
},
{
description: url,
description: 'routes/loader-json-response/$id',
op: 'remix.server.documentRequest',
},
],
Expand Down

0 comments on commit 6f4ad56

Please sign in to comment.