`Sentry.init()` is disabling require cycle warnings for the entire app it's integrated in
OS:
- [ ] Windows
- [x] MacOS
- [ ] Linux
Platform:
- [x] iOS
- [x] Android
SDK:
- [x]
@sentry/react-native(>= 1.0.0) - [ ]
react-native-sentry(<= 0.43.2)
SDK version: 5.15.1
react-native version: 0.72.8
Are you using Expo?
- [ ] Yes
- [x] No
Are you using sentry.io or on-premise?
- [x] sentry.io (SaaS)
- [ ] on-premise
If you are using sentry.io, please post a link to your issue so we can take a look:
N/A
Configuration:
(@sentry/react-native)
Sentry.init({
dsn: 'https://[email protected]/...'
// other options aren't relevant to the bug report
});
I have the following issue:
When the client is initialised it tells RN's LogBox to ignore all require cycles in the app it's integrated with: https://github.com/getsentry/sentry-react-native/blob/5.15.1/src/js/utils/ignorerequirecyclelogs.ts#L7
This is not something I was expecting as it is not @sentry/react-native's responsibility (hiding warnings for the rest of the app it's integrated with). In our case, this code was hiding require cycles we had in our code which we wanted to be informed about. Either this should be a much more specific ignore (since the code talks about a fetch require cycle specifically), be functionality that can be opted out of when initialising the SDK, or it should be removed entirely.
For what it's worth, my recommendation is to remove it entirely. Metro has a resolver configuration option for ignoring specific require cycles and it defaults to ignoring everything under node_modules: https://metrobundler.dev/docs/configuration/#requirecycleignorepatterns. This has been present in Metro since 0.71.2: https://github.com/facebook/metro/commit/100ca6ffcc3ab99b82fac87b41be708040be2e9e. I believe the default ignore pattern would already be hiding any Sentry<->RN<->fetch require cycle.
Steps to reproduce: Create a require cycle between two files:
// a.ts
import {cycleB} from './b'
export const cycleA = 'abc'
console.log(cycleB)
// b.ts
import {cycleA} from './a'
export const cycleB = 'def'
console.log(cycleA)
Then import after Sentry.init call in your app.
Observe metro's console log while loading the app
Actual result:
No output about require cycle
Expected result:
WARN Require cycle: path/to/a.ts -> path/to/b.ts -> path/to/a.ts
Hey @liamjones - thank you for the detailed explanation and the reasoning, highly appreciated! I agree that this is not ideal and we will follow up here, but please be aware that due to PTOs and the holidays this will take a bit longer than usual.
Thanks for the quick reply @kahest. No worries about the delay - I will patch-package the code out for us for now. 👍
Hi @liamjones,
thank you for the info, I checked the metro version shipped with RN, RN 0.70 shipped with metro@72, the version before has metro@70.
I believe the best way forward is to remove https://github.com/getsentry/sentry-react-native/blob/c968ff930497b12ca8549cc000096c3c1395ab22/src/js/utils/ignorerequirecyclelogs.ts#L7 for RN 0.70 and newer. And keep it for older RN version 0.69 and older until the Sentry RN SDK removes support for the older version without the Metro require cycles patch.
@krystofwoldrich sounds sensible, 0.70 is already unsupported as far as RN is concerned: https://github.com/reactwg/react-native-releases#which-versions-are-currently-supported
From what I recall, LogBox.ignoreLogs also supports regexp ignores, so you might be able to tighten the ignore for older versions. Maybe it's not worth it if it only applies to unsupported versions of RN though...