fix(nextjs): Don't wrap res.json and res.send (#6674)

Author: lforst

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/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/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}');
+};