[$250] Accounting - Accounting tab loads infinitely when the workspace is created offline

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.57-3 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+kh22100010ad@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go offline.
3. Create a new workspace.
4. Go to workspace settings > More features.
5. Enable Accounting.
6. Go to Accounting.
7. Go online.

Expected Result:

Accounting tab will load after returning online.

Actual Result:

Accounting tab loads infinitely when the workspace is created offline.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/f980462e-17cd-401f-8f59-f5adf93155ec

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021853787863203385081
• Upwork Job ID: 1853787863203385081
• Last Price Increase: 2024-11-05

Issue Owner

Current Issue Owner: @ntdiary

[melvin-bot[bot]]

Triggered auto assignment to @slafortune (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.

[bernhardoj]

Proposal

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

Infinity loading in accounting page if creating the workspace while offline.

What is the root cause of that problem?

The loading will show if the condition below is met. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/pages/workspace/withPolicyConnections.tsx#L58-L60

The one that we need to focus on is hasConnectionsDataBeenFetched. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/pages/workspace/withPolicyConnections.tsx#L32-L35

The hasConnectionsDataBeenFetched will become true if the OPEN_POLICY_ACCOUNTING_PAGE read is completed. We request it only while online. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/actions/PolicyConnections.ts#L8-L33

When we make a read request, it will wait for all write requests to complete first, by using the isReadyPromise. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/API/index.ts#L170-L179 https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L270-L272

However, in our case, the promise is never resolved even after all write requests are completed. That's because, while waiting for the promise to be resolved, the promise is recreated, so the old one is never resolved. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L155-L158

It's recreated every time flush is called. After the flush is called, isSequentialQueueRunning is set to true which prevents any subsequent flush from being processed when there is one already running. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L153 https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L136-L139

But in our case, isSequentialQueueRunning becomes false. That's because when we create the workspace and enable the accounting while offline, the BE will return shouldPauseQueue for EnablePolicyConnections which pause the queue. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L95-L98

When it processes the next request, the promise is resolved because the queue is paused, https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L68-L71

which sets isSequentialQueueRunning to false. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L168-L170

When the queue is unpaused, we call flush again. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L188-L198

And because isSequentialQueueRunning is false, the isReadyPromise is recreated.

flush -> process (CreateWorkspace) -> process (EnablePolicyConnections) -> pause -> GetMissingOnyxMessages -> unpause -> flush (new isReadyPromise) -> process (ReconnectApp)

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

We shouldn't reset the isReadyPromise when unpausing the queue. To do that, we can accept a new param for flush called resume. If it's true, then don't recreate isReadyPromise.

if (!resume) {
    // Reset the isReadyPromise so that the queue will be flushed as soon as the request is finished
    isReadyPromise = new Promise((resolve) => {
        resolveIsReadyPromise = resolve;
    });
}

And pass true when flushing from unpause. https://github.com/Expensify/App/blob/b1bffa69da157ef7c33e7c6f110b697687fcbd71/src/libs/Network/SequentialQueue.ts#L188-L198

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ntdiary]

Under review.

still verifying the RCA, will provide an update tomorrow.

[ntdiary]

@bernhardoj's RCA is accurate, with only a slight difference from what I checked. :) pause is called here (in saveUpdateInformation), and shouldPauseQueue is also set to true here: https://github.com/Expensify/App/blob/e453cfd324396fc81b42b234aece97ebeb1e2246/src/libs/Middleware/SaveResponseInOnyx.ts#L38-L45

---

BTW, just a note, in our current SequentialQueue, we don’t have an exact concept of "current batch" https://github.com/Expensify/App/blob/e000d80e5310a09997c94c5e57ac600803250d31/src/libs/Network/SequentialQueue.ts#L251-L257 we just ensure that all requests in persistedRequests are executed in order by calling flush in multiple places.

[ntdiary]

Additionally, @bernhardoj, just a small question: since our code calls unpause in several places, do you think adding this parameter will be safe enough to avoid causing regressions? 😂

[bernhardoj]

I think it should be safe. unpause will only take effect if the queue is currently paused. I think in all cases where pause and unpause happens, we don't want to have a new isReadyPromise because it's still in one "batch".

[ntdiary]

Nice, I think it's fine to move forward with @bernhardoj's proposal. BTW, just a tiny thought, I'm not entirely sure if resume is clear and accurate enough (my English isn’t good), maybe can use shouldResetPromise(default value is true). Let's see if the internal engineer has any other thoughts. :)

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @srikarparsi, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot[bot]]

@ntdiary, @slafortune, @srikarparsi Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[srikarparsi]

Will look at this tomorrow

[melvin-bot[bot]]

@ntdiary, @slafortune, @srikarparsi Whoops! This issue is 2 days overdue. Let's get this updated quick!

[srikarparsi]

I agree with the RCA and the proposed solution, lets move forward with @bernhardoj's proposal. I agree that shouldResetPromise is better than resume. One thing I'm not completely sure about is this comment, maybe @danieldoglas does this sound right to you? Saw that you worked on some of the SequentialQueue logic.

[danieldoglas]

thanks for that investigation @bernhardoj !

I agree that it looks a lot like an issue in the frontend, but I think it's an issue in the backend. If you're doing 2 sequential writes and constantly finding a gap between the requests (CreateWorkspace -> EnablePolicyConnections) then that means that the first request is not returning all the data the app needs.

@srikarparsi I recommend you try this flow locally, and find out which onyxUpdates were not returned, and check the backend code to confirm if they're being sent back on the HTTPs request before we start anything in the front end here.

[melvin-bot[bot]]

@ntdiary, @slafortune, @srikarparsi Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

@ntdiary, @slafortune, @srikarparsi 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[melvin-bot[bot]]

@ntdiary @slafortune @srikarparsi this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[ntdiary]

@srikarparsi I recommend you try this flow locally, and find out which onyxUpdates were not returned, and check the backend code to confirm if they're being sent back on the HTTPs request before we start anything in the front end here.

It seems to be waiting for some investigation results from the backend. :)