feat(remix): Export a manual wrapper for custom Express servers.

packages/remix/package.json

@@ -60,10 +60,10 @@
     "lint:eslint": "eslint . --cache --cache-location '../../eslintcache/' --format stylish",
     "lint:prettier": "prettier --check \"{src,test,scripts}/**/*.ts\"",
     "test": "run-s test:unit",
-    "test:integration": "run-s test:integration:prepare test:integration:client test:integration:server",
-    "test:integration:clean": "(cd test/integration && rimraf .cache build node_modules)",
-    "test:integration:ci": "run-s test:integration:prepare test:integration:client:ci test:integration:server",
-    "test:integration:prepare": "yarn test:integration:clean && (cd test/integration && yarn)",
+    "test:integration": "run-s test:integration:clean test:integration:prepare test:integration:client test:integration:server",
+    "test:integration:ci": "run-s test:integration:clean test:integration:prepare test:integration:client:ci test:integration:server",
+    "test:integration:prepare": "(cd test/integration && yarn)",
+    "test:integration:clean": "(cd test/integration && rimraf .cache node_modules build)",
     "test:integration:client": "yarn playwright install-deps && yarn playwright test test/integration/test/client/",
     "test:integration:client:ci": "yarn test:integration:client --browser='all' --reporter='line'",
     "test:integration:server": "jest --config=test/integration/jest.config.js test/integration/test/server/",

packages/remix/src/index.server.ts

@@ -10,6 +10,7 @@ export { ErrorBoundary, withErrorBoundary } from '@sentry/react';
 export { remixRouterInstrumentation, withSentry } from './performance/client';
 export { BrowserTracing, Integrations } from '@sentry/tracing';
 export * from '@sentry/node';
+export { wrapExpressCreateRequestHandler } from './utils/serverAdapters/express';
 
 function sdkAlreadyInitialized(): boolean {
   const hub = getCurrentHub();

packages/remix/src/utils/instrumentServer.ts

@@ -1,103 +1,25 @@
 /* eslint-disable max-lines */
 import { captureException, getCurrentHub } from '@sentry/node';
-import { getActiveTransaction, hasTracingEnabled } from '@sentry/tracing';
+import { getActiveTransaction, hasTracingEnabled, Span } from '@sentry/tracing';
+import { WrappedFunction } from '@sentry/types';
 import { addExceptionMechanism, fill, isNodeEnv, 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
-type AppLoadContext = unknown;
-type AppData = unknown;
-type RequestHandler = (request: Request, loadContext?: AppLoadContext) => Promise<Response>;
-type CreateRequestHandlerFunction = (build: ServerBuild, mode?: string) => RequestHandler;
-type ServerRouteManifest = RouteManifest<Omit<ServerRoute, 'children'>>;
-type Params<Key extends string = string> = {
-  readonly [key in Key]: string | undefined;
-};
-
-interface Route {
-  index?: boolean;
-  caseSensitive?: boolean;
-  id: string;
-  parentId?: string;
-  path?: string;
-}
-interface RouteData {
-  [routeId: string]: AppData;
-}
-
-interface MetaFunction {
-  (args: { data: AppData; parentsData: RouteData; params: Params; location: Location }): HtmlMetaDescriptor;
-}
-
-interface HtmlMetaDescriptor {
-  [name: string]: null | string | undefined | Record<string, string> | Array<Record<string, string> | string>;
-  charset?: 'utf-8';
-  charSet?: 'utf-8';
-  title?: string;
-}
-
-interface ServerRouteModule {
-  action?: DataFunction;
-  headers?: unknown;
-  loader?: DataFunction;
-  meta?: MetaFunction | HtmlMetaDescriptor;
-}
-
-interface ServerRoute extends Route {
-  children: ServerRoute[];
-  module: ServerRouteModule;
-}
-
-interface RouteManifest<Route> {
-  [routeId: string]: Route;
-}
-
-interface ServerBuild {
-  entry: {
-    module: ServerEntryModule;
-  };
-  routes: ServerRouteManifest;
-  assets: unknown;
-}
-
-interface HandleDocumentRequestFunction {
-  (request: Request, responseStatusCode: number, responseHeaders: Headers, context: Record<symbol, unknown>):
-    | Promise<Response>
-    | Response;
-}
-
-interface HandleDataRequestFunction {
-  (response: Response, args: DataFunctionArgs): Promise<Response> | Response;
-}
-
-interface ServerEntryModule {
-  default: HandleDocumentRequestFunction;
-  meta: MetaFunction;
-  loader: DataFunction;
-  handleDataRequest?: HandleDataRequestFunction;
-}
-
-interface DataFunctionArgs {
-  request: Request;
-  context: AppLoadContext;
-  params: Params;
-}
-
-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;
-}
+import {
+  AppData,
+  CreateRequestHandlerFunction,
+  DataFunction,
+  DataFunctionArgs,
+  HandleDocumentRequestFunction,
+  ReactRouterDomPkg,
+  RequestHandler,
+  RouteMatch,
+  ServerBuild,
+  ServerRoute,
+  ServerRouteManifest,
+} from './types';
+
+// Flag to track if the core request handler is instrumented.
+export let isRequestHandlerWrapped = false;
 
 // Taken from Remix Implementation
 // https://github.com/remix-run/remix/blob/32300ec6e6e8025602cea63e17a2201989589eab/packages/remix-server-runtime/responses.ts#L60-L77
@@ -318,7 +240,13 @@ function makeWrappedRootLoader(origLoader: DataFunction): DataFunction {
   };
 }
 
-function createRoutes(manifest: ServerRouteManifest, parentId?: string): ServerRoute[] {
+/**
+ * Creates routes from the server route manifest
+ *
+ * @param manifest
+ * @param parentId
+ */
+export function createRoutes(manifest: ServerRouteManifest, parentId?: string): ServerRoute[] {
   return Object.entries(manifest)
     .filter(([, route]) => route.parentId === parentId)
     .map(([id, route]) => ({
@@ -352,33 +280,50 @@ function matchServerRoutes(
   }));
 }
 
+/**
+ * Starts a new transaction for the given request to be used by different `RequestHandler` wrappers.
+ *
+ * @param request
+ * @param routes
+ * @param pkg
+ */
+export function startRequestHandlerTransaction(
+  url: URL,
+  method: string,
+  routes: ServerRoute[],
+  pkg?: ReactRouterDomPkg,
+): Span | undefined {
+  const hub = getCurrentHub();
+  const currentScope = hub.getScope();
+  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,
+    op: 'http.server',
+    tags: {
+      method: method,
+    },
+    metadata: {
+      source,
+    },
+  });
+
+  if (transaction) {
+    currentScope?.setSpan(transaction);
+  }
+  return transaction;
+}
+
 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,
-      op: 'http.server',
-      tags: {
-        method: request.method,
-      },
-      metadata: {
-        source,
-      },
-    });
 
-    if (transaction) {
-      currentScope?.setSpan(transaction);
-    }
+    const transaction = startRequestHandlerTransaction(url, request.method, routes, pkg);
 
     const res = (await origRequestHandler.call(this, request, loadContext)) as Response;
 
@@ -416,43 +361,60 @@ function getRequestMatch(url: URL, matches: RouteMatch<ServerRoute>[]): RouteMat
   return match;
 }
 
-function makeWrappedCreateRequestHandler(
-  origCreateRequestHandler: CreateRequestHandlerFunction,
-): CreateRequestHandlerFunction {
-  return function (this: unknown, build: ServerBuild, mode: string | undefined): RequestHandler {
-    const routes: ServerRouteManifest = {};
+/**
+ * Instruments `remix` ServerBuild for performance tracing and error tracking.
+ */
+export function instrumentBuild(build: ServerBuild): ServerBuild {
+  const routes: ServerRouteManifest = {};
 
-    const wrappedEntry = { ...build.entry, module: { ...build.entry.module } };
+  const wrappedEntry = { ...build.entry, module: { ...build.entry.module } };
 
+  // Not keeping boolean flags like it's done for `requestHandler` functions,
+  // Because the build can change between build and runtime.
+  // So if there is a new `loader` or`action` or `documentRequest` after build.
+  // We should be able to wrap them, as they may not be wrapped before.
+  if (!(wrappedEntry.module.default as WrappedFunction).__sentry_original__) {
     fill(wrappedEntry.module, 'default', makeWrappedDocumentRequestFunction);
+  }
 
-    for (const [id, route] of Object.entries(build.routes)) {
-      const wrappedRoute = { ...route, module: { ...route.module } };
-
-      if (wrappedRoute.module.action) {
-        fill(wrappedRoute.module, 'action', makeWrappedAction(id));
-      }
+  for (const [id, route] of Object.entries(build.routes)) {
+    const wrappedRoute = { ...route, module: { ...route.module } };
 
-      if (wrappedRoute.module.loader) {
-        fill(wrappedRoute.module, 'loader', makeWrappedLoader(id));
-      }
+    if (wrappedRoute.module.action && !(wrappedRoute.module.action as WrappedFunction).__sentry_original__) {
+      fill(wrappedRoute.module, 'action', makeWrappedAction(id));
+    }
 
-      // Entry module should have a loader function to provide `sentry-trace` and `baggage`
-      // They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage`
-      if (!wrappedRoute.parentId) {
-        if (!wrappedRoute.module.loader) {
-          wrappedRoute.module.loader = () => ({});
-        }
+    if (wrappedRoute.module.loader && !(wrappedRoute.module.loader as WrappedFunction).__sentry_original__) {
+      fill(wrappedRoute.module, 'loader', makeWrappedLoader(id));
+    }
 
-        fill(wrappedRoute.module, 'loader', makeWrappedRootLoader);
+    // Entry module should have a loader function to provide `sentry-trace` and `baggage`
+    // They will be available for the root `meta` function as `data.sentryTrace` and `data.sentryBaggage`
+    if (!wrappedRoute.parentId) {
+      if (!wrappedRoute.module.loader) {
+        wrappedRoute.module.loader = () => ({});
       }
 
-      routes[id] = wrappedRoute;
+      // We want to wrap the root loader regardless of whether it's already wrapped before.
+      fill(wrappedRoute.module, 'loader', makeWrappedRootLoader);
     }
 
-    const newBuild = { ...build, routes, entry: wrappedEntry };
+    routes[id] = wrappedRoute;
+  }
+
+  return { ...build, routes, entry: wrappedEntry };
+}
+
+function makeWrappedCreateRequestHandler(
+  origCreateRequestHandler: CreateRequestHandlerFunction,
+): CreateRequestHandlerFunction {
+  // To track if this wrapper has been applied, before other wrappers.
+  // Can't track `__sentry_original__` because it's not the same function as the potentially manually wrapped one.
+  isRequestHandlerWrapped = true;
 
-    const requestHandler = origCreateRequestHandler.call(this, newBuild, mode);
+  return function (this: unknown, build: ServerBuild, ...args: unknown[]): RequestHandler {
+    const newBuild = instrumentBuild(build);
+    const requestHandler = origCreateRequestHandler.call(this, newBuild, ...args);
 
     return wrapRequestHandler(requestHandler, newBuild);
   };

packages/remix/src/utils/serverAdapters/express.ts

@@ -0,0 +1,67 @@
+import { extractRequestData, loadModule } from '@sentry/utils';
+
+import {
+  createRoutes,
+  instrumentBuild,
+  isRequestHandlerWrapped,
+  startRequestHandlerTransaction,
+} from '../instrumentServer';
+import {
+  ExpressCreateRequestHandler,
+  ExpressCreateRequestHandlerOptions,
+  ExpressNextFunction,
+  ExpressRequest,
+  ExpressRequestHandler,
+  ExpressResponse,
+  ReactRouterDomPkg,
+  ServerBuild,
+} from '../types';
+
+function wrapExpressRequestHandler(
+  origRequestHandler: ExpressRequestHandler,
+  build: ServerBuild,
+): ExpressRequestHandler {
+  const routes = createRoutes(build.routes);
+  const pkg = loadModule<ReactRouterDomPkg>('react-router-dom');
+
+  // If the core request handler is already wrapped, don't wrap Express handler which uses it.
+  if (isRequestHandlerWrapped) {
+    return origRequestHandler;
+  }
+
+  return async function (
+    this: unknown,
+    req: ExpressRequest,
+    res: ExpressResponse,
+    next: ExpressNextFunction,
+  ): Promise<void> {
+    const request = extractRequestData(req);
+
+    if (!request.url || !request.method) {
+      return origRequestHandler.call(this, req, res, next);
+    }
+
+    const url = new URL(request.url);
+
+    const transaction = startRequestHandlerTransaction(url, request.method, routes, pkg);
+
+    await origRequestHandler.call(this, req, res, next);
+
+    transaction?.setHttpStatus(res.statusCode);
+    transaction?.finish();
+  };
+}
+
+/**
+ * Instruments `createRequestHandler` from `@remix-run/express`
+ */
+export function wrapExpressCreateRequestHandler(
+  origCreateRequestHandler: ExpressCreateRequestHandler,
+): (options: any) => ExpressRequestHandler {
+  return function (this: unknown, options: any): ExpressRequestHandler {
+    const newBuild = instrumentBuild((options as ExpressCreateRequestHandlerOptions).build);
+    const requestHandler = origCreateRequestHandler.call(this, { ...options, build: newBuild });
+
+    return wrapExpressRequestHandler(requestHandler, newBuild);
+  };
+}