From dcf34ab46de1e5c4d5515218eb04a4ae508fd22d Mon Sep 17 00:00:00 2001 From: jennmueng Date: Mon, 1 Mar 2021 17:54:53 +0700 Subject: [PATCH 1/6] fix: Fix unhandle promise rejections not being tracked --- .../integrations/reactnativeerrorhandlers.ts | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/js/integrations/reactnativeerrorhandlers.ts b/src/js/integrations/reactnativeerrorhandlers.ts index 28c2ce6a63..2771175c30 100644 --- a/src/js/integrations/reactnativeerrorhandlers.ts +++ b/src/js/integrations/reactnativeerrorhandlers.ts @@ -1,6 +1,6 @@ import { getCurrentHub } from "@sentry/core"; import { Integration, Severity } from "@sentry/types"; -import { logger } from "@sentry/utils"; +import { getGlobalObject, logger } from "@sentry/utils"; import { ReactNativeClient } from "../client"; @@ -54,27 +54,55 @@ export class ReactNativeErrorHandlers implements Integration { enable: (arg: unknown) => void; // eslint-disable-next-line @typescript-eslint/no-var-requires,import/no-extraneous-dependencies } = require("promise/setimmediate/rejection-tracking"); + tracking.disable(); tracking.enable({ allRejections: true, - onHandled: () => { - // We do nothing - }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any onUnhandled: (id: any, error: any) => { if (__DEV__) { + // We mimic the behavior of unhandled promise rejections showing up as a warning. // eslint-disable-next-line no-console console.warn(id, error); } - getCurrentHub().captureException(error, { data: { id }, originalException: error, }); }, }); + + /* eslint-disable + @typescript-eslint/no-var-requires, + import/no-extraneous-dependencies, + @typescript-eslint/no-explicit-any, + @typescript-eslint/no-unsafe-member-access + */ + const Promise = require("promise/setimmediate/core"); + const _global = getGlobalObject(); + + /* In newer RN versions >=0.63, the global promise is not the same reference as the one imported from the promise library. + Due to this, we need to take the methods that tracking.enable sets, and then set them on the global promise. + Note: We do not want to overwrite the whole promise in case there are extensions present. + + If the global promise is the same as the imported promise (expected in RN <0.63), we do nothing. + */ + if ( + Promise !== _global.Promise && + "_Y" in _global.Promise && + "_Z" in _global.Promise + ) { + _global.Promise._Y = Promise._Y; + _global.Promise._Z = Promise._Z; + } + /* eslint-enable + @typescript-eslint/no-var-requires, + import/no-extraneous-dependencies, + @typescript-eslint/no-explicit-any, + @typescript-eslint/no-unsafe-member-access + */ } } - /** * Handle erros */ From 9ce7bed574c236ed9bc7d03b11bec253df27bfc3 Mon Sep 17 00:00:00 2001 From: jennmueng Date: Mon, 1 Mar 2021 17:55:05 +0700 Subject: [PATCH 2/6] test: Add unhandled promise rejection button to sample --- sample/src/screens/HomeScreen.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/sample/src/screens/HomeScreen.tsx b/sample/src/screens/HomeScreen.tsx index f6a516816d..4dc9ade94c 100644 --- a/sample/src/screens/HomeScreen.tsx +++ b/sample/src/screens/HomeScreen.tsx @@ -171,6 +171,15 @@ const HomeScreen = (props: Props) => { Uncaught Thrown Error + { + new Promise(() => { + throw new Error('Unhandled Promise Rejection'); + }); + }}> + Unhandled Promise Rejection + + { Sentry.nativeCrash(); From 464d9fd3dcb30dbf4ee751eb6914ef86ce80200d Mon Sep 17 00:00:00 2001 From: jennmueng Date: Mon, 1 Mar 2021 18:20:43 +0700 Subject: [PATCH 3/6] test(e2e): Add e2e test for unhandled promise rejections --- sample/src/screens/EndToEndTestsScreen.tsx | 9 ++++++++ sample/test/e2e.test.ts | 27 +++++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/sample/src/screens/EndToEndTestsScreen.tsx b/sample/src/screens/EndToEndTestsScreen.tsx index a1be477ba7..0801c68bae 100644 --- a/sample/src/screens/EndToEndTestsScreen.tsx +++ b/sample/src/screens/EndToEndTestsScreen.tsx @@ -52,6 +52,15 @@ const EndToEndTestsScreen = () => { }}> throw new Error + { + new Promise(() => { + throw new Error('Unhandled Promise Rejection'); + }); + }} + {...getTestProps('unhandledPromiseRejection')}> + Unhandled Promise Rejection + { Sentry.nativeCrash(); diff --git a/sample/test/e2e.test.ts b/sample/test/e2e.test.ts index 48da3c6ca5..918643d229 100644 --- a/sample/test/e2e.test.ts +++ b/sample/test/e2e.test.ts @@ -58,7 +58,7 @@ describe('End to end tests for common events', () => { const element = await driver.elementByAccessibilityId('captureMessage'); await element.click(); - await driver.sleep(100); + await driver.sleep(300); expect(await driver.hasElementByAccessibilityId('eventId')).toBe(true); @@ -93,4 +93,29 @@ describe('End to end tests for common events', () => { expect(sentryEvent.eventID).toMatch(eventId); }); + + test('unhandledPromiseRejection', async () => { + expect( + await driver.hasElementByAccessibilityId('unhandledPromiseRejection'), + ).toBe(true); + + const element = await driver.elementByAccessibilityId( + 'unhandledPromiseRejection', + ); + await element.click(); + + // Promises needs a while to fail + await driver.sleep(5000); + + expect(await driver.hasElementByAccessibilityId('eventId')).toBe(true); + + const eventIdElement = await driver.elementByAccessibilityId('eventId'); + const eventId = await eventIdElement.text(); + + await driver.sleep(10000); + + const sentryEvent = await fetchEvent(eventId); + + expect(sentryEvent.eventID).toMatch(eventId); + }); }); From bbca5c3a7ee9932d93b48430213d305253b0f8db Mon Sep 17 00:00:00 2001 From: jennmueng Date: Mon, 1 Mar 2021 18:35:14 +0700 Subject: [PATCH 4/6] ref: Accidentally changed back to wrong number --- sample/test/e2e.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sample/test/e2e.test.ts b/sample/test/e2e.test.ts index 918643d229..f8f11474b3 100644 --- a/sample/test/e2e.test.ts +++ b/sample/test/e2e.test.ts @@ -58,7 +58,7 @@ describe('End to end tests for common events', () => { const element = await driver.elementByAccessibilityId('captureMessage'); await element.click(); - await driver.sleep(300); + await driver.sleep(100); expect(await driver.hasElementByAccessibilityId('eventId')).toBe(true); From 604785e6f753961b9a0526f6937b205b24bc20d7 Mon Sep 17 00:00:00 2001 From: jennmueng Date: Mon, 1 Mar 2021 18:35:40 +0700 Subject: [PATCH 5/6] meta: Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86300d8b65..a64d8297cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- fix: Fix unhandle promise rejections not being tracked #1367 ## 2.2.1 From b1de06bbb0d8f45e98340d3cdae02692ba63879f Mon Sep 17 00:00:00 2001 From: jennmueng Date: Mon, 1 Mar 2021 18:42:53 +0700 Subject: [PATCH 6/6] ref: Safer way of overwriting global promise handlers --- src/js/integrations/reactnativeerrorhandlers.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/js/integrations/reactnativeerrorhandlers.ts b/src/js/integrations/reactnativeerrorhandlers.ts index 2771175c30..a557fd9645 100644 --- a/src/js/integrations/reactnativeerrorhandlers.ts +++ b/src/js/integrations/reactnativeerrorhandlers.ts @@ -87,13 +87,21 @@ export class ReactNativeErrorHandlers implements Integration { If the global promise is the same as the imported promise (expected in RN <0.63), we do nothing. */ + const _onHandle = Promise._onHandle ?? Promise._Y; + const _onReject = Promise._onReject ?? Promise._Z; + if ( Promise !== _global.Promise && - "_Y" in _global.Promise && - "_Z" in _global.Promise + typeof _onHandle !== "undefined" && + typeof _onReject !== "undefined" ) { - _global.Promise._Y = Promise._Y; - _global.Promise._Z = Promise._Z; + 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; + } } /* eslint-enable @typescript-eslint/no-var-requires,