Skip to content
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

fix(nextjs): Don't accidentally trigger static generation bailout #9749

Merged
merged 3 commits into from
Dec 5, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { NextResponse } from 'next/server';

export async function GET() {
return NextResponse.json({ result: 'static response' });
}

// This export makes it so that this route is always dynamically rendered (i.e Sentry will trace)
export const revalidate = 0;

// This export makes it so that this route will throw an error if the Request object is accessed in some way.
export const dynamic = 'error';
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,10 @@ test.describe('Edge runtime', () => {
expect(routehandlerError.contexts?.runtime?.name).toBe('vercel-edge');
});
});

test('should not crash route handlers that are configured with `export const dynamic = "error"`', async ({
request,
}) => {
const response = await request.get('/route-handlers/static');
expect(await response.json()).toStrictEqual({ result: 'static response' });
});
9 changes: 3 additions & 6 deletions packages/nextjs/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,19 @@ export type ServerComponentContext = {
};

export interface RouteHandlerContext {
// TODO(v8): Remove
/**
* @deprecated The SDK will automatically pick up the method from the incoming Request object instead.
*/
method: 'GET' | 'POST' | 'PUT' | 'PATCH' | 'DELETE' | 'HEAD' | 'OPTIONS';
parameterizedRoute: string;
// TODO(v8): Remove
/**
* @deprecated The SDK will automatically pick up the `sentry-trace` header from the incoming Request object instead.
* @deprecated pass a complete `Headers` object with the `headers` field instead.
*/
sentryTraceHeader?: string;
// TODO(v8): Remove
/**
* @deprecated The SDK will automatically pick up the `baggage` header from the incoming Request object instead.
* @deprecated pass a complete `Headers` object with the `headers` field instead.
*/
baggageHeader?: string;
headers?: WebFetchHeaders;
}

export type VercelCronsConfig = { path?: string; schedule?: string }[] | undefined;
Expand Down
21 changes: 8 additions & 13 deletions packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { addTracingExtensions, captureException, flush, getCurrentHub, runWithAsyncContext, trace } from '@sentry/core';
import { tracingContextFromHeaders, winterCGRequestToRequestData } from '@sentry/utils';
import { tracingContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils';

import { isRedirectNavigationError } from './nextNavigationErrorUtils';
import type { RouteHandlerContext } from './types';
Expand All @@ -15,23 +15,16 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
): (...args: Parameters<F>) => ReturnType<F> extends Promise<unknown> ? ReturnType<F> : Promise<ReturnType<F>> {
addTracingExtensions();
// eslint-disable-next-line deprecation/deprecation
const { method, parameterizedRoute, baggageHeader, sentryTraceHeader } = context;
const { method, parameterizedRoute, baggageHeader, sentryTraceHeader, headers } = context;
return new Proxy(routeHandler, {
apply: (originalFunction, thisArg, args) => {
return runWithAsyncContext(async () => {
const hub = getCurrentHub();
const currentScope = hub.getScope();

let req: Request | undefined;
let reqMethod: string | undefined;
if (args[0] instanceof Request) {
req = args[0];
reqMethod = req.method;
}

const { traceparentData, dynamicSamplingContext, propagationContext } = tracingContextFromHeaders(
sentryTraceHeader,
baggageHeader,
sentryTraceHeader ?? headers?.get('sentry-trace') ?? undefined,
baggageHeader ?? headers?.get('baggage'),
);
currentScope.setPropagationContext(propagationContext);

Expand All @@ -40,11 +33,13 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
res = await trace(
{
op: 'http.server',
name: `${reqMethod ?? method} ${parameterizedRoute}`,
name: `${method} ${parameterizedRoute}`,
status: 'ok',
...traceparentData,
metadata: {
request: req ? winterCGRequestToRequestData(req) : undefined,
request: {
headers: headers ? winterCGHeadersToDict(headers) : undefined,
},
source: 'route',
dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as Sentry from '@sentry/nextjs';
import type { WebFetchHeaders } from '@sentry/types';
// @ts-expect-error Because we cannot be sure if the RequestAsyncStorage module exists (it is not part of the Next.js public
// API) we use a shim if it doesn't exist. The logic for this is in the wrapping loader.
import { requestAsyncStorage } from '__SENTRY_NEXTJS_REQUEST_ASYNC_STORAGE_SHIM__';
Expand Down Expand Up @@ -34,12 +35,14 @@ function wrapHandler<T>(handler: T, method: 'GET' | 'POST' | 'PUT' | 'PATCH' | '
apply: (originalFunction, thisArg, args) => {
let sentryTraceHeader: string | undefined | null = undefined;
let baggageHeader: string | undefined | null = undefined;
let headers: WebFetchHeaders | undefined = undefined;

// We try-catch here just in case the API around `requestAsyncStorage` changes unexpectedly since it is not public API
try {
const requestAsyncStore = requestAsyncStorage.getStore();
sentryTraceHeader = requestAsyncStore?.headers.get('sentry-trace') ?? undefined;
baggageHeader = requestAsyncStore?.headers.get('baggage') ?? undefined;
headers = requestAsyncStore?.headers;
} catch (e) {
/** empty */
}
Expand All @@ -50,6 +53,7 @@ function wrapHandler<T>(handler: T, method: 'GET' | 'POST' | 'PUT' | 'PATCH' | '
parameterizedRoute: '__ROUTE__',
sentryTraceHeader,
baggageHeader,
headers,
}).apply(thisArg, args);
},
});
Expand Down