fix: prevent side panel report live tail from crashing on setParams - #99533
fix: prevent side panel report live tail from crashing on setParams#99533WojtekBoman wants to merge 3 commits into
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
| isFocused: () => true, | ||
| addListener: () => () => {}, | ||
| removeListener: () => () => {}, | ||
| setParams: () => logInertCall('setParams'), |
There was a problem hiding this comment.
I wonder if we should avoid making these functions silent noops. Previously, the crashes actually helped us catch when something wasn't right. I just worry this might introduce silent bugs that are harder to track down. What do you think?
There was a problem hiding this comment.
I've added it to mechanism that forwards logs to sentry, I believe it will help in this case!
85d8df2 to
79384aa
Compare
|
@mananjadhav 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] |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safariweb-set-params-1.movweb-set-params-2.mov |
Explanation of Change
Concierge Anywhere on web renders the report in the side panel, which sits outside any navigator screen, so
ScreenWrapper(exported aswithNavigationFallback(ScreenWrapper)) hands the whole subtree a stub navigation object exposing onlyisFocused/addListener/removeListener.useReportActionsNewActionLiveTailruns inside that subtree and callednavigation.setParams({reportActionID: ''}), which wasundefinedon the stub - aTypeErrorinside a passive effect, so the error boundary showed a crash screen. Native is unaffected (isSidePanelReportSupportedisfalse), matching the zero native events in Sentry.Two changes, each individually sufficient; both kept on purpose:
useReportActionsNewActionLiveTail.ts- use the route-targeted global dispatch already used in this tree (Navigation.setParams({reportActionID: ''}, route.key, navigatorKey), same asLinkedActionNotFoundGuard). It goes throughnavigationRef, which is always real, and targets this route rather than the focused one, preserving the existing behavior of not clobbering an open RHP's params. The dispatch is skipped in the side panel, whose synthetic route carries noreportActionIDto clear. Only the dispatch is skipped - the rest of the jump still runs, so the side-panel live tail still scrolls to the newest message.withNavigationFallback.tsx- harden the stub with the rest of the surface used underScreenWrapperso descendants degrade instead of throwing. Action-shaped no-ops log viaLog.hmmm, since this HOC also wrapsButtonandButtonKeyboardShortcutand a bare no-op would turn a loud crash into a silently dead button.Unit tests cover the route-targeted dispatch, the side-panel skip, the fallback-stub path not throwing, and the stub's callable surface.
Before:
Screen.Recording.2026-08-26.at.10.55.51.mov
After:
Screen.Recording.2026-08-26.at.10.59.49.mov
Fixed Issues
$ #99517
PROPOSAL:
Tests
setParams is not a functionin the console.Regression check on real navigator screens:
/r/<reportID>/<reportActionID>) in the central pane.reportActionIDfrom the report route - the URL drops back to/r/<reportID>- while the RHP stays open with its params untouched.Negative control (confirms the crash was side-panel-specific, not Concierge-specific):
Offline tests
The live-tail jump is gated on
!isOffline, so none of this runs while offline.QA Steps
Same as tests. Web only - the side panel report is not supported on native, so there is nothing to verify on iOS or Android native.
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
N/A - the side panel report is not supported on native (
isSidePanelReportSupportedisfalse).Android: mWeb Chrome
iOS: Native
N/A - the side panel report is not supported on native (
isSidePanelReportSupportedisfalse).iOS: mWeb Safari
MacOS: Chrome / Safari