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

Unhandled promise rejections are not logged to sentry #1942

Closed
Arhane opened this issue Dec 9, 2021 · 11 comments · Fixed by #1984
Closed

Unhandled promise rejections are not logged to sentry #1942

Arhane opened this issue Dec 9, 2021 · 11 comments · Fixed by #1984

Comments

@Arhane
Copy link

Arhane commented Dec 9, 2021

My feature request is connected to the bug #1077.

I've tried the solution from docs, but it didn't work out for me. So I found another solution with polyfilling existing Promise in React Native.

I've used this package

import { setUnhandledPromiseRejectionTracker } from 'react-native-promise-rejection-utils';
import { getCurrentHub } from '@sentry/core';
setUnhandledPromiseRejectionTracker((id, error) => {
  getCurrentHub().captureException(error, {
    data: { id },
    originalException: error,
  });
});

Inside the react-native-promise-rejection-utils package polyfills the existing promise. You can check it out here

So I propose changing the method, removing the check for the promise version and using the package react-native-promise-rejection-utils or polyfill directly without relaying on this package.

I think this solution is better in many ways:

  1. It doesn't require user to do or check anything. It just works
  2. _checkPromiseVersion can be removed
@marandaneto marandaneto added this to To Do in kanban via automation Dec 9, 2021
@marandaneto marandaneto moved this from To Do to Needs Discussion in kanban Dec 9, 2021
@marandaneto
Copy link
Contributor

@jennmueng can you have a look at it?

@marandaneto
Copy link
Contributor

@Arhane thanks for the report.

I've tried the solution from docs, but it didn't work out for me

can you provide a minimal reproducible example? it worked on our end.

@marandaneto marandaneto moved this from Needs Discussion to In progress in kanban Dec 10, 2021
@jennmueng
Copy link
Member

jennmueng commented Dec 20, 2021

@marandaneto @Arhane We wanted to avoid polyfilling the global promise instance inside the SDK in fear of compatibility issues with older versions of React Native...but now that I've given it some more thought it might be a good solution out as RN hasn't really changed anything regarding Promises for a while although maybe a third party SDK shouldn't have the behavior of polyfilling commonly used global feature. Will need to think this one through.

@Arhane
Copy link
Author

Arhane commented Dec 20, 2021

@marandaneto I will try to make a demo.

@jennmueng I know, this isn't the best solution, but it seems more user-friendly, than version-matching. I agree, that third party SDK shouldn't polyfill the promise. I guess, we can hide it behind some flag? Like polyfillPromise or something more eloquent

@pors
Copy link

pors commented Dec 24, 2021

I have the same issue. I did match the versions, but that doesn't help.

grep "promise" node_modules/react-native/package.json
    "promise": "^8.0.3",
grep "promise" ./package.json
    "promise": "^8.0.3"
Possible Unhandled Promise Rejection (id: 0):
TypeError: Attempted to assign to readonly property.

Also, setting debug: true doesn't show me the [Sentry] Unhandled promise rejections will be caught by Sentry. message.

@jennmueng
Copy link
Member

@pors What version of the SDK are you running, which version RN, is this on iOS/Android/Both, and are you running Hermes?

@pors
Copy link

pors commented Dec 26, 2021

@jennmueng:

  • SDK 44
  • RN 0.64.3
  • Only tested on iOS
  • Hermes: yes

@checklist
Copy link

I have the same issue with:

  • RN 0.66.4
  • Android
  • iOS
  • Hermes: yes
  • "@sentry/react-native": "^3.2.10",

Please issue an urgent fix. Thanks!

kanban automation moved this from In progress to Done Jan 10, 2022
@marandaneto
Copy link
Contributor

hey, mind testing out the latest release that includes this fix? https://github.com/getsentry/sentry-react-native/releases/tag/3.2.11
Feedback welcomed, thanks.

@Arhane
Copy link
Author

Arhane commented Jan 11, 2022

Just tested the latest version (3.2.11) and it works.
But it works without patchGlobalPromise too, which is strange.

Tested with this code

Promise.reject('Test the new version of sentry');

and Sentry.init

Sentry.init({
  dsn: SENTRY_DSN,
});

@marandaneto
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
6 participants