Skip to content

ref(nextjs): Replace startTrancsaction in withSentry #9941

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.

Sign up for GitHub

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 8 commits into from
Dec 27, 2023
Merged

ref(nextjs): Replace startTrancsaction in withSentry #9941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e06908d
ref(nextjs): Replace `startTrancsaction` in `withSentry`
lforst Dec 20, 2023
9049db6
Merge remote-tracking branch 'origin/develop' into lforst-replace-sta…
lforst Dec 22, 2023
f6fdbbb
Move to newr api
lforst Dec 22, 2023
279bf5b
Fix test
lforst Dec 22, 2023
1d2f80b
Status code
lforst Dec 22, 2023
5167c3e
Set request sdk processing metadata
lforst Dec 22, 2023
af60c56
Set request sdk processing metadata
lforst Dec 22, 2023
855af80
Merge branch 'develop' into lforst-replace-start-transaction
lforst Dec 27, 2023
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
266 changes: 116 additions & 150 deletions packages/nextjs/src/common/wrapApiHandlerWithSentry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,16 @@
import {
addTracingExtensions,
captureException,
getClient,
continueTrace,
getCurrentScope,
runWithAsyncContext,
startTransaction,
startSpanManual,
} from '@sentry/core';
import type { Transaction } from '@sentry/types';
import {
consoleSandbox,
isString,
logger,
objectify,
stripUrlQueryAndFragment,
tracingContextFromHeaders,
} from '@sentry/utils';

import { DEBUG_BUILD } from './debug-build';
import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils';

import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types';
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
import { autoEndTransactionOnResponseEnd, finishTransaction, flushQueue } from './utils/responseEnd';
import { flushQueue } from './utils/responseEnd';

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

addTracingExtensions();

// eslint-disable-next-line complexity, @typescript-eslint/no-explicit-any
const boundHandler = runWithAsyncContext(
// eslint-disable-next-line complexity
async () => {
let transaction: Transaction | undefined;
const currentScope = getCurrentScope();
const options = getClient()?.getOptions();

currentScope.setSDKProcessingMetadata({ request: req });

if (options?.instrumenter === 'sentry') {
const sentryTrace =
req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined;
const baggage = req.headers?.baggage;
const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
sentryTrace,
baggage,
);
currentScope.setPropagationContext(propagationContext);

if (DEBUG_BUILD && traceparentData) {
logger.log(`[Tracing] Continuing trace ${traceparentData.traceId}.`);
return runWithAsyncContext(async () => {
const transactionContext = continueTrace({
sentryTrace: req.headers && isString(req.headers['sentry-trace']) ? req.headers['sentry-trace'] : undefined,
baggage: req.headers?.baggage,
});

// prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler)
let reqPath = parameterizedRoute;

// If not, fake it by just replacing parameter values with their names, hoping that none of them match either
// each other or any hard-coded parts of the path
if (!reqPath) {
const url = `${req.url}`;
// pull off query string, if any
reqPath = stripUrlQueryAndFragment(url);
// Replace with placeholder
if (req.query) {
for (const [key, value] of Object.entries(req.query)) {
reqPath = reqPath.replace(`${value}`, `[${key}]`);
}

// prefer the parameterized route, if we have it (which we will if we've auto-wrapped the route handler)
let reqPath = parameterizedRoute;

// If not, fake it by just replacing parameter values with their names, hoping that none of them match either
// each other or any hard-coded parts of the path
if (!reqPath) {
const url = `${req.url}`;
// pull off query string, if any
reqPath = stripUrlQueryAndFragment(url);
// Replace with placeholder
if (req.query) {
for (const [key, value] of Object.entries(req.query)) {
reqPath = reqPath.replace(`${value}`, `[${key}]`);
}
}

const reqMethod = `${(req.method || 'GET').toUpperCase()} `;

getCurrentScope().setSDKProcessingMetadata({ request: req });

return startSpanManual(
{
...transactionContext,
name: `${reqMethod}${reqPath}`,
op: 'http.server',
origin: 'auto.http.nextjs',
metadata: {
...transactionContext.metadata,
source: 'route',
request: req,
},
},
async span => {
// eslint-disable-next-line @typescript-eslint/unbound-method
res.end = new Proxy(res.end, {
apply(target, thisArg, argArray) {
span?.setHttpStatus(res.statusCode);
span?.end();
if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
target.apply(thisArg, argArray);
} else {
// flushQueue will not reject
// eslint-disable-next-line @typescript-eslint/no-floating-promises
flushQueue().then(() => {
target.apply(thisArg, argArray);
});
}
}
}

const reqMethod = `${(req.method || 'GET').toUpperCase()} `;

transaction = startTransaction(
{
name: `${reqMethod}${reqPath}`,
op: 'http.server',
origin: 'auto.http.nextjs',
...traceparentData,
metadata: {
dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
source: 'route',
request: req,
},
},
// extra context passed to the `tracesSampler`
{ request: req },
);
currentScope.setSpan(transaction);
if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
autoEndTransactionOnResponseEnd(transaction, res);
} else {
// 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;
res.end = async function (this: unknown, ...args: unknown[]) {
if (transaction) {
finishTransaction(transaction, res);
await flushQueue();
}
});

origResEnd.apply(this, args);
};
}
}
try {
const handlerResult = await wrappingTarget.apply(thisArg, args);
if (
process.env.NODE_ENV === 'development' &&
!process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR &&
!res.finished
// TODO(v8): Remove this warning?
// This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating.
// Warning suppression on Next.JS is only necessary in that case.
) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
'[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).',
);
});
}

try {
const handlerResult = await wrappingTarget.apply(thisArg, args);

if (
process.env.NODE_ENV === 'development' &&
!process.env.SENTRY_IGNORE_API_RESOLUTION_ERROR &&
!res.finished
// This can only happen (not always) when the user is using `withSentry` manually, which we're deprecating.
// Warning suppression on Next.JS is only necessary in that case.
) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn(
`[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).`,
);
return handlerResult;
} catch (e) {
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
// store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced
// to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a
// way to prevent it from actually being reported twice.)
const objectifiedErr = objectify(e);

captureException(objectifiedErr, {
mechanism: {
type: 'instrument',
handled: false,
data: {
wrapped_handler: wrappingTarget.name,
function: 'withSentry',
},
},
});
}

return handlerResult;
} catch (e) {
// In case we have a primitive, wrap it in the equivalent wrapper class (string -> String, etc.) so that we can
// store a seen flag on it. (Because of the one-way-on-Vercel-one-way-off-of-Vercel approach we've been forced
// to take, it can happen that the same thrown object gets caught in two different ways, and flagging it is a
// way to prevent it from actually being reported twice.)
const objectifiedErr = objectify(e);

captureException(objectifiedErr, {
mechanism: {
type: 'instrument',
handled: false,
data: {
wrapped_handler: wrappingTarget.name,
function: 'withSentry',
},
},
});
// Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet
// have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that
// the transaction was error-free
res.statusCode = 500;
res.statusMessage = 'Internal Server Error';

span?.setHttpStatus(res.statusCode);
span?.end();

// Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors
// out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the
// 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() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
await flushQueue();
}

// Because we're going to finish and send the transaction before passing the error onto nextjs, it won't yet
// have had a chance to set the status to 500, so unless we do it ourselves now, we'll incorrectly report that
// the transaction was error-free
res.statusCode = 500;
res.statusMessage = 'Internal Server Error';

finishTransaction(transaction, res);

// Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors
// out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the
// 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() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
await flushQueue();
// We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it
// would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark
// the error as already having been captured.)
throw objectifiedErr;
}

// We rethrow here so that nextjs can do with the error whatever it would normally do. (Sometimes "whatever it
// would normally do" is to allow the error to bubble up to the global handlers - another reason we need to mark
// the error as already having been captured.)
throw objectifiedErr;
}
},
);

// Since API route handlers are all async, nextjs always awaits the return value (meaning it's fine for us to return
// a promise here rather than a real result, and it saves us the overhead of an `await` call.)
return boundHandler;
},
);
});
},
});
}
45 changes: 4 additions & 41 deletions packages/nextjs/test/config/withSentry.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as SentryCore from '@sentry/core';
import { addTracingExtensions } from '@sentry/core';
import type { Client, ClientOptions } from '@sentry/types';
import type { NextApiRequest, NextApiResponse } from 'next';

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

const FLUSH_DURATION = 200;

async function sleep(ms: number): Promise<void> {
await new Promise(resolve => setTimeout(resolve, ms));
}
/**
* Helper to prevent tests from ending before `flush()` has finished its work.
*
* This is necessary because, like its real-life counterpart, our mocked `res.send()` below doesn't await `res.end()
* (which becomes async when we wrap it in `withSentry` in order to give `flush()` time to run). In real life, the
* request/response cycle is held open as subsequent steps wait for `end()` to emit its `prefinished` event. Here in
* tests, without any of that other machinery, we have to hold it open ourselves.
*
* @param wrappedHandler
* @param req
* @param res
*/
async function callWrappedHandler(wrappedHandler: NextApiHandler, req: NextApiRequest, res: NextApiResponse) {
await wrappedHandler(req, res);

// we know we need to wait at least this long for `flush()` to finish
await sleep(FLUSH_DURATION);

// should be <1 second, just long enough the `flush()` call to return, the original (pre-wrapping) `res.end()` to be
// called, and the response to be marked as done
while (!res.finished) {
await sleep(100);
}
}

const startTransactionSpy = jest.spyOn(SentryCore, 'startTransaction');
const startSpanManualSpy = jest.spyOn(SentryCore, 'startSpanManual');

describe('withSentry', () => {
let req: NextApiRequest, res: NextApiResponse;
Expand Down Expand Up @@ -70,24 +39,18 @@ describe('withSentry', () => {

describe('tracing', () => {
it('starts a transaction and sets metadata when tracing is enabled', async () => {
jest.spyOn(SentryCore.Hub.prototype, 'getClient').mockReturnValueOnce({
getOptions: () => ({ tracesSampleRate: 1, instrumenter: 'sentry' }) as ClientOptions,
} as Client);

await callWrappedHandler(wrappedHandlerNoError, req, res);

expect(startTransactionSpy).toHaveBeenCalledWith(
await wrappedHandlerNoError(req, res);
expect(startSpanManualSpy).toHaveBeenCalledWith(
{
name: 'GET http://dogs.are.great',
op: 'http.server',
origin: 'auto.http.nextjs',

metadata: {
source: 'route',
request: expect.objectContaining({ url: 'http://dogs.are.great' }),
},
},
{ request: expect.objectContaining({ url: 'http://dogs.are.great' }) },
expect.any(Function),
);
});
});
Expand Down