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

Asynchronous errors aren't being reported in Next.js data-fetching methods on Vercel #7602

Closed
3 tasks done
masonmcelvain opened this issue Mar 23, 2023 · 18 comments
Closed
3 tasks done
Assignees
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug

Comments

@masonmcelvain
Copy link

masonmcelvain commented Mar 23, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using? If you use the CDN bundles, please specify the exact bundle (e.g. bundle.tracing.min.js) in your SDK setup.

@sentry/nextjs

SDK Version

7.44.2

Framework Version

12.2.3

Link to Sentry event

https://ifixit.sentry.io/issues/4031066228/?project=6475315&query=is%3Aunresolved&referrer=issue-stream

SDK Setup

Our source is public here. Debugging branch here, on the latest Sentry version.

Sentry.init({
   debug: true,
   dsn: SENTRY_DSN,
   sampleRate: 1.0,
   normalizeDepth: 5,
   initialScope: {
      tags: {
         'next.runtime': 'server',
      },
   },
   beforeSend(event, hint) {
      const ex = hint.originalException;
      if (ex instanceof Error) {
         // Happens when receiving a bad url that fails to decode
         if (ex.message.match(/URI malformed/)) {
            return null;
         }
      }
      return event;
   },
});

Steps to Reproduce

Similar to #6117 (which fixed this problem for synchronous errors), but reject a promise or throw an error in an async callback. When the project is run locally, the error is reported to Sentry. When deployed to Vercel, the error never appears in Sentry.

import { GetServerSideProps } from 'next';

const MyComponent = () => {
   return <h1>Hello World!</h1>;
};

export const getServerSideProps: GetServerSideProps = async (context) => {
   if (context.query.myParam === 'two') {
      // only throw conditionally so that this page actually builds
      Promise.reject(new Error("We don't like page two!"));
   }

   return { props: {} };
};

export default MyComponent;

Expected Result

An error like this one gets reported to Sentry.

Actual Result

No error is reported to Sentry. Here are the Vercel logs, if it's helpful:

Unhandled Promise Rejection 	{"errorType":"Runtime.UnhandledPromiseRejection","errorMessage":"Error: We don't like page two","reason":{"errorType":"Error","errorMessage":"We don't like page two","stack":["Error: We don't like page two","    at getServerSideProps (/var/task/frontend/.next/server/pages/myPage.js:27:24)","    at Object.renderToHTML (/var/task/node_modules/.pnpm/[email protected]_t7ss3ubh4wigfvyfclbvqff3gm/node_modules/next/dist/server/render.js:573:26)","    at processTicksAndRejections (node:internal/process/task_queues:96:5)","    at async doRender (/var/task/node_modules/.pnpm/[email protected]_t7ss3ubh4wigfvyfclbvqff3gm/node_modules/next/dist/server/base-server.js:915:38)","    at async cacheEntry.responseCache.get.isManualRevalidate.isManualRevalidate (/var/task/node_modules/.pnpm/[email protected]_t7ss3ubh4wigfvyfclbvqff3gm/node_modules/next/dist/server/base-server.js:1020:28)","    at async /var/task/node_modules/.pnpm/[email protected]_t7ss3ubh4wigfvyfclbvqff3gm/node_modules/next/dist/server/response-cache.js:69:36"]},"promise":{},"stack":["Runtime.UnhandledPromiseRejection: Error: We don't like page two","    at process.<anonymous> (file:///var/runtime/index.mjs:1188:17)","    at process.emit (node:events:525:35)","    at process.emit (node:domain:489:12)","    at emit (node:internal/process/promises:140:20)","    at processPromiseRejections (node:internal/process/promises:274:27)","    at processTicksAndRejections (node:internal/process/task_queues:97:32)"]}
Unknown application error occurred
Runtime.Unknown

CC @lforst

masonmcelvain added a commit to iFixit/react-commerce that referenced this issue Mar 24, 2023
Catches and reports `getServerSideProps` errors to sentry explicitly.

This is an attempt to workaround a potential bug in Sentry: getsentry/sentry-javascript#7602
@AbhiPrasad AbhiPrasad added the Package: nextjs Issues related to the Sentry Nextjs SDK label Mar 27, 2023
@lforst
Copy link
Member

lforst commented Mar 27, 2023

Hi, you set autoInstrumentServerFunctions: false in your next.config.js this will disable auto-instrumentation for data fetchers. If you want to have it instrumented with that option set to false you need to wrap it in wrapGetServerSidePropsWithSentry.

More info: https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#opt-in-to-auto-instrumentation-on-specific-routes

@masonmcelvain
Copy link
Author

I overlooked that config option, thank you for replying and pointing it out.

I tested dropping autoInstrumentServerFunctions: false but am still seeing the same results, where no error appears in Sentry despite an async error being thrown in getServerSideProps. As before, the error is reported when run locally, but does not get reported when deployed to Vercel.

I also tested using wrapGetServerSidePropsWithSentry without dropping the config option, and experienced the same results.

@lforst
Copy link
Member

lforst commented Apr 5, 2023

@masonmcelvain Hi, thanks for sharing your thoughts and experiments. I don't know if I can currently give a solution for this. It seems like Vercel is terminating the lambda before we're able to send the event out.

@masonmcelvain
Copy link
Author

I don't know if I can currently give a solution for this.

My team and I haven't found a workaround either, but we'll share if we do.

It seems like Vercel is terminating the lambda before we're able to send the event out.

That's my hunch too, thanks for your take on it.

@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@masonmcelvain
Copy link
Author

Is it possible to tie the new Sentry.runWithAsyncContext feature (#4071 (comment)) into the autoInstrumentServerFunctions feature?

@lforst
Copy link
Member

lforst commented Apr 27, 2023

@masonmcelvain Generally it is already using the new mechanism under the hood. I don't think this will resolve this issue though.

@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@masonmcelvain
Copy link
Author

This is still happening, so I don't think it should be closed.

@github-actions
Copy link
Contributor

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@masonmcelvain
Copy link
Author

Bump

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@AnasSafi
Copy link

The issue here seems to be related to the lifecycle of Next.js 13. I managed to solve the problem by explicitly calling the Sentry.init method before invoking Sentry.captureException(error). Here's an example of how I achieved this:


import * as Sentry from "@sentry/nextjs";

try {
    // Your code here...
} catch (error) {
    // Initialize Sentry before sending the error
    Sentry.init({
        dsn: process.env.SENTRY_DSN,
        // Add other Sentry configuration options as needed
    });

    // Capture and send the error to Sentry
    Sentry.captureException(error);

    // Ensure that the events have been sent to Sentry before the function ends
    await Sentry.flush(2000);
}

@timfish
Copy link
Collaborator

timfish commented Oct 20, 2023

Looking at the original code above:

export const getServerSideProps: GetServerSideProps = async (context) => {
   if (context.query.myParam === 'two') {
      // only throw conditionally so that this page actually builds
      Promise.reject(new Error("We don't like page two!"));
   }

   return { props: {} };
};

I think the issue here is that you're not returning the promise created by Promise.reject. You're essentially creating a floating promise that's not being awaited on which is causing the Unhandled Promise Rejection.

The no-floating-promises ESLint rule is good for highlighting these.

@nandotess
Copy link

nandotess commented Oct 30, 2023

The issue here seems to be related to the lifecycle of Next.js 13. I managed to solve the problem by explicitly calling the Sentry.init method before invoking Sentry.captureException(error). Here's an example of how I achieved this:


import * as Sentry from "@sentry/nextjs";

try {
    // Your code here...
} catch (error) {
    // Initialize Sentry before sending the error
    Sentry.init({
        dsn: process.env.SENTRY_DSN,
        // Add other Sentry configuration options as needed
    });

    // Capture and send the error to Sentry
    Sentry.captureException(error);

    // Ensure that the events have been sent to Sentry before the function ends
    await Sentry.flush(2000);
}

This was the only solution that worked for me.

I have disabled the Sentry instrumentation and started to initialize it by myself.

next.config.js

// ...
// https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/
const sentryOptions = {
  // ...
  // Disable the automatic instrumentation of API route handlers and server-side data fetching functions
  autoInstrumentServerFunctions: false,
  // ...
};
// ...

API example

// ...

/**
 * API configuration for this endpoint.
 */
export const config = {
  runtime: 'edge',
};

/**
 * API endpoint.
 */
export default async function handler(req: NextRequest) {
  try {
    // ...

    // Return success response
  } catch (error) {
    // Sentry.init...

    // Sentry.withScope...
    // ...
    // Sentry.captureException...

    // Return error response
  }
}

Thanks @AnasSafi.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Oct 30, 2023
@masonmcelvain
Copy link
Author

I think the issue here is that you're not returning the promise created by Promise.reject

@timfish that's a great point, I agree that floating promises should be avoided. I confirmed that using await causes an error to be reported to Sentry ✅

If I remember correctly, I used a floating promise to demonstrate the failure method in a simple way. My original error is described in iFixit/react-commerce#1491 (specifically the following line). I think a promise was rejected in third party code from Algolia's Instantsearch library.

2023-03-21T09:31:22.520Z	30b245bf-3743-46ee-85fa-8e1369d37e5f	ERROR	Unhandled Promise Rejection 	{"errorType":"Runtime.UnhandledPromiseRejection","errorMessage":"ApiError: Value too small for \"page\" parameter, expected integer between 0 and 9223372036854775807","trace":["Runtime.UnhandledPromiseRejection: ApiError: Value too small for \"page\" parameter, expected integer between 0 and 9223372036854775807","    at process.<anonymous> (file:///var/runtime/index.mjs:1188:17)","    at process.emit (node:events:525:35)","    at process.emit (node:domain:489:12)","    at emit (node:internal/process/promises:140:20)","    at processPromiseRejections (node:internal/process/promises:274:27)","    at processTicksAndRejections (node:internal/process/task_queues:97:32)"]}

I don't want to wrap all usages of third party code in try/catch blocks, so I think this is still outstanding as long as the pages router is supported by Next.js. If we should file an issue on Vercel, I'd be happy to do so.

@lforst
Copy link
Member

lforst commented Nov 3, 2023

@masonmcelvain Yes, bugging the Next.js maintainers about a proper on-error hook would be amazing. Afaik they're looking into telemetry anyhow so this might even align with their objectives!

@lforst lforst closed this as not planned Won't fix, can't repro, duplicate, stale Nov 3, 2023
@cyborgEneki
Copy link

cyborgEneki commented Nov 7, 2024

"next": "13.0.6",
"@sentry/nextjs": "^7.105.0",

Disclaimer: not specific to Vercel or GetServerSideProps but a similar issue of unlogged async errors on api files. I think it will be helpful to someone else.

What worked for me was:

  1. Initializing Sentry inside the API scope but outside the handler
  2. Calling captureException and flush functions in the catch block, then
  3. Using the wrapApiHandlerWithSentry function
    as follows:
// pages/api/download-apk

import * as Sentry from "@sentry/nextjs";
import { wrapApiHandlerWithSentry } from "@sentry/nextjs";

Sentry.init({
  dsn: process.env.SENTRY_DSN,
   // Add other Sentry configuration options as needed
});

async function handler(req, res) {
  try {
    someFunctionThatMightFail();
  }  catch (error) {
      console.error('Error serving the APK or sending email:', error);

      // Capture error with Sentry
      Sentry.captureException(error);

      await Sentry.flush(2000);
      
      res.status(500).json({ message: 'Internal server error' });
    }
}

export default wrapApiHandlerWithSentry(handler, "/api/download-apk");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: nextjs Issues related to the Sentry Nextjs SDK Type: Bug
Projects
Archived in project
Development

No branches or pull requests

7 participants