Fix: include 1:1 DM participants in OpenReport so stale optimistic reportIDs self-heal - #98855
Conversation
…Ds self-heal When a cached DM is opened by reportID only (LHN, search router, deep link), OpenReport was sent with empty emailList/accountIDList. If the cached reportID was a stale optimistic one, the server had nothing to resolve against and returned 403 "Report not found". Deriving the participant list from the cached report lets the server resolve the real chat and return preexistingReportID, which the existing reconciliation middleware already handles.
Covers: deriving emailList/accountIDList from a cached 1:1 DM, leaving them empty for non-DM reports, and preserving caller-provided participants.
…m-participants-selfheal # Conflicts: # src/libs/actions/Report/index.ts
|
@hoangzinh 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
| const participantAccountIDList = participants.map((p) => p.accountID).filter((id): id is number => id !== undefined); | ||
| const existingReportName = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportName; | ||
| let participantAccountIDList = participants.map((p) => p.accountID).filter((id): id is number => id !== undefined); | ||
| const existingReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; |
There was a problem hiding this comment.
I know we previously used allReports to obtain the report name, but since allReports will be deprecated soon, we should avoid using it to retrieve participants. Do you know why the participants argument of openReport is empty?
There was a problem hiding this comment.
only new-chat creation flows had participant info; fetch-by-reportID paths never did — now ReportFetchHandler derives it from its own useOnyx data.
I fixed the other comment related to allReports
There was a problem hiding this comment.
@elirangoshen, do you know why it has been happening recently?
There was a problem hiding this comment.
Yes — and it turns out to be very recent and very sharp. I pulled the backend log trend for this exact failure (401 Unauthorized thrown from Report.cpp while processing OpenReport, which is what the client renders as the not-found page):
- Zero occurrences before 2026-07-23. Not one, going back through June.
- On Jul 23 it jumps to ~3.2k/day and has been sustained at ~4–6k/day on weekdays ever since (~91k total in 4 weeks). The weekend dips track business usage.
- I also checked the week before Jul 23 for any
401exception onOpenReportunder any message shape/file, in case a refactor just moved the throw — nothing. This failure class genuinely did not exist before that date.
So the mechanism (stale optimistic reportIDs from lost reconciling writes) is old, but something that deployed around Jul 22–23 — on the Auth side (the access check throwing at Report.cpp:17792) or in the App version that hit production that day — flipped this from theoretical to ~5k/day. Recent queue/auth bugs like the stale-token 407 drops discussed in #98314 likely feed the stale-ID population, but the hard on/off edge points at a specific deploy. Worth someone with Auth access diffing what changed around that throw in the Jul-23 deploy.
Either way, the fix here is deploy-agnostic: it gives the server what it needs to self-heal regardless of which producer created the stale ID.
There was a problem hiding this comment.
@elirangoshen can you test again on this PR? I can see emailList, but accountIDList is still empty
Per review feedback: openReport no longer reads allReports/allPersonalDetails (both slated for deprecation). A pure ReportUtils.getOneOnOneChatParticipants helper derives the other DM participant from data the component already subscribes to via useOnyx, and ReportFetchHandler passes it through the existing participants param.
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
the server had nothing to resolve against and returned 403 Report not found, showing the user a "not found" screen. Production logs show ~900 such failures per day across ~470 users.
Wowza, nice. 👍
| } | ||
| return Object.keys(report?.participants ?? {}) | ||
| .map(Number) | ||
| .filter((accountID) => accountID !== currentUserAccountID) |
There was a problem hiding this comment.
Do we need to exclude currentUserAccountID?
3 participants
Explanation of Change
When a 1:1 DM is opened by reportID alone (chat switcher, LHN, deep link), the
OpenReportAPI call was sent with emptyemailList/accountIDList. If the cached reportID was a stale optimistic one that was never reconciled to the real report (the request that would have reconciled it was lost — e.g. dropped connection or the app was closed), the server had nothing to resolve against and returned403 Report not found, showing the user a "not found" screen. Production logs show ~900 such failures per day across ~470 users.The server already self-heals a bad/optimistic reportID when the request includes the DM participant: it looks up the real DM between the two users and returns it as
preexistingReportID. The client already handles that response end-to-end —HandleUnusedOptimisticIDmiddleware rewrites queued requests, andreplaceOptimisticReportWithActualReportswaps navigation to the real report and deletes the stale optimistic report from Onyx.This PR supplies the missing piece: in
openReport, when the caller passed no participants and the cached report is a 1:1 DM, deriveaccountIDListfrom the cached report's participants (excluding the current user) andemailListfrom personal details, and include them in the API call. The change is deliberately scoped to 1:1 DMs, where the participant pair uniquely identifies the chat — rooms, threads, group chats, and expense reports are untouched. When the caller passes participants explicitly (new-chat flows), those are kept as-is.Fixed Issues
$ #98765
Tests
OpenReportand open another existing 1:1 DM.emailListand their accountID inaccountIDList.OpenReportrequests still send emptyemailList/accountIDList.Automated coverage:
tests/actions/ReportTest.ts→describe('openReport participants self-heal')— derives participants for a cached 1:1 DM, leaves them empty for non-DM reports, and preserves caller-provided participants.Offline tests
QA Steps
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