Workspace - App crashed when send attachment offline and delete workspace from other device

[izarutskaya]

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: 1.4.70-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4538783&group_by=cases:section_id&group_order=asc&group_id=306202
Email or phone of affected tester (no customers): applausetester+omq76@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Open app
2. Create a workspace
3. Navigate to announce room
4. Send any message to the room
5. Disable internet connection
6. Send an image attachment to the room
7. Sign in with the same account on other device and delete recently created workspace
8. Enable internet connection on main device

Expected Result:

Error should be displayed

Actual Result:

App crashed when user sent attachment offline, deleted workspace from other device and returned online on main device

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6469558_1714696989845.RPReplay_Final1714696916.mp4

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @iwiznia (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

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

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[izarutskaya]

@lschurr I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[izarutskaya]

We think this issue might be related to the #collect project.

[izarutskaya]

Production

RPReplay_Final1714737209.MP4

[suneox]

Proposal

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

Crash issue no longer happens on latest main, but we have another issue is loading infinity

Screen.Recording.2024-05-03.at.19.42.10.mov

What is the root cause of that problem?

We have 2 issues:

Issue 1: After delete workspace the function loadOlderChats has trigger call infinity due to the report action is empty and FlatList trigger call onEndReached

Issue 2: At this function we have filter out actionName = 'CREATE' and at this function we also filter out actionName = 'CLOSED' so sortedVisibleReportActions is empty and the UI is missing info

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

Fix issue 1:
We should prevent trigger call onEndReached={loadOlderChats} when sortedVisibleReportActions is empty

      onEndReached={() => {
          if (!sortedVisibleReportActions.length || isLoadingOlderReportActions) {
              return;
          }
          loadOlderChats();
      }}

Fix issue 2:
We will update reportActions when workspace is archived by get all reportActions

+   const isArchivedRoom = ReportUtils.isArchivedRoom(report);
    const reportActions = useMemo(() => {
        if (!sortedAllReportActions.length) {
            return [];
        }
+       const filteredReportActions = ReportActionsUtils.getContinuousReportActionChain(sortedAllReportActions, reportActionIDFromRoute);
+       if (isArchivedRoom) {
+           return sortedAllReportActions;
+       };
+       return filteredReportActions;
    }, [isArchivedRoom, reportActionIDFromRoute, sortedAllReportActions]);

Test branch

What alternative solutions did you explore? (Optional)

Update getContinuousReportActionChain function when isArchivedRoom exclude actionName = 'CREATE'

[iwiznia]

I don't think that solution makes sense. You should be able to go back and load all the older messages from an archived chat and the solution proposed will break that, no?

Did anyone find the actual code that makes it crash?

[iwiznia]

@izarutskaya why is there no desktop video? Also, desktop can't crash, so not sure what is happening in desktop...
Is it also not working on web? If so, can you share console logs?

[iwiznia]

ok did the reproduction steps and not seeing a crash in web, seeing the loading older action issues described above (this is on staging, not main).

[iwiznia]

Posted in slack here https://expensify.slack.com/archives/C01GTK53T8Q/p1714747609951879

[iwiznia]

I can reproduce the same issue of infinite loading in android, but not a crash. Don't have an ios to test the crash there.

[iwiznia]

@izarutskaya I don't see any email in the issue. You should always include the email and other relevant data in the issue report (such as workspaceID, reportID, etc). That way I can retrieve logs for cases where I can't reproduce the issue like this one.

[iwiznia]

@suneox did you see this?

I don't think that solution makes sense. You should be able to go back and load all the older messages from an archived chat and the solution proposed will break that, no?

[janicduplessis]

I think this might be fixed by #41558. Would someone be able to test out that PR?

My other theory is that the chat doesn't have a CREATED action as the last one for some reason so we don't know that we are at the end of the chat (see https://github.com/janicduplessis/Expensify-App/blob/10fbd6a53177685702bdfa90e102163506e263cf/src/pages/home/report/ReportActionsView.tsx#L320-L321). I have a proposal to have the backend return whether more data is available but it is more of a medium term fix (#41153).

[suneox]

I don't think that solution makes sense. You should be able to go back and load all the older messages from an archived chat and the solution proposed will break that, no?

Proposal update

@iwiznia I have update proposal to handle load all the older messages and prevent infinity trigger getOlderActions with demo

[iwiznia]

•   if (isArchivedRoom && filteredReportActions.length === 1 && filteredReportActions[0].actionName === CONST.REPORT.ACTIONS.TYPE.CLOSED) {
  

I think this part is wrong as it only works if the actions is of length 1 but if the other app adds a comment before deleting the workspace I still see the broken behavior.
I also don't quite understand what that code is doing exactly.

[suneox]

I think this part is wrong as it only works if the actions is of length 1 but if the other app adds a comment before deleting the workspace I still see the broken behavior. I also don't quite understand what that code is doing exactly.

This update is specific for this issue to avoid regression, so if we still confuse we can check by condition

     if (isArchivedRoom) { return sortedAllReportActions }

[janicduplessis]

Proposal

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

GetOlderActions is called in a loop and new messages are never loaded.

What is the root cause of that problem?

There is a bug in getContinuousReportActionChain where if there is an optimistic action in the middle of a chat we incorrectly detect a gap in the chat. This causes a smaller array to be returned and then this incorrect array is used for pagination.

New messages loaded by GetOlderActions are never included in the chat because of the incorrect gap so we keep calling GetOlderActions in a loop since we never reach the CREATED action, which is the first message in all chats.

For more context and debugging of this problem you can read through this chat in Slack.

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

We need to fix getContinuousReportActionChain to properly ignore optimistic actions since the backend does not know about them and previousReportActionID will be wrong.

What alternative solutions did you explore? (Optional)

[janicduplessis]

I already have a PR ready for the fix since I think this is a deploy blocker #41644, this is based on @roryabraham 's test fix #41598

[suneox]

Proposal updated

Update RCA explanation and simplify logic fix issue

[izarutskaya]

@iwiznia Tester login credentials (email) - applausetester+omq76@applause.expensifail.com.
Let me please include a console logs..

[roryabraham]

nice, let's move forward with #41644, that looks correct. I kicked off an AdHoc build of that so we can test.

[roryabraham]

Verified that this is fixed and we didn't use any C+ services so I'm closing this out