Skip to content

Commit

Permalink
fix(nextjs): Catch rejecting flushes (#9811)
Browse files Browse the repository at this point in the history
  • Loading branch information
lforst committed Dec 13, 2023
1 parent 7303cda commit be32c1e
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 23 deletions.
9 changes: 2 additions & 7 deletions packages/nextjs/src/common/_error.ts
@@ -1,5 +1,6 @@
import { captureException, getClient, withScope } from '@sentry/core';
import type { NextPageContext } from 'next';
import { flushQueue } from './utils/responseEnd';

type ContextOrProps = {
req?: NextPageContext['req'];
Expand All @@ -9,12 +10,6 @@ type ContextOrProps = {
statusCode?: number;
};

/** Platform-agnostic version of `flush` */
function flush(timeout?: number): PromiseLike<boolean> {
const client = getClient();
return client ? client.flush(timeout) : Promise.resolve(false);
}

/**
* Capture the exception passed by nextjs to the `_error` page, adding context data as appropriate.
*
Expand Down Expand Up @@ -60,5 +55,5 @@ export async function captureUnderscoreErrorException(contextOrProps: ContextOrP

// In case this is being run as part of a serverless function (as is the case with the server half of nextjs apps
// deployed to vercel), make sure the error gets sent to Sentry before the lambda exits.
await flush(2000);
await flushQueue();
}
3 changes: 2 additions & 1 deletion packages/nextjs/src/common/utils/edgeWrapperUtils.ts
Expand Up @@ -10,6 +10,7 @@ import {

import type { EdgeRouteHandler } from '../../edge/types';
import { DEBUG_BUILD } from '../debug-build';
import { flushQueue } from './responseEnd';

/**
* Wraps a function on the edge runtime with error and performance monitoring.
Expand Down Expand Up @@ -97,7 +98,7 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
} finally {
span?.finish();
currentScope?.setSpan(prevSpan);
await flush(2000);
await flushQueue();
}
};
}
5 changes: 3 additions & 2 deletions packages/nextjs/src/common/withServerActionInstrumentation.ts
Expand Up @@ -3,6 +3,7 @@ import { logger, tracingContextFromHeaders } from '@sentry/utils';

import { DEBUG_BUILD } from './debug-build';
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
import { flushQueue } from './utils/responseEnd';

interface Options {
formData?: FormData;
Expand Down Expand Up @@ -118,11 +119,11 @@ async function withServerActionInstrumentationImplementation<A extends (...args:
} finally {
if (!platformSupportsStreaming()) {
// Lambdas require manual flushing to prevent execution freeze before the event is sent
await flush(1000);
await flushQueue();
}

if (process.env.NEXT_RUNTIME === 'edge') {
void flush();
void flushQueue();
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts
Expand Up @@ -4,6 +4,7 @@ import { tracingContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'
import { isRedirectNavigationError } from './nextNavigationErrorUtils';
import type { RouteHandlerContext } from './types';
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
import { flushQueue } from './utils/responseEnd';

/**
* Wraps a Next.js route handler with performance and error instrumentation.
Expand Down Expand Up @@ -70,7 +71,7 @@ export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
if (!platformSupportsStreaming() || process.env.NEXT_RUNTIME === 'edge') {
// 1. Edge tranpsort requires manual flushing
// 2. Lambdas require manual flushing to prevent execution freeze before the event is sent
await flush(1000);
await flushQueue();
}
}

Expand Down
27 changes: 15 additions & 12 deletions packages/nextjs/src/common/wrapServerComponentWithSentry.ts
@@ -1,7 +1,6 @@
import {
addTracingExtensions,
captureException,
flush,
getCurrentHub,
runWithAsyncContext,
startTransaction,
Expand All @@ -10,6 +9,7 @@ import { tracingContextFromHeaders, winterCGHeadersToDict } from '@sentry/utils'

import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils';
import type { ServerComponentContext } from '../common/types';
import { flushQueue } from './utils/responseEnd';

/**
* Wraps an `app` directory server component with Sentry error instrumentation.
Expand Down Expand Up @@ -85,28 +85,31 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
maybePromiseResult = originalFunction.apply(thisArg, args);
} catch (e) {
handleErrorCase(e);
void flush();
void flushQueue();
throw e;
}

if (typeof maybePromiseResult === 'object' && maybePromiseResult !== null && 'then' in maybePromiseResult) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
Promise.resolve(maybePromiseResult).then(
() => {
transaction.finish();
},
e => {
handleErrorCase(e);
},
);
void flush();
Promise.resolve(maybePromiseResult)
.then(
() => {
transaction.finish();
},
e => {
handleErrorCase(e);
},
)
.finally(() => {
void flushQueue();
});

// It is very important that we return the original promise here, because Next.js attaches various properties
// to that promise and will throw if they are not on the returned value.
return maybePromiseResult;
} else {
transaction.finish();
void flush();
void flushQueue();
return maybePromiseResult;
}
});
Expand Down

0 comments on commit be32c1e

Please sign in to comment.