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 promises are not logged at all (iOS and Android) #1077

Closed
4 of 6 tasks
lukewlms opened this issue Sep 15, 2020 · 30 comments · Fixed by #1367 or #1886
Closed
4 of 6 tasks

Unhandled promises are not logged at all (iOS and Android) #1077

lukewlms opened this issue Sep 15, 2020 · 30 comments · Fixed by #1367 or #1886
Assignees

Comments

@lukewlms
Copy link

lukewlms commented Sep 15, 2020

Platform:

  • iOS

SDK:

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

SDK version: "@sentry/react-native": "^1.7.2",

react-native version: "react-native": "^0.63.2",

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:

No issue was created

Configuration:

(@sentry/react-native)

Sentry.init({
  dsn: 'https://[email protected]/...'
  // no extra options
});

I have following issue:

Normal exceptions are logged to Sentry.io just fine, but unhandled promises are not logged at all. My understanding is that this should happen by default. I do get a yellow box showing the unhandled exception but it seems that Sentry never sees it.

This repros on both Android and iOS.

Steps to reproduce:

  1. Install sentry with minimal config
  2. Generate an unhandled promise rejection

Actual result:

Yellow box appears but no error logged to sentry.io

image

Expected result:

Unhandled promise rejections are logged to sentry.io

@azai91
Copy link

azai91 commented Sep 16, 2020

Having the same issue.

@WinstonKamau
Copy link

I am also experiencing the same issue for unhandled promises.

@cruzach
Copy link
Contributor

cruzach commented Jan 5, 2021

I think that this is due to the same issue as this- facebook/fbjs#413 (mismatch of promise versions)

@jer-sen
Copy link

jer-sen commented Feb 2, 2021

@cruzach you are right but its a big issue since it makes sentry nearly useless!
With Expo you can add this to your package.json as a workaround until fbjs dependencies are upgraded:

  "resolutions": {
    "expo/**/promise": "^8.0.3"
  },

@cruzach maybe sentry should hook itself directly on Promise global object as promise/setimmediate/rejection-tracking does and instead of using it. It would avoid the issue to reappear. Or add to the doc that there must be only one version of promise module per project.

@stevefai
Copy link

Is this working for anybody right now with the latest versions? Just created a brand new react native project and running into the same issue.

@Baloche
Copy link

Baloche commented Feb 25, 2021

Same issue here with the latest versions. I am currently using @jer-sen's workaround and it works !

@jennmueng
Copy link
Member

I have reproduced the issue and confirmed that unhandled promise rejections are not being caught on my end as well. Adding this to our immediate TODO.

@somebody32
Copy link

@jennmueng I'm still having this problem even with the latest version of the SDK. Anything I can do to help pin it down?

@jennmueng
Copy link
Member

@somebody32 Can you provide a reproduction for us where this occurs?

@somebody32
Copy link

@jennmueng happily

here is my test code

Sentry.init({
  dsn:  'dsn',
  debug: true,
});

function App() {
  useEffect(() => {
    (async () => { throw 'boom'; })();
  }, []);

  return  <Button onPress={async () => { throw 'boom'; }} title="Exception"  />
}

and both are not sent to sentry, nothing in logs either

@somebody32
Copy link

@jennmueng can open another issue if you think it would be more useful, but just confirmed that it happens in "release" mode too: the app sometimes shows just a white screen because of unhandled rejection but nothing goes into sentry

@jer-sen
Copy link

jer-sen commented Apr 6, 2021

@jennmueng your fix does not work. You expect Promise._Y to store the _onHandle but during promise module's build a random name is chosen depending on a hash of the source code (cf https://github.com/then/promise/blob/91b7b4cb6ad0cacc1c70560677458fe0aac2fa67/build.js#L16). So you can not access directly to the _onHandle of the global Promise nor to the _onHandle of module promise. A solution may be to remove sentry dependency on promise module and to use a peer dependency instead, so that you can require the only and so the right promise/setimmediate/core. Once again, its a major issue, please fix it quickly.

@stevefai
Copy link

stevefai commented Apr 13, 2021

The fix is unfortunately not working for me either. I feel like this should get a lot more attention if it's indeed a general problem and I'm not misconfiguring something.

@jennmueng I assume you have tested the fix and it's working on your side, could you maybe provide your setup information? (also, if I can do anything to help with any information please let me know)

@jennmueng jennmueng reopened this Apr 14, 2021
@somebody32
Copy link

It looks like I've managed to make it work by forcing the particular version of promise in my package.json:

 "resolutions": {
    "promise": "^8.1.0"
  },

@jennmueng
Copy link
Member

@somebody32 That sounds like the most viable workaround I have so far until Facebook fixes this.

@jer-sen
Copy link

jer-sen commented Apr 17, 2021

@jennmueng you should really remove your dependency on promise and replace it with a peerDependency so that you will use the already available version. Or you can downgrade dependency to a more compatible one such as 8.x to match react-native dependency ^8.0.3

@jennmueng
Copy link
Member

jennmueng commented Apr 17, 2021

@jer-sen We do not have a dependency on promise. The source of this issue is that we are using the already available version which is the incorrect one provided by fbjs.

We considered adding a peerDependency of 8.x for it to pull the one from React Native but that would also break people from older versions of React Native that do not use React Native >=0.63 with promise ^8.0.3

@jer-sen
Copy link

jer-sen commented Apr 17, 2021

@jennmueng you are right :(

Then maybe you can improve your fix with some reverse engineering to find the right properties names:

const noopKey = Object.entries(Promise).find(([k, v]) => typeof v === 'function' && (v as { name: string }).name === 'noop')[0][1];
const characterSet = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
const noopIndex = characterSet.indexOf(noopKey);

const onHandle = Promise['_' + characterSet[(noopIndex + characterSet.length - 2) % characterSet.length]];
const onReject  = Promise['_' + characterSet[(noopIndex + characterSet.length - 1) % characterSet.length]];

console.log({ onHandle, onReject });

Not perfect but it will work for small code change of promise module.

@jennmueng
Copy link
Member

I will consider your solution @jer-sen. What's really interesting if we have an unhandled promise rejection test in our end-to-end tests on CI, and they've been passing no problem on the latest React Native versions 0.63.x-0.64.x. In theory if the versions are the same, promise version 7.1.1 should the minified names be the same?

Maybe the best solution is for people to bump this PR that has been sitting for a while that would fix the source of the problem: facebook/fbjs#413

@jer-sen
Copy link

jer-sen commented Apr 19, 2021

Maybe something wrong with your tests...
But I'm quite sure that your fix can not work.

      // Will return undefined (_onHandle/_onReject does not exist after build script, _Y/_Z only exists with current RN version)
      const _onHandle = Promise._onHandle ?? Promise._Y; 
      const _onReject = Promise._onReject ?? Promise._Z;


      if ( // Never true since _onHandle === undefined and _onReject === undefined
        Promise !== _global.Promise &&
        typeof _onHandle !== "undefined" &&
        typeof _onReject !== "undefined"
      ) {
        if ("_onHandle" in _global.Promise && "_onReject" in _global.Promise) {
          _global.Promise._onHandle = _onHandle;
          _global.Promise._onReject = _onReject;
        } else if ("_Y" in _global.Promise && "_Z" in _global.Promise) {
          _global.Promise._Y = _onHandle;
          _global.Promise._Z = _onReject;
        }
      }

And by the way, I don't think you can take _onHandle and _onReject from a version and apply them to another version because the _onReject generated by the rejection tracking uses property _deferredState of the Promise object which is minified (_V for current RN version but will be different with your version). You have to build your one rejection tracking (reusing the code of promise module) with the right minified properties (found with some reverse engineering).

@jer-sen
Copy link

jer-sen commented Jun 16, 2021

@jennmueng any update?

@jennmueng
Copy link
Member

jennmueng commented Jun 16, 2021

@jer-sen The best solution we have now is to just wait for the PR to be merged over there. Any stopgap solution would be just prone to more issues. We will document the resolutions fix and have that be our official method for now.

@kylerjensen
Copy link

@jennmueng From the looks of this comment it seems that facebook/fbjs#413 is not going to solve this issue. Am I reading that correctly?

@jer-sen
Copy link

jer-sen commented Oct 19, 2021

react-native-web still relies on fbjs so I'm still facing this issue :(

@jer-sen
Copy link

jer-sen commented Oct 22, 2021

@jennmueng as explained in my comment here facebook/fbjs#413 (comment), I think you should do 2 or 3 things to close this issue:

  • add to the installation doc that promise dependency must be added to the project identical to react-native's dependency
  • add a runtime check with an alert to the developer if Promise !== _global.Promise here
    const Promise = require("promise/setimmediate/core");
  • add a peer dependency to promise@^8.0.3? But it will break for older/newer versions of RN depending on another versoin of promise...

What do you think about all this?

@jennmueng
Copy link
Member

@jer-sen We're going to add a troubleshooting/install doc and I like your second idea, we will try it out and add that in.

@marandaneto
Copy link
Contributor

@jennmueng whats the outcome of it besides the pending docs? do we need to fix something on the SDK?

@jer-sen
Copy link

jer-sen commented Nov 12, 2021

@jennmueng & @marandaneto you should also clean the code by removing the current workaround when the runtime check will be implemented

@marandaneto
Copy link
Contributor

@jennmueng is currently experimenting with this #1886

@jer-sen
Copy link

jer-sen commented Dec 1, 2021

@marandaneto I have tried your fix but in dev mode with Expo the loger.warn message does not appear in terminal nor on my Android device. Replacing:

logger.warn("Unhandled promise rejections will not be caught by Sentry. Read about how to fix this on our troubleshooting page.");

by

setTimeout(() => console.warn("Unhandled promise rejections will not be caught by Sentry. Read about how to fix this on our troubleshooting page."), 0);

solves the issue (setTimeout and console instead of logger are both required). Any idea?

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