Merge remote-tracking branch 'origin/master' into lforst-refactor-proxy-loader

Author: lforst

docs/event-sending.md

@@ -77,8 +77,8 @@ This document gives an outline for how event sending works, and which which plac
 ## Replay (WIP)
 
 * `replay.sendReplayRequest()`
-  * `createPayload()`
-  * `getReplayEvent()`
+  * `createRecordingData()`
+  * `prepareReplayEvent()`
     * `client._prepareEvent()` (see baseclient)
       * `baseclient._applyClientOptions()`
       * `baseclient._applyIntegrationsMetadata()`

packages/nextjs/src/config/loaders/wrappingLoader.ts

@@ -22,6 +22,7 @@ type LoaderOptions = {
   pagesDir: string;
   pageExtensionRegex: string;
   excludeServerRoutes: Array<RegExp | string>;
+  isEdgeRuntime: boolean;
 };
 
 /**
@@ -33,16 +34,22 @@ export default function wrappingLoader(
   this: LoaderThis<LoaderOptions>,
   userCode: string,
   userModuleSourceMap: any,
-): void {
-  this.async();
-
+): void | string {
   // We know one or the other will be defined, depending on the version of webpack being used
   const {
     pagesDir,
     pageExtensionRegex,
     excludeServerRoutes = [],
+    isEdgeRuntime,
   } = 'getOptions' in this ? this.getOptions() : this.query;
 
+  // We currently don't support the edge runtime
+  if (isEdgeRuntime) {
+    return userCode;
+  }
+
+  this.async();
+
   // Get the parameterized route name from this page's filepath
   const parameterizedRoute = path
     // Get the path of the file insde of the pages directory

packages/nextjs/src/config/templates/apiWrapperTemplate.ts

@@ -23,7 +23,7 @@ type NextApiModule = (
     }
   // CJS export
   | NextApiHandler
-) & { config?: PageConfig & { runtime?: string } };
+) & { config?: PageConfig };
 
 const userApiModule = origModule as NextApiModule;
 
@@ -53,21 +53,7 @@ export const config = {
   },
 };
 
-// This is a variable that Next.js will string replace during build with a string if run in an edge runtime from Next.js
-// v12.2.1-canary.3 onwards:
-// https://github.com/vercel/next.js/blob/166e5fb9b92f64c4b5d1f6560a05e2b9778c16fb/packages/next/build/webpack-config.ts#L206
-// https://edge-runtime.vercel.sh/features/available-apis#addressing-the-runtime
-declare const EdgeRuntime: string | undefined;
-
-let exportedHandler;
-
-if (typeof EdgeRuntime === 'string') {
-  exportedHandler = userProvidedHandler;
-} else {
-  exportedHandler = userProvidedHandler ? Sentry.withSentryAPI(userProvidedHandler, '__ROUTE__') : undefined;
-}
-
-export default exportedHandler;
+export default userProvidedHandler ? Sentry.withSentryAPI(userProvidedHandler, '__ROUTE__') : undefined;
 
 // Re-export anything exported by the page module we're wrapping. When processing this code, Rollup is smart enough to
 // not include anything whose name matchs something we've explicitly exported above.

packages/nextjs/src/config/types.ts

@@ -137,7 +137,7 @@ export type BuildContext = {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   defaultLoaders: any;
   totalPages: number;
-  nextRuntime?: 'nodejs' | 'edge';
+  nextRuntime?: 'nodejs' | 'edge'; // Added in Next.js 12+
 };
 
 /**

packages/nextjs/src/config/webpack.ts

@@ -85,6 +85,13 @@ export function constructWebpackConfigFunction(
     // Add a loader which will inject code that sets global values
     addValueInjectionLoader(newConfig, userNextConfig, userSentryOptions);
 
+    if (buildContext.nextRuntime === 'edge') {
+      // eslint-disable-next-line no-console
+      console.warn(
+        '[@sentry/nextjs] You are using edge functions or middleware. Please note that Sentry does not yet support error monitoring for these features.',
+      );
+    }
+
     if (isServer) {
       if (userSentryOptions.autoInstrumentServerFunctions !== false) {
         const pagesDir = newConfig.resolve?.alias?.['private-next-pages'] as string;
@@ -103,6 +110,7 @@ export function constructWebpackConfigFunction(
                 pagesDir,
                 pageExtensionRegex,
                 excludeServerRoutes: userSentryOptions.excludeServerRoutes,
+                isEdgeRuntime: buildContext.nextRuntime === 'edge',
               },
             },
           ],
@@ -306,7 +314,15 @@ async function addSentryToEntryProperty(
 
   // inject into all entry points which might contain user's code
   for (const entryPointName in newEntryProperty) {
-    if (shouldAddSentryToEntryPoint(entryPointName, isServer, userSentryOptions.excludeServerRoutes, isDev)) {
+    if (
+      shouldAddSentryToEntryPoint(
+        entryPointName,
+        isServer,
+        userSentryOptions.excludeServerRoutes,
+        isDev,
+        buildContext.nextRuntime === 'edge',
+      )
+    ) {
       addFilesToExistingEntryPoint(newEntryProperty, entryPointName, filesToInject);
     } else {
       if (
@@ -433,7 +449,13 @@ function shouldAddSentryToEntryPoint(
   isServer: boolean,
   excludeServerRoutes: Array<string | RegExp> = [],
   isDev: boolean,
+  isEdgeRuntime: boolean,
 ): boolean {
+  // We don't support the Edge runtime yet
+  if (isEdgeRuntime) {
+    return false;
+  }
+
   // On the server side, by default we inject the `Sentry.init()` code into every page (with a few exceptions).
   if (isServer) {
     const entryPointRoute = entryPointName.replace(/^pages/, '');
@@ -530,7 +552,13 @@ export function getWebpackPluginOptions(
     stripPrefix: ['webpack://_N_E/'],
     urlPrefix,
     entries: (entryPointName: string) =>
-      shouldAddSentryToEntryPoint(entryPointName, isServer, userSentryOptions.excludeServerRoutes, isDev),
+      shouldAddSentryToEntryPoint(
+        entryPointName,
+        isServer,
+        userSentryOptions.excludeServerRoutes,
+        isDev,
+        buildContext.nextRuntime === 'edge',
+      ),
     release: getSentryRelease(buildId),
     dryRun: isDev,
   });

packages/nextjs/src/config/wrappers/types.ts

@@ -25,6 +25,11 @@ import type { NextApiRequest, NextApiResponse } from 'next';
 export type NextApiHandler = {
   (req: NextApiRequest, res: NextApiResponse): void | Promise<void> | unknown | Promise<unknown>;
   __sentry_route__?: string;
+
+  /**
+   * A property we set in our integration tests to simulate running an API route on platforms that don't support streaming.
+   */
+  __sentry_test_doesnt_support_streaming__?: true;
 };
 
 export type WrappedNextApiHandler = {

packages/nextjs/src/config/wrappers/utils/responseEnd.ts

@@ -31,7 +31,8 @@ export function autoEndTransactionOnResponseEnd(transaction: Transaction, res: S
   };
 
   // Prevent double-wrapping
-  if (!(res.end as WrappedResponseEndMethod).__sentry_original__) {
+  // res.end may be undefined during build when using `next export` to statically export a Next.js app
+  if (res.end && !(res.end as WrappedResponseEndMethod).__sentry_original__) {
     fill(res, 'end', wrapEndMethod);
   }
 }

packages/nextjs/src/config/wrappers/withSentryAPI.ts

@@ -130,31 +130,11 @@ export function withSentry(origHandler: NextApiHandler, parameterizedRoute?: str
           );
           currentScope.setSpan(transaction);
 
-          if (platformSupportsStreaming()) {
+          if (platformSupportsStreaming() && !origHandler.__sentry_test_doesnt_support_streaming__) {
             autoEndTransactionOnResponseEnd(transaction, res);
           } else {
-            // If we're not on a platform that supports streaming, we're blocking all response-ending methods until the
-            // queue is flushed.
-
-            const origResSend = res.send;
-            res.send = async function (this: unknown, ...args: unknown[]) {
-              if (transaction) {
-                await finishTransaction(transaction, res);
-                await flushQueue();
-              }
-
-              origResSend.apply(this, args);
-            };
-
-            const origResJson = res.json;
-            res.json = async function (this: unknown, ...args: unknown[]) {
-              if (transaction) {
-                await finishTransaction(transaction, res);
-                await flushQueue();
-              }
-
-              origResJson.apply(this, args);
-            };
+            // If we're not on a platform that supports streaming, we're blocking res.end() until the queue is flushed.
+            // res.json() and res.send() will implicitly call res.end(), so it is enough to wrap res.end().
 
             // eslint-disable-next-line @typescript-eslint/unbound-method
             const origResEnd = res.end;
@@ -223,7 +203,7 @@ export function withSentry(origHandler: NextApiHandler, parameterizedRoute?: str
         // moment they detect an error, so it's important to get this done before rethrowing the error. Apps not
         // deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already
         // be finished and the queue will already be empty, so effectively it'll just no-op.)
-        if (platformSupportsStreaming()) {
+        if (platformSupportsStreaming() && !origHandler.__sentry_test_doesnt_support_streaming__) {
           void finishTransaction(transaction, res);
         } else {
           await finishTransaction(transaction, res);

packages/nextjs/src/index.client.ts

@@ -2,7 +2,6 @@ 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 { logger } from '@sentry/utils';
 
 import { nextRouterInstrumentation } from './performance/client';
 import { buildMetadata } from './utils/metadata';
@@ -31,26 +30,12 @@ export { BrowserTracing };
 // Treeshakable guard to remove all code related to tracing
 declare const __SENTRY_TRACING__: boolean;
 
-// This is a variable that Next.js will string replace during build with a string if run in an edge runtime from Next.js
-// v12.2.1-canary.3 onwards:
-// https://github.com/vercel/next.js/blob/166e5fb9b92f64c4b5d1f6560a05e2b9778c16fb/packages/next/build/webpack-config.ts#L206
-// https://edge-runtime.vercel.sh/features/available-apis#addressing-the-runtime
-declare const EdgeRuntime: string | undefined;
-
 const globalWithInjectedValues = global as typeof global & {
   __rewriteFramesAssetPrefixPath__: string;
 };
 
 /** Inits the Sentry NextJS SDK on the browser with the React SDK. */
 export function init(options: NextjsOptions): void {
-  if (typeof EdgeRuntime === 'string') {
-    // If the SDK is imported when using the Vercel Edge Runtime, it will import the browser SDK, even though it is
-    // running the server part of a Next.js application. We can use the `EdgeRuntime` to check for that case and make
-    // the init call a no-op. This will prevent the SDK from crashing on the Edge Runtime.
-    __DEBUG_BUILD__ && logger.log('Vercel Edge Runtime detected. Will not initialize SDK.');
-    return;
-  }
-
   applyTunnelRouteOption(options);
   buildMetadata(options, ['nextjs', 'react']);
   options.environment = options.environment || process.env.NODE_ENV;

packages/nextjs/src/index.server.ts

@@ -30,12 +30,6 @@ const globalWithInjectedValues = global as typeof global & {
 
 const domain = domainModule as typeof domainModule & { active: (domainModule.Domain & Carrier) | null };
 
-// This is a variable that Next.js will string replace during build with a string if run in an edge runtime from Next.js
-// v12.2.1-canary.3 onwards:
-// https://github.com/vercel/next.js/blob/166e5fb9b92f64c4b5d1f6560a05e2b9778c16fb/packages/next/build/webpack-config.ts#L206
-// https://edge-runtime.vercel.sh/features/available-apis#addressing-the-runtime
-declare const EdgeRuntime: string | undefined;
-
 // Exporting this constant means we can compute it without the linter complaining, even if we stop directly using it in
 // this file. It's important that it be computed as early as possible, because one of its indicators is seeing 'build'
 // (as in the CLI command `next build`) in `process.argv`. Later on in the build process, everything's been spun out
@@ -51,11 +45,6 @@ export function init(options: NextjsOptions): void {
     logger.enable();
   }
 
-  if (typeof EdgeRuntime === 'string') {
-    __DEBUG_BUILD__ && logger.log('Vercel Edge Runtime detected. Will not initialize SDK.');
-    return;
-  }
-
   __DEBUG_BUILD__ && logger.log('Initializing SDK...');
 
   if (sdkAlreadyInitialized()) {

packages/nextjs/test/integration/pages/api/doubleEndMethodOnVercel.ts

@@ -0,0 +1,11 @@
+import { NextApiRequest, NextApiResponse } from 'next';
+
+const handler = async (_req: NextApiRequest, res: NextApiResponse): Promise<void> => {
+  // This handler calls .end twice. We test this to verify that this still doesn't throw because we're wrapping `.end`.
+  res.status(200).json({ success: true });
+  res.end();
+};
+
+handler.__sentry_test_doesnt_support_streaming__ = true;
+
+export default handler;

packages/nextjs/test/integration/test/server/doubleEndMethodOnVercel.js

@@ -0,0 +1,10 @@
+const assert = require('assert');
+const { getAsync } = require('../utils/server');
+
+// This test asserts that our wrapping of `res.end` doesn't break API routes on Vercel if people call `res.json` or
+// `res.send` multiple times in one request handler.
+//  https://github.com/getsentry/sentry-javascript/issues/6670
+module.exports = async ({ url: urlBase }) => {
+  const response = await getAsync(`${urlBase}/api/doubleEndMethodOnVercel`);
+  assert.equal(response, '{"success":true}');
+};

packages/replay/.eslintrc.js

@@ -25,10 +25,8 @@ module.exports = {
       rules: {
         // TODO (high-prio): Re-enable this after migration
         '@typescript-eslint/explicit-member-accessibility': 'off',
-        // TODO (high-prio): Re-enable this after migration
+        // Since we target only es6 here, we can leave this off
         '@sentry-internal/sdk/no-async-await': 'off',
-        // TODO (medium-prio): Re-enable this after migration
-        'jsdoc/require-jsdoc': 'off',
       },
     },
     {

packages/replay/src/coreHandlers/breadcrumbHandler.ts

@@ -4,6 +4,9 @@ import type { InstrumentationTypeBreadcrumb } from '../types';
 import { DomHandlerData, handleDom } from './handleDom';
 import { handleScope } from './handleScope';
 
+/**
+ * An event handler to react to breadcrumbs.
+ */
 export function breadcrumbHandler(type: InstrumentationTypeBreadcrumb, handlerData: unknown): Breadcrumb | null {
   if (type === 'scope') {
     return handleScope(handlerData as Scope);

packages/replay/src/coreHandlers/handleDom.ts

@@ -9,6 +9,9 @@ export interface DomHandlerData {
   event: Node | { target: Node };
 }
 
+/**
+ * An event handler to react to DOM events.
+ */
 export function handleDom(handlerData: DomHandlerData): Breadcrumb | null {
   // Taken from https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/breadcrumbs.ts#L112
   let target;

packages/replay/src/coreHandlers/handleFetch.ts

@@ -1,5 +1,4 @@
-import type { ReplayPerformanceEntry } from '../createPerformanceEntry';
-import type { ReplayContainer } from '../types';
+import type { ReplayContainer, ReplayPerformanceEntry } from '../types';
 import { createPerformanceSpans } from '../util/createPerformanceSpans';
 import { shouldFilterRequest } from '../util/shouldFilterRequest';
 

packages/replay/src/coreHandlers/handleHistory.ts

@@ -1,5 +1,4 @@
-import { ReplayPerformanceEntry } from '../createPerformanceEntry';
-import type { ReplayContainer } from '../types';
+import type { ReplayContainer, ReplayPerformanceEntry } from '../types';
 import { createPerformanceSpans } from '../util/createPerformanceSpans';
 
 interface HistoryHandlerData {

packages/replay/src/coreHandlers/handleScope.ts

@@ -4,6 +4,9 @@ import { createBreadcrumb } from '../util/createBreadcrumb';
 
 let _LAST_BREADCRUMB: null | Breadcrumb = null;
 
+/**
+ * An event handler to handle scope changes.
+ */
 export function handleScope(scope: Scope): Breadcrumb | null {
   const newBreadcrumb = scope.getLastBreadcrumb();
 

packages/replay/src/coreHandlers/handleXhr.ts

@@ -1,5 +1,4 @@
-import { ReplayPerformanceEntry } from '../createPerformanceEntry';
-import type { ReplayContainer } from '../types';
+import type { ReplayContainer, ReplayPerformanceEntry } from '../types';
 import { createPerformanceSpans } from '../util/createPerformanceSpans';
 import { shouldFilterRequest } from '../util/shouldFilterRequest';