Skip to content

Commit 152e901

Browse files
lforstlforst
authored
ref(nextjs): Replace startTrancsaction in withSentry (#9941)
1 parent eadfd61 commit 152e901

File tree

2 files changed

+120
-191
lines changed

2 files changed

+120
-191
lines changed

‎packages/nextjs/src/common/wrapApiHandlerWithSentry.ts

+116-150
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,16 @@
11
import {
22
addTracingExtensions,
33
captureException,
4-
getClient,
4+
continueTrace,
55
getCurrentScope,
66
runWithAsyncContext,
7-
startTransaction,
7+
startSpanManual,
88
} from '@sentry/core';
9-
import type { Transaction } from '@sentry/types';
10-
import {
11-
consoleSandbox,
12-
isString,
13-
logger,
14-
objectify,
15-
stripUrlQueryAndFragment,
16-
tracingContextFromHeaders,
17-
} from '@sentry/utils';
18-
19-
import { DEBUG_BUILD } from './debug-build';
9+
import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils';
10+
2011
import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types';
2112
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
22-
import { autoEndTransactionOnResponseEnd, finishTransaction, flushQueue } from './utils/responseEnd';
13+
import { flushQueue } from './utils/responseEnd';
2314

2415
/**
2516
* Wrap the given API route handler for tracing and error capturing. Thin wrapper around `withSentry`, which only
@@ -84,151 +75,126 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
8475

8576
addTracingExtensions();
8677

87-
// eslint-disable-next-line complexity, @typescript-eslint/no-explicit-any
88-
const boundHandler = runWithAsyncContext(
89-
// eslint-disable-next-line complexity
90-
async () => {
91-
let transaction: Transaction | undefined;
92-
const currentScope = getCurrentScope();
93-
const options = getClient()?.getOptions();
94-
95-
currentScope.setSDKProcessingMetadata({ request: req });
96-
97-
if (options?.instrumenter === 'sentry') {
98-
const sentryTrace =
99-
req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined;
100-
const baggage = req.headers?.baggage;
101-
const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
102-
sentryTrace,
103-
baggage,
104-
);
105-
currentScope.setPropagationContext(propagationContext);
106-
107-
if (DEBUG_BUILD && traceparentData) {
108-
logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`);
78+
return runWithAsyncContext(async () => {
79+
const transactionContext = continueTrace({
80+
sentryTrace: req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined,
81+
baggage: req.headers?.baggage,
82+
});
83+
84+
// prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler)
85+
let reqPath = parameterizedRoute;
86+
87+
// If not, fake it by just replacing parameter values with their names, hoping that none of them match either
88+
// each other or any hard-coded parts of the path
89+
if (!reqPath) {
90+
const url = `${req.url}`;
91+
// pull off query string, if any
92+
reqPath = stripUrlQueryAndFragment(url);
93+
// Replace with placeholder
94+
if (req.query) {
95+
for (const [key, value] of Object.entries(req.query)) {
96+
reqPath = reqPath.replace(`${value}`, `[${key}]`);
10997
}
110-
111-
// prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler)
112-
let reqPath = parameterizedRoute;
113-
114-
// If not, fake it by just replacing parameter values with their names, hoping that none of them match either
115-
// each other or any hard-coded parts of the path
116-
if (!reqPath) {
117-
const url = `${req.url}`;
118-
// pull off query string, if any
119-
reqPath = stripUrlQueryAndFragment(url);
120-
// Replace with placeholder
121-
if (req.query) {
122-
for (const [key, value] of Object.entries(req.query)) {
123-
reqPath = reqPath.replace(`${value}`, `[${key}]`);
98+
}
99+
}
100+
101+
const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
102+
103+
getCurrentScope().setSDKProcessingMetadata({ request: req });
104+
105+
return startSpanManual(
106+
{
107+
...transactionContext,
108+
name: `${reqMethod}${reqPath}`,
109+
op: 'http.server',
110+
origin: 'auto.http.nextjs',
111+
metadata: {
112+
...transactionContext.metadata,
113+
source: 'route',
114+
request: req,
115+
},
116+
},
117+
async span => {
118+
// eslint-disable-next-line @typescript-eslint/unbound-method
119+
res.end = new Proxy(res.end, {
120+
apply(target, thisArg, argArray) {
121+
span?.setHttpStatus(res.statusCode);
122+
span?.end();
123+
if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
124+
target.apply(thisArg, argArray);
125+
} else {
126+
// flushQueue will not reject
127+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
128+
flushQueue().then(() => {
129+
target.apply(thisArg, argArray);
130+
});
124131
}
125-
}
126-
}
127-
128-
const reqMethod = `${(req.method || 'GET').toUpperCase()} `;
129-
130-
transaction = startTransaction(
131-
{
132-
name: `${reqMethod}${reqPath}`,
133-
op: 'http.server',
134-
origin: 'auto.http.nextjs',
135-
...traceparentData,
136-
metadata: {
137-
dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
138-
source: 'route',
139-
request: req,
140-
},
141132
},
142-
// extra context passed to the `tracesSampler`
143-
{ request: req },
144-
);
145-
currentScope.setSpan(transaction);
146-
if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
147-
autoEndTransactionOnResponseEnd(transaction, res);
148-
} else {
149-
// If we're not on a platform that supports streaming, we're blocking res.end() until the queue is flushed.
150-
// res.json() and res.send() will implicitly call res.end(), so it is enough to wrap res.end().
151-
152-
// eslint-disable-next-line @typescript-eslint/unbound-method
153-
const origResEnd = res.end;
154-
res.end = async function (this: unknown, ...args: unknown[]) {
155-
if (transaction) {
156-
finishTransaction(transaction, res);
157-
await flushQueue();
158-
}
133+
});
159134

160-
origResEnd.apply(this, args);
161-
};
162-
}
163-
}
135+
try {
136+
const handlerResult = await wrappingTarget.apply(thisArg, args);
137+
if (
138+
process.env.NODE_ENV === 'development' &&
139+
!process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR &&
140+
!res.finished
141+
// TODO(v8): Remove this warning?
142+
// This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating.
143+
// Warning suppression on Next.JS is only necessary in that case.
144+
) {
145+
consoleSandbox(() => {
146+
// eslint-disable-next-line no-console
147+
console.warn(
148+
'[sentry] If Next.js logs a warning "API resolved without sending a response", it\'s a false positive, which may happen when you use `withSentry` manually to wrap your routes. To suppress this warning, set `SENTRY_IGNORE_API_RESOLUTION_ERROR` to 1 in your env. To suppress the nextjs warning, use the `externalResolver` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).',
149+
);
150+
});
151+
}
164152

165-
try {
166-
const handlerResult = await wrappingTarget.apply(thisArg, args);
167-
168-
if (
169-
process.env.NODE_ENV === 'development' &&
170-
!process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR &&
171-
!res.finished
172-
// This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating.
173-
// Warning suppression on Next.JS is only necessary in that case.
174-
) {
175-
consoleSandbox(() => {
176-
// eslint-disable-next-line no-console
177-
console.warn(
178-
`[sentry] If Next.js logs a warning "API resolved without sending a response", it's a false positive, which may happen when you use \`withSentry\` manually to wrap your routes.
179-
To suppress this warning, set \`SENTRY_IGNORE_API_RESOLUTION_ERROR\` to 1 in your env.
180-
To suppress the nextjs warning, use the \`externalResolver\` API route option (see https://nextjs.org/docs/api-routes/api-middlewares#custom-config for details).`,
181-
);
153+
return handlerResult;
154+
} catch (e) {
155+
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
156+
// store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced
157+
// to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a
158+
// way to prevent it from actually being reported twice.)
159+
const objectifiedErr = objectify(e);
160+
161+
captureException(objectifiedErr, {
162+
mechanism: {
163+
type: 'instrument',
164+
handled: false,
165+
data: {
166+
wrapped_handler: wrappingTarget.name,
167+
function: 'withSentry',
168+
},
169+
},
182170
});
183-
}
184171

185-
return handlerResult;
186-
} catch (e) {
187-
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
188-
// store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced
189-
// to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a
190-
// way to prevent it from actually being reported twice.)
191-
const objectifiedErr = objectify(e);
192-
193-
captureException(objectifiedErr, {
194-
mechanism: {
195-
type: 'instrument',
196-
handled: false,
197-
data: {
198-
wrapped_handler: wrappingTarget.name,
199-
function: 'withSentry',
200-
},
201-
},
202-
});
172+
// Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet
173+
// have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that
174+
// the transaction was error-free
175+
res.statusCode = 500;
176+
res.statusMessage = 'Internal Server Error';
177+
178+
span?.setHttpStatus(res.statusCode);
179+
span?.end();
180+
181+
// Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors
182+
// out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the
183+
// moment they detect an error, so it's important to get this done before rethrowing the error. Apps not
184+
// deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already
185+
// be finished and the queue will already be empty, so effectively it'll just no-op.)
186+
if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
187+
await flushQueue();
188+
}
203189

204-
// Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet
205-
// have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that
206-
// the transaction was error-free
207-
res.statusCode = 500;
208-
res.statusMessage = 'Internal Server Error';
209-
210-
finishTransaction(transaction, res);
211-
212-
// Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors
213-
// out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the
214-
// moment they detect an error, so it's important to get this done before rethrowing the error. Apps not
215-
// deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already
216-
// be finished and the queue will already be empty, so effectively it'll just no-op.)
217-
if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
218-
await flushQueue();
190+
// We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it
191+
// would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark
192+
// the error as already having been captured.)
193+
throw objectifiedErr;
219194
}
220-
221-
// We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it
222-
// would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark
223-
// the error as already having been captured.)
224-
throw objectifiedErr;
225-
}
226-
},
227-
);
228-
229-
// Since API route handlers are all async, nextjs always awaits the return value (meaning it's fine for us to return
230-
// a promise here rather than a real result, and it saves us the overhead of an `await` call.)
231-
return boundHandler;
195+
},
196+
);
197+
});
232198
},
233199
});
234200
}

‎packages/nextjs/test/config/withSentry.test.ts

+4-41
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import * as SentryCore from '@sentry/core';
22
import { addTracingExtensions } from '@sentry/core';
3-
import type { Client, ClientOptions } from '@sentry/types';
43
import type { NextApiRequest, NextApiResponse } from 'next';
54

65
import type { AugmentedNextApiResponse, NextApiHandler } from '../../src/common/types';
@@ -10,37 +9,7 @@ import { withSentry } from '../../src/server';
109
// constructor but the client isn't used in these tests.
1110
addTracingExtensions();
1211

13-
const FLUSH_DURATION = 200;
14-
15-
async function sleep(ms: number): Promise<void> {
16-
await new Promise(resolve => setTimeout(resolve, ms));
17-
}
18-
/**
19-
* Helper to prevent tests from ending before `flush()` has finished its work.
20-
*
21-
* This is necessary because, like its real-life counterpart, our mocked `res.send()` below doesn't await `res.end()
22-
* (which becomes async when we wrap it in `withSentry` in order to give `flush()` time to run). In real life, the
23-
* request/response cycle is held open as subsequent steps wait for `end()` to emit its `prefinished` event. Here in
24-
* tests, without any of that other machinery, we have to hold it open ourselves.
25-
*
26-
* @param wrappedHandler
27-
* @param req
28-
* @param res
29-
*/
30-
async function callWrappedHandler(wrappedHandler: NextApiHandler, req: NextApiRequest, res: NextApiResponse) {
31-
await wrappedHandler(req, res);
32-
33-
// we know we need to wait at least this long for `flush()` to finish
34-
await sleep(FLUSH_DURATION);
35-
36-
// should be <1 second, just long enough the `flush()` call to return, the original (pre-wrapping) `res.end()` to be
37-
// called, and the response to be marked as done
38-
while (!res.finished) {
39-
await sleep(100);
40-
}
41-
}
42-
43-
const startTransactionSpy = jest.spyOn(SentryCore, 'startTransaction');
12+
const startSpanManualSpy = jest.spyOn(SentryCore, 'startSpanManual');
4413

4514
describe('withSentry', () => {
4615
let req: NextApiRequest, res: NextApiResponse;
@@ -70,24 +39,18 @@ describe('withSentry', () => {
7039

7140
describe('tracing', () => {
7241
it('starts a transaction and sets metadata when tracing is enabled', async () => {
73-
jest.spyOn(SentryCore.Hub.prototype, 'getClient').mockReturnValueOnce({
74-
getOptions: () => ({ tracesSampleRate: 1, instrumenter: 'sentry' }) as ClientOptions,
75-
} as Client);
76-
77-
await callWrappedHandler(wrappedHandlerNoError, req, res);
78-
79-
expect(startTransactionSpy).toHaveBeenCalledWith(
42+
await wrappedHandlerNoError(req, res);
43+
expect(startSpanManualSpy).toHaveBeenCalledWith(
8044
{
8145
name: 'GET http://dogs.are.great',
8246
op: 'http.server',
8347
origin: 'auto.http.nextjs',
84-
8548
metadata: {
8649
source: 'route',
8750
request: expect.objectContaining({ url: 'http://dogs.are.great' }),
8851
},
8952
},
90-
{ request: expect.objectContaining({ url: 'http://dogs.are.great' }) },
53+
expect.any(Function),
9154
);
9255
});
9356
});

0 commit comments

Comments
 (0)