[Due for payment 2025-04-23] [$250] Create report - No error is shown and the composer is presented after turning on failing request

[vincdargento]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Issue was found while executing QA for PR #59386

Version Number: 9.1.24-4
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: #59386
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: MacBook Air 15.4 Chrome, iPhone 15 iOS 8.3.2 Safari
App Component: Other

Action Performed:

1. Navigate to the staging.new.expensify.com and sign in with expensifail account
2. Create a workspace, if needed
3. Turn on "Simulate failing network requests"
4. Open Reports tab
5. Click on the green plus button and select Create report
6. Observe the page

Expected Result:

The composer is gone and the error shows up when trying to create expense report with the failing requests turned on

Actual Result:

The composer is still presented and the error is not shown when trying to create expense report with the failing requests turned on

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021909791535722529701
• Upwork Job ID: 1909791535722529701
• Last Price Increase: 2025-04-09
• Automatic offers:
• nkdengineer | Contributor | 106854604

Issue Owner

Current Issue Owner: @sakluger

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021909791535722529701

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @allroundexperts (External)

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The composer is still presented and the error is not shown when trying to create expense report with the failing requests turned on

What is the root cause of that problem?

Currently, it's not a bug, the request is retried 10 times before applying the failureData. But I think we can improve the to not run the retry logic if we're Simulate failing network requests and merge the failureData immediately

App/src/libs/Network/SequentialQueue.ts

Lines 117 to 120 in 563386f

	if (error.name === CONST.ERROR.REQUEST_CANCELLED || error.message === CONST.ERROR.DUPLICATE_RECORD) {
	Log.info("[SequentialQueue] Removing persisted request because it failed and doesn't need to be retried.", false, {error, request: requestToProcess});
	endPersistedRequestAndRemoveFromQueue(requestToProcess);
	sequentialQueueRequestThrottle.clear();

What changes do you think we should make in order to solve the problem?

Update the logic to merge the failureData immediately if we force fail network requests. With that, we can see the fail data faster on the testing purpose with Simulate failing network requests

let shouldFailAllRequests: boolean;
Onyx.connect({
    key: ONYXKEYS.NETWORK,
    callback: (network) => {
        if (!network) {
            return;
        }
        shouldFailAllRequests = !!network.shouldFailAllRequests;
    },
});

if (error.name === CONST.ERROR.REQUEST_CANCELLED || error.message === CONST.ERROR.DUPLICATE_RECORD || shouldFailAllRequests) {
    if (shouldFailAllRequests) {
        Onyx.update(requestToProcess.failureData ?? []);
    }
    ...

App/src/libs/Network/SequentialQueue.ts

Lines 117 to 120 in 563386f

	if (error.name === CONST.ERROR.REQUEST_CANCELLED || error.message === CONST.ERROR.DUPLICATE_RECORD) {
	Log.info("[SequentialQueue] Removing persisted request because it failed and doesn't need to be retried.", false, {error, request: requestToProcess});
	endPersistedRequestAndRemoveFromQueue(requestToProcess);
	sequentialQueueRequestThrottle.clear();

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can add a test case in SequentialQueueTest with the setting Simulate failing network requests and verify that the request is only called once.

What alternative solutions did you explore? (Optional)

Result

Screen.Recording.2025-04-09.at.09.50.16.mov

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[SzymczakJ]

This is not a bug, failing requests setting takes really long time to apply failure state. Perhaps some day we should make this faster. Getting this failure state took me at least two full minutes. I'm posting a trimmed video(split into two parts) as a proof.

part1.mov

partlast.mov

cc @mountiny

[nkdengineer]

@SzymczakJ Yes, currently, we're still running the retry logic if Simulate failing network requests is enabled. What do you think about my idea here?

Or I think we can reduce the request wait time. It's increased x2 for the next request. Then for the last retry request the time x1024.

App/src/libs/RequestThrottle.ts

Line 32 in cd227b6

this.requestWaitTime = Math.min(this.requestWaitTime * 2, CONST.NETWORK.MAX_RETRY_WAIT_TIME_MS);

[SzymczakJ]

It's a troubleshoot option, I'm not sure, if we want to spend our time on changing something like this right now, as this doesn't give any value to the user. Issue owner should make this decision.

What I think about proposed solution:

Or I think we can reduce the request wait time. It's increased x2 for the next request.

I think it's not a good idea to reduce wait time as It's a exponential backoff algorithm and it's common solution for retrying failing requests.

I liked your previous idea with merging the failureData immediately better.

[mountiny]

Yeah I am not sure if this is a bug really, this is not representative of this request actually failing

[mountiny]

Noting that in the BE this caused bunch of errors as the same request with the same params was used causing the subsequent calls to fail as the same IDs cannot be inserted the db obviously

[nkdengineer]

@mountiny As I mentioned above we can prevent the retry logic with the failing setting then we can prevent the thing you mentioned and also that will improve the testing experience.

[mountiny]

@nkdengineer if this settings is not on and the CreateAppReport call fails, we would retry 10times for the normal user? I dont think so right?

[nkdengineer]

@nkdengineer if this settings is not on and the CreateAppReport call fails, we would retry 10times for the normal user? I dont think so right?

@mountiny If this setting is not on, the API fails from the backend (that means this problem will not happens ), and we will do the retry logic normally. If all of them still fail, the failureData will be applied. Otherwise, if one of them is a success, the logic is skipped. Not sure what is wrong here

[mountiny]

Ok then your change makes sense to me, it does not make much sense to me to retry those failing requests

[melvin-bot]

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.1.28-15-staging and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• No error is shown and the composer is presented after turning on failing request #59977

If no regressions arise, payment will be issued on 2025-04-23. 🎊

For reference, here are some details about the assignees on this issue:

• @allroundexperts requires payment through NewDot Manual Requests
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

@allroundexperts @kadiealexander @allroundexperts The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[kadiealexander]

Reassigning to handle payment as I'm heading OOO. Thanks for your help @sakluger!

[mountiny]

$250 to @nkdengineer and to @allroundexperts

[allroundexperts]

I guess no checklist is needed here as this was an improvement and not a bug.

[mountiny]

Yeah all good, I think no tests required here either

[sakluger]

Payment Summary

Contributor: @nkdengineer paid $250 via Upwork
Contributor+: @allroundexperts due $250 via NewDot