@sentry/nextjs throws false when Sentry.flush() times out

[Vadorequest]

Important Details

How are you running Sentry?

Vercel

Description

import { logEvent } from '@/modules/core/amplitude/amplitudeServerClient';
import {
  AMPLITUDE_API_ENDPOINTS,
  AMPLITUDE_EVENTS,
} from '@/modules/core/amplitude/events';
import { filterExternalAbsoluteUrl } from '@/modules/core/js/url';
import { createLogger } from '@/modules/core/logging/logger';
import { FLUSH_TIMEOUT } from '@/modules/core/sentry/config';
import { configureReq } from '@/modules/core/sentry/server';
import * as Sentry from '@sentry/nextjs';
import appendQueryParameter from 'append-query';
import {
  NextApiRequest,
  NextApiResponse,
} from 'next';

const fileLabel = 'api/preview';
const logger = createLogger({
  fileLabel,
});

/**
 * Key to use in order to force disable "auto preview mode".
 *
 * If a page is loaded with noAutoPreviewMode=true in query parameter, then it won't try to enable the Preview mode, even if it's disabled.
 *
 * @example ?noAutoPreviewMode=true
 */
export const NO_AUTO_PREVIEW_MODE_KEY = 'noAutoPreviewMode';

type EndpointRequestQuery = {
  /**
   * Whether to start/stop the Preview Mode.
   *
   * @example ?stop=true Will stop the preview mode.
   * @example ?stop=false Will start the preview mode.
   * @default ?stop=false
   */
  stop: string;

  /**
   * Url to redirect to once the preview mode has been started/stopped.
   *
   * @example ?redirectTo=/en
   * @example ?redirectTo=/fr/solutions
   * @default ?redirectTo=/
   */
  redirectTo: string;
}

type EndpointRequest = NextApiRequest & {
  query: EndpointRequestQuery;
};

/**
 * Preview Mode API.
 *
 * Enables and disables preview mode.
 *
 * The official example uses a security token to enable the preview mode, we don't.
 * This is a choice, as we don't need/want to protect our preview mode.
 * Protecting the preview mode makes most sense when this mode can be used in production, so that you can preview content served by Next.js from a CMS/tool of your choice.
 * Thus, it's strongly related to how you're planning on using it, and we decided to keep it simpler, by not using any kind of security.
 *
 * @param req
 * @param res
 * @method GET
 *
 * @see https://nextjs.org/docs/advanced-features/preview-mode#step-1-create-and-access-a-preview-api-route
 * @see https://nextjs.org/docs/advanced-features/preview-mode#clear-the-preview-mode-cookies
 */
export const preview = async (req: EndpointRequest, res: NextApiResponse): Promise<void> => {
  try {
    configureReq(req, { fileLabel });

    await logEvent(AMPLITUDE_EVENTS.API_INVOKED, null, {
      apiEndpoint: AMPLITUDE_API_ENDPOINTS.PREVIEW,
    });

    const {
      stop = 'false',
      redirectTo = '/',
    }: EndpointRequestQuery = req.query;
    // Add NO_AUTO_PREVIEW_MODE_KEY parameter to query, to avoid running into infinite loops if the Preview mode couldn't start
    // Useful when the cookie created by Next.js cannot be written (Incognito mode)
    const safeRedirectUrl = appendQueryParameter(filterExternalAbsoluteUrl(redirectTo as string), `${NO_AUTO_PREVIEW_MODE_KEY}=true`);

    // XXX We don't want to enable preview mode for the production stage, it's only allowed for non-production stages
    //  It's allowed during development for testing purpose
    //  It's allowed during staging because this stage is being used as a "preview environment"
    if (process.env.NEXT_PUBLIC_APP_STAGE !== 'production') {
      if (stop === 'true') {
        res.clearPreviewData();

        logger.info('Preview mode stopped');
      } else {
        res.setPreviewData({});

        logger.info('Preview mode enabled');
      }
    } else {
      logger.error('Preview mode is not allowed in production');
      Sentry.captureMessage('Preview mode is not allowed in production', Sentry.Severity.Warning);
    }

    // It's necessary to flush all events because Vercel runs on AWS Lambda, see https://vercel.com/docs/platform/limits#streaming-responses
    await Sentry.flush(FLUSH_TIMEOUT);


    res.writeHead(307, { Location: safeRedirectUrl });
    res.end();
  } catch (e) {
    Sentry.captureException(e);
    logger.error(e);

    // It's necessary to flush all events because Vercel runs on AWS Lambda, see https://vercel.com/docs/platform/limits#streaming-responses
    await Sentry.flush(FLUSH_TIMEOUT);

    res.json({
      error: true,
      message: process.env.NEXT_PUBLIC_APP_STAGE === 'production' ? undefined : e.message,
    });
  }
};

export default preview;

When await Sentry.flush(2000); is executed, if it times out, Sentry throws an exception. The exception value is false.

From the TS doc (sdk.d.ts):

/**
 * A promise that resolves when all current events have been sent.
 * If you provide a timeout and the queue takes longer to drain the promise returns false.
 *
 * @param timeout Maximum time in ms the client should wait.
 */
export declare function flush(timeout?: number): Promise<boolean>;

It should return false, not throw false.
This is broken, it shouldn't throw an exception when timing out, as it breaks the code.
It's sort of a shame that the tool meant to help devs make their app more robust actually breaks them.

When encapsulating with try/catch it works fine.

Using "@sentry/nextjs": "6.6.0". Updating to 6.7.2 to see if it solves the issue.

Steps to Reproduce

1. Call await Sentry.flush(1); in a Next.js API endpoint and deploy to Vercel.

See PR at UnlyEd/next-right-now#370 (migrating from @sentry/node to @sentry/next.js)

What you expected to happen

It shouldn't throw an exception.

[Vadorequest]

The reason behind this bug was because I wasn't calling Sentry.init() priori to call Sentry.flush().

The error message (true, without any kind of explanation) was misleading and didn't help figure out the origin of the problem.

[natterstefan]

Hi @Vadorequest,

I have a similar issue at the moment (natterstefan/next-with-sentry#2).

The reason behind this bug was because I wasn't calling Sentry.init() priori to call Sentry.flush().

Where did you add Sentry.init() now? Isn't doing what I do here enough?

Thanks for your help!

[Vadorequest]

Where did you add Sentry.init() now? Isn't doing what I do here enough?

What you're doing seems legit. Sentry.init() is called in the properties files, and you use the withSentry. I don't see anything wrong there.

Note that even though I marked this issue with @sentry/nextjs, I actually fixed it using @sentry/node (I was working with both libs because I was trying to migrate from node to nextjs. But eventually I'm still stuck with @sentry/nextjs because of #3750, and the #3746 (comment) comment explained how I fixed it using @sentry/node.

[lobsterkatie]

Hi, @Vadorequest and @natterstefan. This should be fixed by #3846. Once that's released, can you please remove your workaround and let us know if you're still having problems? Thanks!

[natterstefan]

Hi, @Vadorequest and @natterstefan. This should be fixed by #3846. Once that's released, can you please remove your workaround and let us know if you're still having problems? Thanks!

FTR: thanks @lobsterkatie, I can confirm that it works with "@sentry/nextjs": "6.13.2" both with next@10 and next@11 on Vercel.