Skip to content

Commit

Permalink
fix(modules): Modules loaders integration handles native error and ev…
Browse files Browse the repository at this point in the history
…ent modules are prioritised over native (#2730)
  • Loading branch information
krystofwoldrich authored Jan 4, 2023
1 parent 4b32ab4 commit c35c86b
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ([#2730](https://github.com/getsentry/sentry-react-native/pull/2730))

### Dependencies

Expand Down
9 changes: 7 additions & 2 deletions src/js/integrations/modulesloader.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Event,EventProcessor, Integration } from '@sentry/types';
import { logger } from '@sentry/utils';

import { NATIVE } from '../wrapper';

Expand All @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions src/js/wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ interface SentryNativeWrapper {
*/
export const NATIVE: SentryNativeWrapper = {
async fetchModules(): Promise<Record<string, string> | null> {
if (!this.enableNative) {
throw this._DisabledNativeError;
}
if (!this._isModuleLoaded(RNSentry)) {
throw this._NativeClientError;
}
Expand Down
59 changes: 59 additions & 0 deletions test/integrations/modulesloader.test.ts
Original file line number Diff line number Diff line change
@@ -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<Event | null> {
return new Promise((resolve, reject) => {
integration.setupOnce(async (eventProcessor) => {
try {
const processedEvent = await eventProcessor(mockedEvent, mockedHint);
resolve(processedEvent);
} catch (e) {
reject(e);
}
});
});
}
});

0 comments on commit c35c86b

Please sign in to comment.