Fix: Surface specific payment error from backend instead of generic fallback#85154
Fix: Surface specific payment error from backend instead of generic fallback#85154
Conversation
…allback When a payment fails (e.g., report exceeds $20,000 reimbursement limit), the backend sends a specific error message via Onyx. However, the client's failureData also sets a generic "Unexpected error. Please try again later." on the same reportAction. After Onyx deep-merges both errors, getLatestErrorMessageField() picks whichever has the latest timestamp key. By giving the client-side fallback error a timestamp of 0, backend errors (with real timestamps) always take precedence. The fallback still displays for network failures when no backend error is available. Fixed Issues: Expensify/Expensify#610370 Co-authored-by: Lydia Barclay <lydiabarclay@users.noreply.github.com>
|
The failing check is unrelated to this PR. Analysis: The Failure details:
Evidence: The only file changed in this PR is Re-running the failed job now. |
|
@MelvinBot can you pull |
|
I'd be happy to merge Currently:
To have me merge |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb Safari |
| value: { | ||
| [optimisticIOUReportAction.reportActionID]: { | ||
| errors: getMicroSecondOnyxErrorWithTranslationKey('iou.error.other'), | ||
| errors: getMicroSecondOnyxErrorWithTranslationKey('iou.error.other', 0), |
There was a problem hiding this comment.
@MelvinBot is this existing pattern in the app?
There was a problem hiding this comment.
@aimane-chnaif what does this PR do? I don't see how this would dix that error
Lines 42 to 50 in c4db6c4
There was a problem hiding this comment.
If 0 is passed, 0 ?? DateUtils.getMicroseconds() = 0 so it's saved in Onyx like this:
errors: {
0: "Unexpected error. Please try again later."
timestampe_BE: "backend error message"
}
Before fix:
errors: {
timestamp_FE: "Unexpected error. Please try again later."
timestamp_BE: "backend error message"
}
So as an edge of timestamp_FE > timestamp_BE (maybe local time is ahead of server time), app shows timestamp_FE value instead of timestamp_BE.
|
The function getMicroSecondOnyxErrorWithTranslationKey(error: TranslationPaths, errorKey?: number): Errors {
return {[errorKey ?? DateUtils.getMicroseconds()]: translateLocal(error)};
}There are several existing callers that pass a custom
However, passing hardcoded const key = Object.keys(errors).sort().reverse().at(0) ?? '';So a key of TL;DR: The |
Code ReviewVerdict: The change looks correct. The one-line fix is minimal, well-targeted, and works cleanly with the existing error infrastructure. Why it worksThe client-side fallback error now gets key Verified no edge cases
One observation (non-blocking)Five other payment-related flows in the same file use |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ 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". |
|
🎯 @aimane-chnaif, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #85480. |
trjExpensify
left a comment
There was a problem hiding this comment.
Makes sense from a product perspective to be more descriptive on the error. 👍
|
Friendly bump @grgia |
|
Reassigning since @grgia is OOO. |
|
@yuwenmemon 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] |
|
My mistake, I missed that Georgia commented, It'll have to wait till she's back. |
|
Georgia is out all week. Do we really need to wait when there's "comments" not "requested changes"? 🤔 |
TBH, I'm not really sure what to do here. Georgia did not request changes, but she didn't approve it either. So right now we have a MelvinBot coded PR, that I took over as an internal engineer, but changed nothing. We then had @aimane-chnaif test and review it, resulting in their approving it. Since I'll I did was pull main and do some testing of my own, I'm going to say my review and approval should push it over the "good enough" line and merge it. 😅 |
|
🚧 @deetergp has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/deetergp in version: 9.3.41-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|


Summary
When a payment fails (e.g., report exceeds the $20,000 reimbursement limit), the backend sends a specific, actionable error message via Onyx (e.g., "The reimbursement limit for a single report is $20,000.00. This report will need to be broken into smaller portions in order to be reimbursed."). However, the client-side
failureDataingetPayMoneyRequestParams()also sets a generic "Unexpected error. Please try again later." on the same reportAction.After Onyx deep-merges both error entries,
getLatestErrorMessageField()picks whichever has the latest timestamp key. Depending on client/server clock synchronization, the generic error may win over the specific backend error.Fix: Give the client-side fallback error a fixed timestamp of
0, so backend errors (with real timestamps) always take precedence ingetLatestErrorMessageField(). The fallback still displays for network failures when no backend error is available.Fixed Issues
$ https://github.com/Expensify/Expensify/issues/610370
PROPOSAL:
Tests
Verify that no errors appear in the JS console
Verified the existing
payMoneyRequesttest inIOUTest.tsis compatible — it checks the error value (Object.values(errors).at(0)), not the key, so passingerrorKey: 0does not break it.Scenario 1 (backend error): Backend returns specific error → errors object contains
{0: 'generic', <timestamp>: 'specific'}→getLatestErrorMessageField()picks the real timestamp (larger key) → specific error shown.Scenario 2 (network failure): No backend response → errors object contains only
{0: 'generic'}→ generic fallback shown as before.Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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
Co-authored-by: Lydia Barclay