[$1000] App crashed when revisiting thread from left room

[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!

---

Action Performed:

1. Create a new room
2. Send a message, create a thread with that message, pin that thread for later use.
3. Go back to our room and leave it then go back to the previous thread that you pinned.
4. Notice that app is crashed.

Expected Result:

App shouldn't be crashed

Actual Result:

App crashed when revisiting thread from left room.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.58-3

Reproducible in staging?: Y

Reproducible in production?: Y

If this was caught during regression testing, add the test name, ID and link from TestRail:

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-08-22.At.17.15.56.mp4

Recording.1369.mp4

Expensify/Expensify Issue URL:

Issue reported by: @hungvu193

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692703142297279

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011ed400862f0d676e
• Upwork Job ID: 1696683478876925952
• Last Price Increase: 2023-08-30

[OSBotify]

👋 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.

[melvin-bot]

Triggered auto assignment to @jasperhuangg (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot]

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[luacmartins]

This happens in prod, so NAB. This is the console error I'm seeing.

Thrown here:

[dummy-1111]

Proposal

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

App crashes when revisiting a thread from left room

What is the root cause of that problem?

When we leave the room, the API returns the following data, that is, clears the room report

When we revisit the chat thread after that, the originalReport becomes null below

App/src/pages/home/report/ReportActionItem.js

Lines 136 to 137 in cb98bec

	const originalReportID = ReportUtils.getOriginalReportID(props.report.reportID, props.action);
	const originalReport = props.report.reportID === originalReportID ? props.report : ReportUtils.getReport(originalReportID);

When rendering mini context menu, we call ReportUtils.chatIncludesChronos below

App/src/pages/home/report/ReportActionItem.js

Line 565 in cb98bec

isChronosReport={ReportUtils.chatIncludesChronos(originalReport)}

and chatIncludesChronos doesn't check the validity of the report as you can see below

App/src/libs/ReportUtils.js

Lines 2895 to 2897 in cb98bec

	function chatIncludesChronos(report) {
	return report.participantAccountIDs && _.contains(report.participantAccountIDs, CONST.ACCOUNT_ID.CHRONOS);
	}

This is the root cause

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

We can use lodashGet in chatIncludesChronos function instead of referring the property directly

function chatIncludesChronos(report) {
    const participants = lodashGet(report, ['participantAccountIDs']);
    return participants && _.contains(participants, CONST.ACCOUNT_ID.CHRONOS);
}

This works as expected

What alternative solutions did you explore? (Optional)

[hungvu193]

Post the proposal where I'm the reporter, I think @izarutskaya forgot to post it 🤦

Proposal

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

App crashed when revisiting thread from left room.

What is the root cause of that problem?

When leave a room, we merge our report to null here:

App/src/libs/actions/Report.js

Line 1812 in 893c2a5

value: _.object(reportKeys, Array(reportKeys.length).fill(null)),

Later when we use it here, our originalReport will be temporary null.

App/src/pages/home/report/ReportActionItem.js

Line 135 in 893c2a5

const originalReport = props.report.reportID === originalReportID ? props.report : ReportUtils.getReport(originalReportID);

It will lead the App to crash here:

App/src/libs/ReportUtils.js

Lines 2734 to 2736 in 893c2a5

	function chatIncludesChronos(report) {
	return report.participantAccountIDs && _.contains(report.participantAccountIDs, CONST.ACCOUNT_ID.CHRONOS);
	}

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

We can use lodashGetto get the value of participantAccountIDs inside chatIncludesChronos function.

const participantAccountIDs = lodashGet(report, 'participantAccountIDs', []);
// then use `participantAccountIDs` after that.

Or we can add more condition here to early return if report is null:

return report && report.participantAccountIDs && _.contains(report.participantAccountIDs, CONST.ACCOUNT_ID.CHRONOS);

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot]

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

[melvin-bot]

Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[robertKozik]

Hi all @s-alves10 @hungvu193 ! I wanted to check your proposals but I have problems with reproduction. Could you check on your end that it's still reproducible?

Screen.Recording.2023-08-31.at.13.40.44.mov

[kbecciv]

@hungvu193 No worries about your proposal.
@CortneyOfstad @luacmartins - he is reporter of the issue and his proposal - #26256 (comment)

[robertKozik]

Thanks for confirming @kbecciv 🙇🏼‍♂️

[hoangzinh]

I could not reproduce the bug too.

[CortneyOfstad]

@hungvu193 Are you still able to reproduce this?

[hungvu193]

I also couldn't reproduce

[melvin-bot]

@CortneyOfstad, @luacmartins, @robertKozik Whoops! This issue is 2 days overdue. Let's get this updated quick!

[CortneyOfstad]

@luacmartins @robertKozik Since this doesn't seem to be reproducible, good to close?

[luacmartins]

I think if we can't reliably reproduce this we should just close the issue

[dummy-1111]

For a few mins, please

[dummy-1111]

Yes. I wasn't able to reproduce this. Sorry

[luacmartins]

Ok, gonna close this issue. Thanks for the discussion!