sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

Errors in child server components aren't tracked

Open qnten opened this issue 1 year ago • 3 comments

Is there an existing issue for this?

  • [X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
  • [X] I have reviewed the documentation https://docs.sentry.io/
  • [X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/nextjs

SDK Version

8.9.2

Framework Version

Next.js canary, but same result using Next.js 14.2.4

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

  1. Clone https://github.com/qnten/sentry-next-server-component-error-reproduction
  2. Add sentry dsn strings to the 3 configs
  3. Install, build and start using pnpm install && pnpm run build && pnpm run start
  4. Visit http://localhost:3000/
  5. Visit http://localhost:3000/error-in-page

Expected Result

Two errors should show up in sentry:

Actual Result

Only the error thrown right in the root page component shows up.

qnten avatar Jun 18 '24 11:06 qnten

I will take a look!

lforst avatar Jun 20 '24 11:06 lforst

To give an update here: I think this is kind of a big oversight on our end, and Sentry should report these errors. However, when I tried to come up with a way to do so, I noticed that it is de facto impossible to do unless we completely parse and rewrite your AST, which as of now is too risky for us.

I had a quick convo with Vercel since I thought Next.js should support such a use-case and expose some kind of onError hook we can use to collect these errors. They mentioned that they have plans in the form of an internal RFC to add something along these lines but actual work hasn't started.

I also opened a PR in the Next.js repo which would technically allow us to track these errors: https://github.com/vercel/next.js/pull/67064 It is a bit hacky but would serve as a temporary work-around. However, as it stands right now, there is no appetite to do something like this and they would like a more complete solution instead - which I respect.

For now, we can explain in our docs that nested RSC errors will not be collected, since I agree that this is very confusing. I am super surprised this didn't come up earlier, during development, and testing. I guess, I never tested this case and always only had 1 layer deep RSCs. I hope this makes sense and you understand. Thanks for reporting this!

lforst avatar Jun 21 '24 11:06 lforst

@lforst thank you for confirming und investigating! seems like such a common use case. so think a warning in the docs would nice.

from my user point of view its definitely surprising that component nesting has anything to do with how errors are collected.

qnten avatar Jun 21 '24 12:06 qnten

@lforst is the workaround for now to wrap the component's code in a try...catch and use Sentry.captureException (and then I guess rethrow it since we aren't actually handling the exception)

davidbarratt avatar Jul 25 '24 15:07 davidbarratt

@davidbarratt Correct. If you want to be super brave, you can use the latest Next.js canary and do what we do here: https://github.com/getsentry/sentry-javascript/blob/4d61a6db447a3b523fde60390d77046237dc3c27/dev-packages/e2e-tests/test-applications/nextjs-15/instrumentation.ts#L13

We worked with the Vercel team to figure out an API that can be used to track these errors but it's not out in stable yet.

lforst avatar Jul 25 '24 15:07 lforst

@lforst this is solved with https://nextjs.org/blog/next-15#instrumentationjs-stable, right?

qnten avatar Nov 06 '24 10:11 qnten

Correct! This is solved by upgrading to Next.js 15 and adding

// instrumentation.ts

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

export const onRequestError = Sentry.captureRequestError;

to instrumentation.ts.

lforst avatar Nov 06 '24 10:11 lforst