From 974a7e8df330a1dcbf15a06091c6f3aaf3c0faaa Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 4 Jan 2023 10:21:40 +0100 Subject: [PATCH 1/2] fix(modules): Modules loaders integration handles native error and prioritizes event modules --- CHANGELOG.md | 1 + src/js/integrations/modulesloader.ts | 9 +++- src/js/wrapper.ts | 3 ++ test/integrations/modulesloader.test.ts | 59 +++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 test/integrations/modulesloader.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index e3a055b64a..5c84a48f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Missing `originalException` in `beforeSend` for events from react native error handler ([#2706](https://github.com/getsentry/sentry-react-native/pull/2706)) +- ModulesLoader integration returns original event if native is not available and event modules overwrite native modules ([#](https://github.com/getsentry/sentry-react-native/pull/)) ### Dependencies diff --git a/src/js/integrations/modulesloader.ts b/src/js/integrations/modulesloader.ts index 0480b85639..4418532275 100644 --- a/src/js/integrations/modulesloader.ts +++ b/src/js/integrations/modulesloader.ts @@ -1,4 +1,5 @@ import { Event,EventProcessor, Integration } from '@sentry/types'; +import { logger } from '@sentry/utils'; import { NATIVE } from '../wrapper'; @@ -23,13 +24,17 @@ export class ModulesLoader implements Integration { addGlobalEventProcessor(async (event: Event) => { if (!isSetup) { - modules = await NATIVE.fetchModules(); + try { + modules = await NATIVE.fetchModules(); + } catch (e) { + logger.log(`Failed to get modules from native: ${e}`); + } isSetup = true; } if (modules) { event.modules = { - ...event.modules, ...modules, + ...event.modules, }; } return event; diff --git a/src/js/wrapper.ts b/src/js/wrapper.ts index 5abd733ae6..33be9c0996 100644 --- a/src/js/wrapper.ts +++ b/src/js/wrapper.ts @@ -84,6 +84,9 @@ interface SentryNativeWrapper { */ export const NATIVE: SentryNativeWrapper = { async fetchModules(): Promise | null> { + if (!this.enableNative) { + throw this._DisabledNativeError; + } if (!this._isModuleLoaded(RNSentry)) { throw this._NativeClientError; } diff --git a/test/integrations/modulesloader.test.ts b/test/integrations/modulesloader.test.ts new file mode 100644 index 0000000000..6dc9d26422 --- /dev/null +++ b/test/integrations/modulesloader.test.ts @@ -0,0 +1,59 @@ +import { Event, EventHint } from '@sentry/types'; + +import { ModulesLoader } from '../../src/js/integrations'; +import { NATIVE } from '../../src/js/wrapper'; + +jest.mock('../../src/js/wrapper'); + +describe('Modules Loader', () => { + let integration: ModulesLoader; + + beforeEach(() => { + integration = new ModulesLoader(); + }); + + it('integration event processor does not throw on native error', async () => { + (NATIVE.fetchModules as jest.Mock).mockImplementation(() => { throw new Error('Test Error') }); + const mockEvent: Event = { + modules: { + eventModule: 'eventModuleVersion', + }, + }; + const processedEvent = await executeIntegrationFor(mockEvent); + + expect(processedEvent).toEqual(mockEvent); + }); + + it('merges event modules with native modules', async () => { + (NATIVE.fetchModules as jest.Mock).mockImplementation(() => ({ + nativeModules: 'nativeModuleVersion', + duplicateModule: 'duplicateNativeModuleVersion', + })); + const mockEvent: Event = { + modules: { + eventModule: 'eventModuleVersion', + duplicateModule: 'duplicateEventModuleVersion', + } + }; + const processedEvent = await executeIntegrationFor(mockEvent); + + expect(processedEvent?.modules).toEqual({ + eventModule: 'eventModuleVersion', + nativeModules: 'nativeModuleVersion', + duplicateModule: 'duplicateEventModuleVersion', + }); + }); + + function executeIntegrationFor(mockedEvent: Event, mockedHint: EventHint = {}): Promise { + return new Promise((resolve, reject) => { + integration.setupOnce(async (eventProcessor) => { + try { + const processedEvent = await eventProcessor(mockedEvent, mockedHint); + resolve(processedEvent); + } catch (e) { + reject(e); + } + }); + }); + } +}); From e467c06e569542253326b25fc0402e77dd7d99c4 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 4 Jan 2023 10:28:54 +0100 Subject: [PATCH 2/2] Add pr id --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c84a48f9e..d2d21ac052 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Fixes - Missing `originalException` in `beforeSend` for events from react native error handler ([#2706](https://github.com/getsentry/sentry-react-native/pull/2706)) -- ModulesLoader integration returns original event if native is not available and event modules overwrite native modules ([#](https://github.com/getsentry/sentry-react-native/pull/)) +- ModulesLoader integration returns original event if native is not available and event modules overwrite native modules ([#2730](https://github.com/getsentry/sentry-react-native/pull/2730)) ### Dependencies