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

ErrorBoundary errors are being incorrectly marked as unhandled since 5.9.2 #10985

Closed
7 of 11 tasks
julianD77 opened this issue Feb 27, 2024 · 7 comments · Fixed by #10989
Closed
7 of 11 tasks

ErrorBoundary errors are being incorrectly marked as unhandled since 5.9.2 #10985

julianD77 opened this issue Feb 27, 2024 · 7 comments · Fixed by #10989

Comments

@julianD77
Copy link

OS:

  • Windows
  • MacOS
  • Linux

Platform:

  • iOS
  • Android

SDK:

  • @sentry/react-native (>= 1.0.0)
  • react-native-sentry (<= 0.43.2)

SDK version: 5.15.2

react-native version: 0.72.9
react version: 18.2.0

Are you using Expo?

  • Yes
  • No

Are you using sentry.io or on-premise?

  • sentry.io (SaaS)
  • on-premise

If you are using sentry.io, please post a link to your issue so we can take a look:

Issue where the error is classified as unhandled
https://mixcloud.sentry.io/discover/ios:45b823357fd441a6bb2ccc68581d293e

Issue (in older release) where the same error is classified as handled
https://mixcloud.sentry.io/discover/ios:18fd9d35a97c4f49a920b4d1ca465d25

Configuration:

Sentry.init({
  dsn: env.SENTRY,
  ignoreErrors: IGNORE_ERRORS,
  environment: IS_DEVELOPMENT_BUILD ? 'development' : 'production',
  debug: IS_DEVELOPMENT_BUILD,
  release: sentryRelease,
  dist: getBuildNumber()
});

I have the following issue:

Some Javascript errors that are being thrown explicitly by our Application code are being reported as Unhandled i.e. handled: false in Sentry - yet the code that throws the error is surrounded by an ErrorBoundary and does not result in a hard crash when the error occurs. The fallback component we provide to the error boundary is triggered.

Potentially relevant is the fact that in older releases of our App where we were using react-native 0.66.5, react 17.0.2 and sentry/react-native version 4.15.0 the very same errors were thrown from the same code are reported as handled: true by Sentry.

Steps to reproduce:

  • Trigger code that throws an error but is contained within an ErrorBoundary, such as :

ExampleComponent

    ...
    if (!cloudcast) {
        throw new Error(`No cloudcast found for id ${cloudcastId}`);
    }
    ...

Surrounding ErrorBoundary

            <ErrorBoundary
                fallback={fallback}
                onBack={onBack}
                onRetry={onRetry}
            >
                <OfflineBoundary fallback={fallback}>
                    <React.Suspense fallback={fallback}>
                        <ExampleComponent>
                    </React.Suspense>
                </OfflineBoundary>
            </ErrorBoundary>

Actual result:

  • The fallback component renders but the error is reported to Sentry and classified as unhandled

Expected result:

  • The fallback component renders and the error is reported to Sentry and classified as handled, since it has been caught by an ErrorBoundary

Is the fact that in the Sentry issue UX (and the JSON for error) the metadata.type is React ErrorBoundary Error in the newer release where the error is classified as unhandled, and Error in the older release where the error is correctly classified as handled? As mentioned the relevant application code has not changed - but the underlying RN dependencies have.

Here is a screenshot showing the same error across the two different releases, the left-hand side is our newer release using RN0.72.9 where the error is classified as unhandled. The right-hand side is the older release where the error is classified as handled.

Sentry-unhandled-error

Can anyone think of any reason why the same error scenarios are now being classified as Unhandled by Sentry?

@krystofwoldrich krystofwoldrich changed the title Errors surrounded by an ErrorBoundary are being incorrectly displayed in Sentry Dashboard as Unhandled since updating to RN0.72.9 and @sentry/react-native 5.1.5.2 Errors surrounded by an ErrorBoundary are being incorrectly displayed in Sentry Dashboard as Unhandled since updating to RN0.72.9 and @sentry/react-native 5.15.2 Feb 28, 2024
@krystofwoldrich
Copy link
Member

Hi @julianD77,
thank you for the message,
this change was introduced by #8914 (since RN SDK release https://github.com/getsentry/sentry-react-native/releases/tag/5.9.2).

@Lms24 could you share more details about the reasons for this change?

@krystofwoldrich krystofwoldrich changed the title Errors surrounded by an ErrorBoundary are being incorrectly displayed in Sentry Dashboard as Unhandled since updating to RN0.72.9 and @sentry/react-native 5.15.2 ErrorBoundary errors are being incorrectly marked as unhandled since 5.9.2 Feb 29, 2024
@krystofwoldrich
Copy link
Member

@julianD77 You can overwrite the default values in beforeSend, but note this example applies to all error boundaries, not only the ones with fallback.

  beforeSend: (event: Sentry.Event, hint) => {
    if (event.exception?.values?.[1]?.type?.startsWith('React ErrorBoundary')) {
      event.exception?.values?.forEach(value => {
        if (value.mechanism) {
          value.mechanism.handled = true;
        } else {
          value.mechanism = {
            type: 'generic',
            handled: true,
          };
        }
      });
    }
    return event;
  },

@Lms24
Copy link
Member

Lms24 commented Mar 5, 2024

So the initial reasoning for setting handled: false across the instrumentations where we automatically catch errors is because we can never know if users actually have handling logic around the instrumentation. Also, we wanted to unify the behaviour because some instrumentation sent handled vs others that sent unhandled.

More details: #6073 (comment)

However, as you correctly pointed out, we can actually determine this in the error boundary case by checking if a fallback was passed to it. After a quick discussion, we decided to set handled based on the presence of fallback. @julianD77 just to confirm, would this fix your use case?

@krystofwoldrich
Copy link
Member

Since this is completely implemented in JS I'm moving this issue to the JS repository.

@krystofwoldrich krystofwoldrich transferred this issue from getsentry/sentry-react-native Mar 8, 2024
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Mar 8, 2024
@Lms24
Copy link
Member

Lms24 commented Mar 8, 2024

I opened #10989 with the fix. Appreciate reviews!

@getsantry getsantry bot removed the status in GitHub Issues with 👀 2 Mar 8, 2024
Lms24 added a commit that referenced this issue Mar 8, 2024
#10989)

Previously, we made a change to always mark such errors as unhandled
(#8914). However, as pointed out and discussed in #10985, this isn't
always the best default value. I propose we set handled/unhandled
depending on the presence of the `fallback` component. If users specify
a fallback, I think it's safe to mark the error handled, given that
users show something else instead of the errored component.
@github-project-automation github-project-automation bot moved this from Needs Discussion to Done in Mobile & Cross Platform SDK Mar 8, 2024
Lms24 added a commit that referenced this issue Mar 12, 2024
#10989)

Previously, we made a change to always mark such errors as unhandled
(#8914). However, as pointed out and discussed in #10985, this isn't
always the best default value. I propose we set handled/unhandled
depending on the presence of the `fallback` component. If users specify
a fallback, I think it's safe to mark the error handled, given that
users show something else instead of the errored component.
@Lms24
Copy link
Member

Lms24 commented Mar 12, 2024

I backported this fix to v7 and it will be released with 7.108.0 soon. cc @krystofwoldrich

@AbhiPrasad
Copy link
Member

Fixed released with https://github.com/getsentry/sentry-javascript/releases/tag/7.107.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Archived in project
4 participants