fix: Polyfill the promise library to permanently fix unhandled rejections

[jennmueng]

📢 Type of change

• Bugfix
• New feature
• Enhancement
• Refactoring

📜 Description

Instead of warning the user and requiring them to set the Promise library resolution, we instead polyfill the global promise library to ensure we can install our handler.

Adds a new option patchGlobalPromise that can be passed to either Sentry.init or the ReactNativeErrorHandlers integration.

There is the expectation that a third party SDK shouldn't have the side effect of polyfilling a commonly used global instance, but as RN has not changed the functionality of its promises for quite a while we should be fine. If you use a polyfill such as core-js for example, such as adding Promise.allSettled, they would have to use the polyfill after Sentry init. We can also add back in the Promise warning along with a boolean toggle for people who do not want to polyfill.

💡 Motivation and Context

Resolves #1942

NOTE
Polyfilling like this also fixes unhandled promise rejections in Hermes, which was what has been blocking #1836.

💚 How did you test it?

e2e tests test unhandled promise rejections.

📝 Checklist

• I reviewed submitted code
• I added tests to verify changes
• All tests passing
• No breaking changes

Next Steps:

Rewrite https://docs.sentry.io/platforms/react-native/troubleshooting/#unhandled-promise-rejections

-> getsentry/sentry-docs#4525

[AbhiPrasad]

Lemme dig into this so I understand a little better before ✅ - tho scream at me on discord if you feel like this is prio and I’ll focus on it.

[marandaneto]

@bruno-garcia after discussing with @jennmueng, We've agreed to add a flag to the Options, that is enabled by default, but is possible to disable patching the Global promise, which removes the side effect of poly filling the Global promise during SDK init, obviously catching unhandled promises won't work then.

Something like patchGlobalPromise or polyfillGlobalPromise.

This flag would also go to the docs, likely rewriting this section https://docs.sentry.io/platforms/react-native/troubleshooting/#unhandled-promise-rejections

[bruno-garcia]

@bruno-garcia after discussing with @jennmueng, We've agreed to add a flag to the Options, that is enabled by default, but is possible to disable patching the Global promise, which removes the side effect of poly filling the Global promise during SDK init, obviously catching unhandled promises won't work then.

Something like patchGlobalPromise or polyfillGlobalPromise.

This flag would also go to the docs, likely rewriting this section docs.sentry.io/platforms/react-native/troubleshooting/#unhandled-promise-rejections

It's a very good idea to have an opt-out!

[AbhiPrasad]

Ok I like the global flag and general approach, but I’ll keep reading up in case we get more problems with this again.

also the react native code base at https://github.com/facebook/react-native is quite the adventure to peruse 😁

[AbhiPrasad]

While we here, can we leave a comment explaining why we have the disable then enable?

[jennmueng]

Hmm, looks like it just sets the properties on Promise to null before setting it again anyways in enable. So it's not needed. It's something we've kept here since the start. I'll remove it.

[AbhiPrasad]

Also, another thing I wanna make sure is fine - what happens if somebody else polyfills the global promise? Is there a danger of our implementations conflicting? Is this a pattern used by other libraries?

[jennmueng]

Also, another thing I wanna make sure is fine - what happens if somebody else polyfills the global promise? Is there a danger of our implementations conflicting? Is this a pattern used by other libraries?

@AbhiPrasad Yep, the possible conflict if the user or other libraries are polyfilling the global promise instance like us is what kept me off of this solution originally. I think having the option to disable our polyfill while also being able to manually ensure the promise instances are the same is good enough to get promise rejection tracking working on the latest RN.

[AbhiPrasad]

Happy new year! Let’s 🚢

[SimenB]

Not sure if this is the correct place, but a new version of promise has been released which includes support for Promise.allSettled. However, updating to it causes a bunch of errors (TypeError: Promise.allSettled is not a function.). I've tried setting patchGlobalPromise: false which makes no difference. Rolling back to 8.1.0 works

An update has also landed within RN, so this will hit everybody somewhat soonish: facebook/react-native#34544

[krystofwoldrich]

Not sure if this is the correct place, but a new version of promise has been released which includes support for Promise.allSettled. However, updating to it causes a bunch of errors (TypeError: Promise.allSettled is not a function.). I've tried setting patchGlobalPromise: false which makes no difference. Rolling back to 8.1.0 works

An update has also landed within RN, so this will hit everybody somewhat soonish: facebook/react-native#34544

Thank you for the heads up. We will definitely take a look on that. If the issue are related to our sdk, I would suggest opening a new issue.

I've tried to upgrade the library in new RN project with sentry and I got no errors.

[SimenB]

I'm using expo, might have something to do with it?

I'll try to find the time to create a reproduction (no promises tho, I've locked down the dep on our side so we're not affected).

---

Have you tried actually using Promise.allSettled?

[marandaneto]

@SimenB please raise a new issue with more context, this issue is already closed, a minimal reproducible example would help as well.

[marandaneto]

Since you are using expo, also double-check with them in case there are conflicts.