Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/nextjs/src/config/wrappers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
28 changes: 4 additions & 24 deletions packages/nextjs/src/config/wrappers/withSentryAPI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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}');
};