Skip to content

fix(nextjs): Use domains to prevent scope bleed #3788

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
Jul 14, 2021
Merged
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
121 changes: 67 additions & 54 deletions packages/nextjs/src/utils/handlers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { captureException, flush, getCurrentHub, Handlers, startTransaction, withScope } from '@sentry/node';
import { extractTraceparentData, getActiveTransaction, hasTracingEnabled } from '@sentry/tracing';
import { addExceptionMechanism, isString, logger, stripUrlQueryAndFragment } from '@sentry/utils';
import * as domain from 'domain';
import { NextApiHandler } from 'next';

import { addRequestDataToEvent, NextRequest } from './instrumentServer';
Expand All @@ -14,70 +15,82 @@ type WrappedNextApiHandler = NextApiHandler;
export const withSentry = (handler: NextApiHandler): WrappedNextApiHandler => {
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
return async (req, res) => {
try {
const currentScope = getCurrentHub().getScope();
// wrap everything in a domain in order to prevent scope bleed between requests
const local = domain.create();
local.add(req);
local.add(res);

if (currentScope) {
currentScope.addEventProcessor(event => addRequestDataToEvent(event, req as NextRequest));
// `local.bind` causes everything to run inside a domain, just like `local.run` does, but it also lets the callback
// return a value. In our case, all any of the codepaths return is a promise of `void`, but nextjs still counts on
// getting that before it will finish the response.
const boundHandler = local.bind(async () => {
try {
const currentScope = getCurrentHub().getScope();

if (hasTracingEnabled()) {
// If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision)
let traceparentData;
if (req.headers && isString(req.headers['sentry-trace'])) {
traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string);
logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
}
if (currentScope) {
currentScope.addEventProcessor(event => addRequestDataToEvent(event, req as NextRequest));

const url = `${req.url}`;
// pull off query string, if any
let reqPath = stripUrlQueryAndFragment(url);
// Replace with placeholder
if (req.query) {
// TODO get this from next if possible, to avoid accidentally replacing non-dynamic parts of the path if
// they match dynamic parts
for (const [key, value] of Object.entries(req.query)) {
reqPath = reqPath.replace(`${value}`, `[${key}]`);
if (hasTracingEnabled()) {
// If there is a trace header set, extract the data from it (parentSpanId, traceId, and sampling decision)
let traceparentData;
if (req.headers && isString(req.headers['sentry-trace'])) {
traceparentData = extractTraceparentData(req.headers['sentry-trace'] as string);
logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
}
}
const reqMethod = `${(req.method || 'GET').toUpperCase()} `;

const transaction = startTransaction(
{
name: `${reqMethod}${reqPath}`,
op: 'http.server',
...traceparentData,
},
// extra context passed to the `tracesSampler`
{ request: req },
);
currentScope.setSpan(transaction);
const url = `${req.url}`;
// pull off query string, if any
let reqPath = stripUrlQueryAndFragment(url);
// Replace with placeholder
if (req.query) {
// TODO get this from next if possible, to avoid accidentally replacing non-dynamic parts of the path if
// they match dynamic parts
for (const [key, value] of Object.entries(req.query)) {
reqPath = reqPath.replace(`${value}`, `[${key}]`);
}
}
const reqMethod = `${(req.method || 'GET').toUpperCase()} `;

const transaction = startTransaction(
{
name: `${reqMethod}${reqPath}`,
op: 'http.server',
...traceparentData,
},
// extra context passed to the `tracesSampler`
{ request: req },
);
currentScope.setSpan(transaction);
}
}
}

return await handler(req, res); // Call original handler
} catch (e) {
withScope(scope => {
scope.addEventProcessor(event => {
addExceptionMechanism(event, {
handled: false,
return await handler(req, res); // Call original handler
} catch (e) {
withScope(scope => {
scope.addEventProcessor(event => {
addExceptionMechanism(event, {
handled: false,
});
return parseRequest(event, req);
});
return parseRequest(event, req);
captureException(e);
});
captureException(e);
});
throw e;
} finally {
const transaction = getActiveTransaction();
if (transaction) {
transaction.setHttpStatus(res.statusCode);
throw e;
} finally {
const transaction = getActiveTransaction();
if (transaction) {
transaction.setHttpStatus(res.statusCode);

transaction.finish();
}
try {
await flush(2000);
} catch (e) {
// no-empty
transaction.finish();
}
try {
await flush(2000);
} catch (e) {
// no-empty
}
}
}
});

return await boundHandler();
};
};