diff --git a/CHANGELOG.md b/CHANGELOG.md index d712d13913..fe21c53506 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Fix duplicate JS error reporting on iOS New Architecture when the native SDK is initialized early via `sentry.options.json` ("Capture App Start Errors"). It's done by applying the `ExceptionsManager.reportException` C++ wrapper filter in both init paths ([#6145](https://github.com/getsentry/sentry-react-native/pull/6145)) - Fix boolean options from `sentry.options.json` being ignored on Android when using `RNSentrySDK.init` ([#6130](https://github.com/getsentry/sentry-react-native/pull/6130)) +- Fix `includeWebFeedback: false` Metro config option causing crash at startup ([#6150](https://github.com/getsentry/sentry-react-native/pull/6150)) ### Dependencies diff --git a/packages/core/src/js/tools/metroconfig.ts b/packages/core/src/js/tools/metroconfig.ts index f5057b5a77..e059baa9ce 100644 --- a/packages/core/src/js/tools/metroconfig.ts +++ b/packages/core/src/js/tools/metroconfig.ts @@ -257,7 +257,7 @@ type CustomResolverBeforeMetro068 = ( function buildSentryPackageExcludeResolver( config: MetroConfig, includePackage: boolean | undefined, - moduleRegex: RegExp, + shouldExcludeModule: (moduleName: string, context: CustomResolutionContext) => boolean, optionName: string, ): MetroConfig { const originalResolver = config.resolver?.resolveRequest as CustomResolver | CustomResolverBeforeMetro068 | undefined; @@ -270,7 +270,7 @@ function buildSentryPackageExcludeResolver( ) => { if ( (includePackage === false || (includePackage === undefined && (platform === 'android' || platform === 'ios'))) && - !!(oldMetroModuleName ?? moduleName).match(moduleRegex) + shouldExcludeModule(oldMetroModuleName ?? moduleName, context) ) { return { type: 'empty' } as Resolution; } @@ -314,11 +314,23 @@ export function withSentryResolver(config: MetroConfig, includeWebReplay: boolea return buildSentryPackageExcludeResolver( config, includeWebReplay, - /@sentry(?:-internal)?\/replay/, + moduleName => /@sentry(?:-internal)?\/replay/.test(moduleName), 'includeWebReplay', ); } +/** + * `@sentry/browser` re-exports feedback through `feedbackSync.js` and `feedbackAsync.js` + * which call `buildFeedbackIntegration()` at module evaluation time. + * Stubbing only `@sentry-internal/feedback` makes that call crash, + * so we also stub these wrapper modules. + */ +const SENTRY_BROWSER_FEEDBACK_WRAPPER_RE = /\/feedback(Sync|Async)/; + +function isFromSentryBrowser(originModulePath: string): boolean { + return originModulePath.includes('@sentry/browser'); +} + /** * Includes `@sentry-internal/feedback` packages based on the `includeWebFeedback` flag and current bundle `platform`. */ @@ -326,7 +338,10 @@ export function withSentryFeedbackResolver(config: MetroConfig, includeWebFeedba return buildSentryPackageExcludeResolver( config, includeWebFeedback, - /@sentry(?:-internal)?\/feedback/, + (moduleName, context) => + /@sentry(?:-internal)?\/feedback/.test(moduleName) || + (isFromSentryBrowser((context as { originModulePath?: string }).originModulePath ?? '') && + SENTRY_BROWSER_FEEDBACK_WRAPPER_RE.test(moduleName)), 'includeWebFeedback', ); } diff --git a/packages/core/test/tools/metroconfig.test.ts b/packages/core/test/tools/metroconfig.test.ts index a07e14dbdd..e4cf2a121b 100644 --- a/packages/core/test/tools/metroconfig.test.ts +++ b/packages/core/test/tools/metroconfig.test.ts @@ -467,6 +467,53 @@ describe('metroconfig', () => { }); }); + describe.each([['./feedbackSync.js'], ['./feedbackAsync.js']])('with %s from @sentry/browser', wrapperModule => { + const SENTRY_BROWSER_ORIGIN = '/project/node_modules/@sentry/browser/build/npm/esm/prod/index.js'; + + let browserContextMock: any; + + beforeEach(() => { + browserContextMock = { + resolveRequest: jest.fn(), + originModulePath: SENTRY_BROWSER_ORIGIN, + }; + }); + + test('removes wrapper when includeWebFeedback is false', () => { + const modifiedConfig = withSentryFeedbackResolver(config, false); + const result = resolveRequest(modifiedConfig, browserContextMock, wrapperModule, 'android'); + + expect(result).toEqual({ type: 'empty' }); + expect(originalResolverMock).not.toHaveBeenCalled(); + }); + + test('removes wrapper when includeWebFeedback is undefined on native', () => { + const modifiedConfig = withSentryFeedbackResolver(config, undefined); + const result = resolveRequest(modifiedConfig, browserContextMock, wrapperModule, 'ios'); + + expect(result).toEqual({ type: 'empty' }); + expect(originalResolverMock).not.toHaveBeenCalled(); + }); + + test('keeps wrapper when includeWebFeedback is true', () => { + const modifiedConfig = withSentryFeedbackResolver(config, true); + resolveRequest(modifiedConfig, browserContextMock, wrapperModule, 'web'); + + ExpectToBeCalledWithMetroParameters(originalResolverMock, browserContextMock, wrapperModule, 'web'); + }); + + test('keeps wrapper when origin is not @sentry/browser', () => { + const nonBrowserContextMock = { + resolveRequest: jest.fn(), + originModulePath: '/project/node_modules/some-other-package/index.js', + }; + const modifiedConfig = withSentryFeedbackResolver(config, false); + resolveRequest(modifiedConfig, nonBrowserContextMock, wrapperModule, 'android'); + + ExpectToBeCalledWithMetroParameters(originalResolverMock, nonBrowserContextMock, wrapperModule, 'android'); + }); + }); + test('calls originalResolver when moduleName is not @sentry-internal/feedback', () => { const modifiedConfig = withSentryFeedbackResolver(config, true); const moduleName = 'some/other/module'; @@ -662,6 +709,48 @@ describe('metroconfig', () => { expect(mockExit).toHaveBeenCalledWith(-1); }); }); + describe('feedback resolver handles @sentry/browser barrel imports', () => { + const SENTRY_BROWSER_BARREL = '/project/node_modules/@sentry/browser/build/npm/esm/prod/index.js'; + + const browserBarrelImports = [ + { moduleName: './feedbackSync.js', shouldBeEmpty: true }, + { moduleName: './feedbackAsync.js', shouldBeEmpty: true }, + { moduleName: '@sentry-internal/feedback', shouldBeEmpty: true }, + { moduleName: '@sentry-internal/replay', shouldBeEmpty: false }, + { moduleName: '@sentry-internal/replay-canvas', shouldBeEmpty: false }, + { moduleName: '@sentry-internal/browser-utils', shouldBeEmpty: false }, + { moduleName: '@sentry/core/browser', shouldBeEmpty: false }, + { moduleName: './helpers.js', shouldBeEmpty: false }, + { moduleName: './client.js', shouldBeEmpty: false }, + { moduleName: './sdk.js', shouldBeEmpty: false }, + { moduleName: './userfeedback.js', shouldBeEmpty: false }, + ]; + + test.each(browserBarrelImports)( + '$moduleName is resolved to empty=$shouldBeEmpty when includeWebFeedback is false', + ({ moduleName, shouldBeEmpty }) => { + const originalResolver = jest.fn(); + const config: MetroConfig = { + resolver: { resolveRequest: originalResolver }, + }; + const context = { + resolveRequest: jest.fn(), + originModulePath: SENTRY_BROWSER_BARREL, + }; + + const modifiedConfig = withSentryFeedbackResolver(config, false); + const result = modifiedConfig.resolver?.resolveRequest?.(context, moduleName, 'android'); + + if (shouldBeEmpty) { + expect(result).toEqual({ type: 'empty' }); + expect(originalResolver).not.toHaveBeenCalled(); + } else { + expect(result).not.toEqual({ type: 'empty' }); + expect(originalResolver).toHaveBeenCalled(); + } + }, + ); + }); }); // function create mock metro frame