Skip to content

Commit

Permalink
fix(nextjs): Unwrap req and res if necessary when instrumenting s…
Browse files Browse the repository at this point in the history
…erver (#4467)

In vercel/next.js#32999 (released as part of 12.0.9), Next.js made an internal change, so that it now wraps the raw `http.IncomingMessage` and `http.ServerResponse` objects which get passed to the main server request handler in `NodeNextRequest` and `NodeNextResponse` objects, respectively. We therefore need to unwrap them before we can use them.

This does that unwrapping (if necessary; older versions of nextjs still don't need it) and fixes the relevant types.

Fixes #4463
Fixes vercel/next.js#33726
  • Loading branch information
lobsterkatie committed Jan 28, 2022
1 parent 8cbcff2 commit f794dbd
Showing 1 changed file with 38 additions and 15 deletions.
53 changes: 38 additions & 15 deletions packages/nextjs/src/utils/instrumentServer.ts
Expand Up @@ -34,18 +34,31 @@ interface Server {
publicDir: string;
}

export interface NextRequest extends http.IncomingMessage {
export type NextRequest = (
| http.IncomingMessage // in nextjs versions < 12.0.9, `NextRequest` extends `http.IncomingMessage`
| {
_req: http.IncomingMessage; // in nextjs versions >= 12.0.9, `NextRequest` wraps `http.IncomingMessage`
}
) & {
cookies: Record<string, string>;
url: string;
query: { [key: string]: string };
headers: { [key: string]: string };
body: string | { [key: string]: unknown };
}
type NextResponse = http.ServerResponse;
method: string;
};

type NextResponse =
// in nextjs versions < 12.0.9, `NextResponse` extends `http.ServerResponse`
| http.ServerResponse
// in nextjs versions >= 12.0.9, `NextResponse` wraps `http.ServerResponse`
| {
_res: http.ServerResponse;
};

// the methods we'll wrap
type HandlerGetter = () => Promise<ReqHandler>;
type ReqHandler = (req: NextRequest, res: NextResponse, parsedUrl?: url.UrlWithParsedQuery) => Promise<void>;
type ReqHandler = (nextReq: NextRequest, nextRes: NextResponse, parsedUrl?: url.UrlWithParsedQuery) => Promise<void>;
type ErrorLogger = (err: Error) => void;
type ApiPageEnsurer = (path: string) => Promise<void>;
type PageComponentFinder = (
Expand Down Expand Up @@ -205,10 +218,16 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
// add transaction start and stop to the normal request handling
const wrappedReqHandler = async function (
this: Server,
req: NextRequest,
res: NextResponse,
nextReq: NextRequest,
nextRes: NextResponse,
parsedUrl?: url.UrlWithParsedQuery,
): Promise<void> {
// Starting with version 12.0.9, nextjs wraps the incoming request in a `NodeNextRequest` object and the outgoing
// response in a `NodeNextResponse` object before passing them to the handler. (This is necessary here but not in
// `withSentry` because by the time nextjs passes them to an API handler, it's unwrapped them again.)
const req = '_req' in nextReq ? nextReq._req : nextReq;
const res = '_res' in nextRes ? nextRes._res : nextRes;

// wrap everything in a domain in order to prevent scope bleed between requests
const local = domain.create();
local.add(req);
Expand All @@ -220,23 +239,23 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
const currentScope = getCurrentHub().getScope();

if (currentScope) {
currentScope.addEventProcessor(event => parseRequest(event, req));
currentScope.addEventProcessor(event => parseRequest(event, nextReq));

// We only want to record page and API requests
if (hasTracingEnabled() && shouldTraceRequest(req.url, publicDirFiles)) {
if (hasTracingEnabled() && shouldTraceRequest(nextReq.url, publicDirFiles)) {
// 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);
if (nextReq.headers && isString(nextReq.headers['sentry-trace'])) {
traceparentData = extractTraceparentData(nextReq.headers['sentry-trace'] as string);
logger.log(`[Tracing] Continuing trace ${traceparentData?.traceId}.`);
}

// pull off query string, if any
const reqPath = stripUrlQueryAndFragment(req.url);
const reqPath = stripUrlQueryAndFragment(nextReq.url);

// requests for pages will only ever be GET requests, so don't bother to include the method in the transaction
// name; requests to API routes could be GET, POST, PUT, etc, so do include it there
const namePrefix = req.url.startsWith('/api') ? `${(req.method || 'GET').toUpperCase()} ` : '';
const namePrefix = nextReq.url.startsWith('/api') ? `${nextReq.method.toUpperCase()} ` : '';

const transaction = startTransaction(
{
Expand All @@ -245,8 +264,12 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
metadata: { requestPath: reqPath },
...traceparentData,
},
// extra context passed to the `tracesSampler`
{ request: req },
// Extra context passed to the `tracesSampler` (Note: We're combining `nextReq` and `req` this way in order
// to not break people's `tracesSampler` functions, even though the format of `nextReq` has changed (see
// note above re: nextjs 12.0.9). If `nextReq === req` (pre 12.0.9), then spreading `req` is a no-op - we're
// just spreading the same stuff twice. If `nextReq` contains `req` (12.0.9 and later), then spreading `req`
// mimics the old format by flattening the data.)
{ request: { ...nextReq, ...req } },
);

currentScope.setSpan(transaction);
Expand All @@ -270,7 +293,7 @@ function makeWrappedReqHandler(origReqHandler: ReqHandler): WrappedReqHandler {
}
}

return origReqHandler.call(this, req, res, parsedUrl);
return origReqHandler.call(this, nextReq, nextRes, parsedUrl);
});
};

Expand Down

0 comments on commit f794dbd

Please sign in to comment.