fix(telemetry): drop untraceable Cannot call a class as a function noise from Sentry - #98866
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
fd637d6 to
873e845
Compare
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
dariusz-biela
left a comment
dariusz-biela
left a comment
There was a problem hiding this comment.
LGTM - the filter itself is verified: I ran the predicate against a raw production event (5caae171da694062b20387250ec7bf58, release 9.4.56-3, Safari 26.5 / Mac) and it matches, with frames identical to the test fixture across two different releases.
One thing to fix in the PR description before merge:
Test step 2 is not executable as written. Sentry is not exposed on window in this app: grepping src/ for window.Sentry / globalThis.Sentry returns nothing, and src/types/modules/window.d.ts:6-24 declares the full Window augmentation without it. A reviewer following the steps gets ReferenceError: Sentry is not defined. Either add a dev-only handle in src/setup/addUtilsToWindow.ts or rewrite steps 2-4.
cb8b40d to
ec5a4ab
Compare
|
@marufsharifi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
marufsharifi
left a comment
marufsharifi
left a comment
There was a problem hiding this comment.
Looks good, just one minor suggestion
|
@marufsharifi It should be ready! |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.61-0 🚀
|
|
🤖 No help site changes required — I did not create a draft PR. This PR adds a web-only Sentry integration (
Since there's no customer-facing behavior to document, there's nothing to update in the help site and no draft PR to open. @WojtekBoman, let me know if you'd like me to reconsider any specific behavior in this PR as customer-facing. |
|
@WojtekBoman We don't have access to Sentry. Could you please check this internally? Thank you |
|
@izarutskaya Sure! I'll check it out once it lands on prod :) |
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.4.61-4 🚀
Bundle Size Analysis (Sentry): |
|
I checked Sentry and the issue doesn't show up anymore on the version with the fix (9.4.61-4), the only events since the deploy are from users still on older cached bundles. I'll keep observing before resolving the issues 👀 |
7 participants
Explanation of Change
This adds a web-only
classCallCheckNoiseFilterIntegrationthat drops the untraceableTypeError: Cannot call a class as a functionnoise (APP-CN8 and siblings) coming from injected third-party scripts. It only discards an event when all three conditions hold: the message matches Babel's_classCallChecksignature, every stack frame is anonymous, and the event is already taggedthird_party_code. The integration must run afterthirdPartyErrorFilterIntegration, since it reads the tag that one sets - that ordering is pinned insetupSentryand covered by a dedicated unit test.Fixed Issues
$ #93837
PROPOSAL:
Tests
Run
npx jest tests/unit/classCallCheckNoiseFilterTest.ts tests/unit/setupSentryIntegrationOrderTest.tsand verify both suites pass. These cover the whole behavior of the change: the predicate (exact APP-JY0 frames dropped; named frame, missingthird_party_codetag, other messages, chained errors and transactions all kept) and the fact thatclassCallCheckNoiseFilterIntegrationis registered afterthirdPartyErrorFilterIntegrationinsetupSentry.There is no useful manual browser step for the drop itself: on the dev server
sentryWebpackPluginis not installed (config/rsbuild/rsbuild.common.ts:335), so__SENTRY_APPLICATION_KEY_STAMPED__is false,thirdPartyErrorFilterIntegrationisundefined, and thethird_party_codetag this filter keys on is never set. The event has to be hand-built to reach the predicate, which is exactly what the unit tests do.Start the web app (
npm run web), open it in Chrome, and open the Troubleshoot panel (Settings > Troubleshoot). Turn "Send data to Sentry" OFF and "Log Sentry to console" ON. Trigger a normal app error (e.g.throw new Error('sentry sanity check')in the console) and verify it is still logged as an outgoing[SENTRY]envelope, confirming no regression in regular error reporting.On Android/iOS native, launch the app and verify it boots with no JS console errors - the filter is stubbed to
undefinedon native and must not appear in the integration list.Offline tests
N/A - no API calls or Onyx operations; the change only filters events inside the Sentry SDK pipeline before transport.
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests (steps 1-2 are dev-only). On staging, verify the app loads on all platforms and that errors intentionally triggered in the app still appear in Sentry, while
third_party_code:TrueCannot call a class as a functionevents stop arriving for the staging release.PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari