Don't start transactions for tunneled events

Luca Forstner · Luca Forstner · commit f3500271a8ad · 2022-12-09T10:37:27.000Z
diff --git a/packages/nextjs/rollup.npm.config.js b/packages/nextjs/rollup.npm.config.js
@@ -20,8 +20,6 @@ export default [
   ...makeNPMConfigVariants(
     makeBaseNPMConfig({
       entrypoints: [
-        'src/config/templates/serverRewriteFramesPrefixLoaderTemplate.ts',
-        'src/config/templates/clientRewriteFramesPrefixLoaderTemplate.ts',
         'src/config/templates/pageProxyLoaderTemplate.ts',
         'src/config/templates/apiProxyLoaderTemplate.ts',
       ],
diff --git a/packages/nextjs/src/config/loaders/index.ts b/packages/nextjs/src/config/loaders/index.ts
@@ -1,2 +1,2 @@
-export { default as prefixLoader } from './prefixLoader';
+export { default as valueInjectionLoader } from './valueInjectionLoader';
 export { default as proxyLoader } from './proxyLoader';
diff --git a/packages/nextjs/src/config/loaders/prefixLoader.ts b/packages/nextjs/src/config/loaders/prefixLoader.ts
diff --git a/packages/nextjs/src/config/loaders/valueInjectionLoader.ts b/packages/nextjs/src/config/loaders/valueInjectionLoader.ts
@@ -0,0 +1,26 @@
+import { LoaderThis } from './types';
+
+type LoaderOptions = {
+  values: Record<string, string>;
+};
+
+/**
+ * Set values on the global/window object at the start of a module.
+ *
+ * Options:
+ *   - `values`: A record where the keys correspond to the keys of the global values to set and the values
+ *        correspond to the values of the values on the global object.
+ */
+export default function valueInjectionLoader(this: LoaderThis<LoaderOptions>, userCode: string): string {
+  // We know one or the other will be defined, depending on the version of webpack being used
+  const { values } = 'getOptions' in this ? this.getOptions() : this.query;
+
+  // Define some global proxy that works on server and on the browser.
+  let injectedCode = 'var _sentryCollisionFreeGlobalObject = typeof window === "undefined" ? global : window;\n';
+
+  Object.entries(values).forEach(([key, value]) => {
+    injectedCode += `_sentryCollisionFreeGlobalObject.${key} = '${value}';\n`;
+  });
+
+  return `${injectedCode}\n${userCode}`;
+}
diff --git a/packages/nextjs/src/config/templates/clientRewriteFramesPrefixLoaderTemplate.ts b/packages/nextjs/src/config/templates/clientRewriteFramesPrefixLoaderTemplate.ts
diff --git a/packages/nextjs/src/config/templates/serverRewriteFramesPrefixLoaderTemplate.ts b/packages/nextjs/src/config/templates/serverRewriteFramesPrefixLoaderTemplate.ts
diff --git a/packages/nextjs/src/config/types.ts b/packages/nextjs/src/config/types.ts
@@ -26,26 +26,10 @@ export type NextConfigFunctionWithSentry = (
   defaults: { defaultConfig: NextConfigObject },
 ) => NextConfigObjectWithSentry;
 
-// Vendored from Next.js
-type RouteHas =
-  | {
-      type: 'header' | 'query' | 'cookie';
-      key: string;
-      value?: string;
-    }
-  | {
-      type: 'host';
-      key?: undefined;
-      value: string;
-    };
-
-// Vendored from Next.js
+// Vendored from Next.js (this type is not complete - extend if necessary)
 type Rewrite = {
   source: string;
   destination: string;
-  basePath?: false;
-  locale?: false;
-  has?: RouteHas[];
 };
 
 export type NextConfigObject = {
@@ -63,7 +47,7 @@ export type NextConfigObject = {
   publicRuntimeConfig?: { [key: string]: unknown };
   // File extensions that count as pages in the `pages/` directory
   pageExtensions?: string[];
-  // TODO
+  // Paths to reroute when requested
   rewrites?: () => Promise<
     | Rewrite[]
     | {
diff --git a/packages/nextjs/src/config/webpack.ts b/packages/nextjs/src/config/webpack.ts
@@ -13,7 +13,6 @@ import { default as SentryWebpackPlugin } from '@sentry/webpack-plugin';
 import * as chalk from 'chalk';
 import * as fs from 'fs';
 import * as path from 'path';
-import * as webpack from 'webpack';
 
 // Note: If you need to import a type from Webpack, do it in `types.ts` and export it from there. Otherwise, our
 // circular dependency check thinks this file is importing from itself. See https://github.com/pahen/madge/issues/306.
@@ -82,8 +81,8 @@ export function constructWebpackConfigFunction(
     // `newConfig.module.rules` is required, so we don't have to keep asserting its existence
     const newConfig = setUpModuleRules(rawNewConfig);
 
-    // Add a loader which will inject code that sets global values for use by `RewriteFrames`
-    addRewriteFramesLoader(newConfig, isServer ? 'server' : 'client', userNextConfig);
+    // Add a loader which will inject code that sets global values
+    addValueInjectionLoader(newConfig, userNextConfig, userSentryOptions);
 
     if (isServer) {
       if (userSentryOptions.autoInstrumentServerFunctions !== false) {
@@ -202,15 +201,6 @@ export function constructWebpackConfigFunction(
       );
     }
 
-    if (userSentryOptions.rewritesTunnel) {
-      newConfig.plugins = newConfig.plugins || [];
-      newConfig.plugins.push(
-        new webpack.DefinePlugin({
-          __SENTRY_REWRITES_TUNNEL_PATH__: JSON.stringify(userSentryOptions.rewritesTunnel),
-        }),
-      );
-    }
-
     return newConfig;
   };
 }
@@ -656,47 +646,50 @@ function setUpModuleRules(newConfig: WebpackConfigObject): WebpackConfigObjectWi
 }
 
 /**
- * Support the `distDir` and `assetPrefix` options by making their values (easy to get here at build-time) available at
- * runtime (for use by `RewriteFrames`), by injecting code to attach their values to `global` or `window`.
- *
- * @param newConfig The webpack config object being constructed
- * @param target Either 'server' or 'client'
- * @param userNextConfig The user's nextjs config options
+ * TODO
  */
-function addRewriteFramesLoader(
+function addValueInjectionLoader(
   newConfig: WebpackConfigObjectWithModuleRules,
-  target: 'server' | 'client',
   userNextConfig: NextConfigObject,
+  userSentryOptions: UserSentryOptions,
 ): void {
-  const replacements = {
-    server: [
-      [
-        '__DIST_DIR__',
-        // Make sure that if we have a windows path, the backslashes are interpreted as such (rather than as escape
-        // characters)
-        userNextConfig.distDir?.replace(/\\/g, '\\\\') || '.next',
-      ],
-    ],
-    client: [
-      [
-        '__ASSET_PREFIX_PATH__',
-        // Get the path part of `assetPrefix`, minus any trailing slash. (We use a placeholder for the origin if
-        // `assetPreix` doesn't include one. Since we only care about the path, it doesn't matter what it is.)
-        userNextConfig.assetPrefix
-          ? new URL(userNextConfig.assetPrefix, 'http://dogs.are.great').pathname.replace(/\/$/, '')
-          : '',
-      ],
-    ],
+  const serverValues = {
+    // Make sure that if we have a windows path, the backslashes are interpreted as such (rather than as escape
+    // characters)
+    __rewriteFramesDistDir__: userNextConfig.distDir?.replace(/\\/g, '\\\\') || '.next',
+    // TODO
+    __sentryRewritesTunnelPath__: userSentryOptions.rewritesTunnel,
+  };
+
+  const clientValues = {
+    // Get the path part of `assetPrefix`, minus any trailing slash. (We use a placeholder for the origin if
+    // `assetPreix` doesn't include one. Since we only care about the path, it doesn't matter what it is.)
+    __rewriteFramesAssetPrefixPath__: userNextConfig.assetPrefix
+      ? new URL(userNextConfig.assetPrefix, 'http://dogs.are.great').pathname.replace(/\/$/, '')
+      : '',
+    // TODO
+    __sentryRewritesTunnelPath__: userSentryOptions.rewritesTunnel,
   };
 
   newConfig.module.rules.push({
-    test: new RegExp(`sentry\\.${target}\\.config\\.(jsx?|tsx?)`),
+    test: /sentry\.server\.config\.(jsx?|tsx?)/,
+    use: [
+      {
+        loader: path.resolve(__dirname, 'loaders/valueInjectionLoader.js'),
+        options: {
+          values: serverValues,
+        },
+      },
+    ],
+  });
+
+  newConfig.module.rules.push({
+    test: /sentry\.client\.config\.(jsx?|tsx?)/,
     use: [
       {
-        loader: path.resolve(__dirname, 'loaders/prefixLoader.js'),
+        loader: path.resolve(__dirname, 'loaders/valueInjectionLoader.js'),
         options: {
-          templatePrefix: `${target}RewriteFrames`,
-          replacements: replacements[target],
+          values: clientValues,
         },
       },
     ],
diff --git a/packages/nextjs/src/config/withSentryConfig.ts b/packages/nextjs/src/config/withSentryConfig.ts
@@ -42,7 +42,7 @@ function getFinalConfigObject(
   delete incomingUserNextConfigObject.sentry;
 
   if (userSentryOptions?.rewritesTunnel) {
-    setUpRewriteRules(incomingUserNextConfigObject, userSentryOptions.rewritesTunnel);
+    setUpTunnelRewriteRules(incomingUserNextConfigObject, userSentryOptions.rewritesTunnel);
   }
 
   // In order to prevent all of our build-time code from being bundled in people's route-handling serverless functions,
@@ -66,23 +66,22 @@ function getFinalConfigObject(
 }
 
 /**
- * TODO
+ * Injects rewrite rules into the Next.js config provided by the user to tunnel
+ * requests from the `tunnelPath` to Sentry.
  */
-function setUpRewriteRules(userNextConfig: NextConfigObject, tunnelPath: string): void {
+function setUpTunnelRewriteRules(userNextConfig: NextConfigObject, tunnelPath: string): void {
   const originalRewrites = userNextConfig.rewrites;
 
   // This function doesn't take any arguments at the time of writing but we future-proof
   // here in case Next.js ever decides to pass some
   userNextConfig.rewrites = async (...args: unknown[]) => {
-    // TODO: Consider trailing slash option
-
     const injectedRewrites = [
       {
-        source: `${tunnelPath}/:orgid/:projectid`, // without slash
+        source: `${tunnelPath}/:orgid/:projectid`, // without trailing slash
         destination: 'https://:orgid.ingest.sentry.io/api/:projectid/envelope/',
       },
       {
-        source: `${tunnelPath}/:orgid/:projectid/`, // with slash
+        source: `${tunnelPath}/:orgid/:projectid/`, // with trailing slash
         destination: 'https://:orgid.ingest.sentry.io/api/:projectid/envelope/',
       },
     ];
diff --git a/packages/nextjs/src/index.client.ts b/packages/nextjs/src/index.client.ts
@@ -2,7 +2,7 @@ import { RewriteFrames } from '@sentry/integrations';
 import { configureScope, init as reactInit, Integrations } from '@sentry/react';
 import { BrowserTracing, defaultRequestInstrumentationOptions, hasTracingEnabled } from '@sentry/tracing';
 import { EventProcessor } from '@sentry/types';
-import { dsnFromString } from '@sentry/utils';
+import { dsnFromString, logger } from '@sentry/utils';
 
 import { nextRouterInstrumentation } from './performance/client';
 import { buildMetadata } from './utils/metadata';
@@ -30,18 +30,23 @@ export { BrowserTracing };
 // Treeshakable guard to remove all code related to tracing
 declare const __SENTRY_TRACING__: boolean;
 
-// Variable that is injected at build-time to have an automatic tunneling setup that uses Next.js' rewrites option
-declare const __SENTRY_REWRITES_TUNNEL_PATH__: string | undefined;
-
-type GlobalWithAssetPrefixPath = typeof global & { __rewriteFramesAssetPrefixPath__: string };
+const globalWithInjectedValues = global as typeof global & {
+  __rewriteFramesAssetPrefixPath__: string;
+  __sentryRewritesTunnelPath__?: string;
+};
 
 /** Inits the Sentry NextJS SDK on the browser with the React SDK. */
 export function init(options: NextjsOptions): void {
-  if (typeof __SENTRY_REWRITES_TUNNEL_PATH__ === 'string' && options.dsn) {
+  let tunnelPath: string | undefined;
+  if (globalWithInjectedValues.__sentryRewritesTunnelPath__ && options.dsn) {
     const dsnComponents = dsnFromString(options.dsn);
-    if (dsnComponents.host.match(/^o\d+\.ingest.sentry.io$/)) {
+    if (dsnComponents.host.match(/^o\d+\.ingest\.sentry\.io$/)) {
       const orgId = dsnComponents.host.split('.')[0];
-      options.tunnel = `${__SENTRY_REWRITES_TUNNEL_PATH__}/${orgId}/${dsnComponents.projectId}/`;
+      tunnelPath = `${globalWithInjectedValues.__sentryRewritesTunnelPath__}/${orgId}/${dsnComponents.projectId}/`;
+      __DEBUG_BUILD__ && logger.info(`Tunneling events to "${tunnelPath}"`);
+      options.tunnel = tunnelPath;
+    } else {
+      __DEBUG_BUILD__ && logger.warn('Provided DSN is not a Sentry SaaS DSN. Will not tunnel events.');
     }
   }
 
@@ -65,7 +70,7 @@ function addClientIntegrations(options: NextjsOptions): void {
 
   // This value is injected at build time, based on the output directory specified in the build config. Though a default
   // is set there, we set it here as well, just in case something has gone wrong with the injection.
-  const assetPrefixPath = (global as GlobalWithAssetPrefixPath).__rewriteFramesAssetPrefixPath__ || '';
+  const assetPrefixPath = globalWithInjectedValues.__rewriteFramesAssetPrefixPath__ || '';
 
   const defaultRewriteFramesIntegration = new RewriteFrames({
     // Turn `<origin>/<path>/_next/static/...` into `app:///_next/static/...`
diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts
@@ -19,7 +19,10 @@ export { captureUnderscoreErrorException } from './utils/_error';
 // because or SSR of next.js we can only use this.
 export { ErrorBoundary, showReportDialog, withErrorBoundary } from '@sentry/react';
 
-type GlobalWithDistDir = typeof global & { __rewriteFramesDistDir__: string };
+const globalWithInjectedValues = global as typeof global & {
+  __rewriteFramesDistDir__: string;
+};
+
 const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null };
 
 // Exporting this constant means we can compute it without the linter complaining, even if we stop directly using it in
@@ -104,7 +107,7 @@ function addServerIntegrations(options: NextjsOptions): void {
 
   // This value is injected at build time, based on the output directory specified in the build config. Though a default
   // is set there, we set it here as well, just in case something has gone wrong with the injection.
-  const distDirName = (global as GlobalWithDistDir).__rewriteFramesDistDir__ || '.next';
+  const distDirName = globalWithInjectedValues.__rewriteFramesDistDir__ || '.next';
   // nextjs always puts the build directory at the project root level, which is also where you run `next start` from, so
   // we can read in the project directory from the currently running process
   const distDirAbsPath = path.resolve(process.cwd(), distDirName);
diff --git a/packages/nextjs/src/utils/instrumentServer.ts b/packages/nextjs/src/utils/instrumentServer.ts
@@ -76,6 +76,10 @@ type WrappedPageComponentFinder = PageComponentFinder;
 let liveServer: Server;
 let sdkSetupComplete = false;
 
+const globalWithInjectedValues = global as typeof global & {
+  __sentryRewritesTunnelPath__?: string;
+};
+
 /**
  * Do the monkeypatching and wrapping necessary to catch errors in page routes and record transactions for both page and
  * API routes.
@@ -346,12 +350,26 @@ function makeWrappedMethodForGettingParameterizedPath(
 /**
  * Determine if the request should be traced, by filtering out requests for internal next files and static resources.
  *
- * @param url The URL of the request
+ * @param path The path of the request
  * @param publicDirFiles A set containing relative paths to all available static resources (note that this does not
  * include static *pages*, but rather images and the like)
  * @returns false if the URL is for an internal or static resource
  */
-function shouldTraceRequest(url: string, publicDirFiles: Set<string>): boolean {
+function shouldTraceRequest(path: string, publicDirFiles: Set<string>): boolean {
+  const tunnelPath = globalWithInjectedValues.__sentryRewritesTunnelPath__;
+  if (tunnelPath) {
+    const startsWithTunnelPath = path.startsWith(tunnelPath);
+    const transactionNameRest = path.substring(tunnelPath.length);
+
+    // Check if the requested path matches the path we tunnel requests to
+    const restMatchesTunnelStructure = transactionNameRest.match(/^\/o\d+\/\d+\/?$/);
+
+    if (startsWithTunnelPath && restMatchesTunnelStructure) {
+      __DEBUG_BUILD__ && logger.log(`Tunneling Sentry event to "${path}"`);
+      return false;
+    }
+  }
+
   // `static` is a deprecated but still-functional location for static resources
-  return !url.startsWith('/_next/') && !url.startsWith('/static/') && !publicDirFiles.has(url);
+  return !path.startsWith('/_next/') && !path.startsWith('/static/') && !publicDirFiles.has(path);
 }
diff --git a/packages/nextjs/test/config/loaders.test.ts b/packages/nextjs/test/config/loaders.test.ts

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`		`-export { default as prefixLoader } from './prefixLoader';`
	`1`	`+export { default as valueInjectionLoader } from './valueInjectionLoader';`
`2`	`2`	`export { default as proxyLoader } from './proxyLoader';`