[Awaiting C+ payment] [$1000] App crashes on login after opening split bill details page

[kavimuru]

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

---

Action Performed:

1. Open the app
2. Click on plus and click on split bill
3. Enter any amount and continue
4. Select any 1 user and complete the split bill process
5. In the user report, click on split bill message to open split bill details page
6. Copy the URL, logout, paste the URL and login again, eg URL format: https://staging.new.expensify.com/r/5686244339424710/split/2464435729537161526
7. Observe that app crashes

Expected Result:

App should not crash when we open split bill details page

Actual Result:

App crashes when we logout and login with split bill details page

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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: 1.3.45-2
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

crash.on.split.bill.URL.open.mp4

Recording.1336.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690262928105189

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0168304d5ea41d503d
• Upwork Job ID: 1684650192147705856
• Last Price Increase: 2023-07-27
• Automatic offers:
• dukenv0307 | Contributor | 25865336
• dhanashree-sawant | Reporter | 25865338

[melvin-bot]

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

[dukenv0307]

Proposal

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

App crashes on login after opening split bill details page

What is the root cause of that problem?

When we open split bill page by deep link, we use HOC withReportOrNotFound to display loading page if openApp API is calling

But after this API is complete, props.reportActions still is empty because it must wait openReport API is complete. That makes reportAction is empty and the app crashes when we get participantAccountIDs from reportAction

App/src/pages/iou/SplitBillDetailsPage.js

Lines 66 to 70 in 031b4a3

	const reportAction = props.reportActions[`${props.route.params.reportActionID.toString()}`];
	const participantAccountIDs = reportAction.originalMessage.participantAccountIDs;
	const participants = OptionsListUtils.getParticipantsOptions(
	_.map(participantAccountIDs, (accountID) => ({accountID, selected: true})),
	props.personalDetails,

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

We should display loading page until both report data and report action data are loaded completely

1. Remove the HOC withReportOrNotFound, subscribe report and isLoadingReportData from Onyx

2. We should add the check to return loading page before this line

const isLoadingInitialReport = props.isLoadingReportData && _.isEmpty(props.report);
const isLoadingInitialReportActions = _.isEmpty(props.reportActions) || (props.report.isLoadingReportActions && _.isEmpty(reportAction);
const isExistReport = isLoadingInitialReport || (!_.isEmpty(props.report) && props.report.reportID);

const shouldShowLoading = (isLoadingInitialReport || isLoadingInitialReportActions) && isExistReport;
if (shouldShowLoading) {
    return <FullscreenLoadingIndicator />;
}

App/src/pages/iou/SplitBillDetailsPage.js

Line 67 in 031b4a3

const participantAccountIDs = reportAction.originalMessage.participantAccountIDs;

3. We also should bring the not found page before this line because with reportActionID is invalid or not found , App also crashes

if (_.isEmpty(reportAction) || _.isEmpty(props.report)) {
    return (
        <FullPageNotFoundView shouldShow/>
    )
}

App/src/pages/iou/SplitBillDetailsPage.js

Line 67 in 031b4a3

const participantAccountIDs = reportAction.originalMessage.participantAccountIDs;

What alternative solutions did you explore? (Optional)

NA

Result

Screencast-from-26-07-2023-01_38_16.mp4

[trjExpensify]

👋 @dukenv0307 how related is this to this issue? CC: @aldo-expensify

[dukenv0307]

@trjExpensify This issue and #21268 are different issue because the root cause is different.

[aldo-expensify]

If the root cause @dukenv0307 explained here is correct, they do appear unrelated to me.

[trjExpensify]

Cool, will move on external for now! :)

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[gulxarahmad]

Root cause of the problem:
The root cause of the app crash needs to be investigated further to provide a definitive answer. It could be due to a variety of reasons, such as a bug in the React Native code, compatibility issues between different platforms, memory management problems, or even server-related issues.

Changes to solve the problem:
To address the app crash issue, the following steps can be taken:

Bug Investigation: Start by thoroughly investigating the crash logs and error messages reported by the app. Identify patterns or common scenarios that lead to the crash. This will help in pinpointing the cause of the issue.

Reproduce the Bug: Try to reproduce the crash on different devices and platforms (iOS and Android) using the same version of the app. This step is essential to ensure the bug is replicable and consistent across devices.

Inspect Split Bill Details Page: Examine the Split Bill Details page to check if there are any specific elements or interactions triggering the crash. Review the React Native code associated with this page to identify potential issues.

Memory Management: Analyze the memory usage of the app during the login process after opening the Split Bill Details page. Check for any memory leaks or excessive memory usage that could lead to crashes, especially if the user navigates between different pages frequently.

Unit Testing: Write unit tests to cover the critical parts of the code related to the login process and the Split Bill Details page. These tests will help catch regressions and ensure the stability of the app.

React Native Version and Dependencies: Ensure that the React Native version used is stable and compatible with the required dependencies. Consider upgrading to the latest stable version if it addresses any known issues.

Error Handling: Implement proper error handling mechanisms throughout the app to gracefully handle unexpected situations and prevent crashes.

Platform-Specific Optimization: If the crash is specific to a particular platform (iOS or Android), perform platform-specific optimizations and debugging to resolve the issue.

Beta Testing: Once the proposed changes are implemented, conduct beta testing with a group of users to validate the fix and gather feedback.

Remember that the actual solution will require a more detailed analysis of the codebase and the crash reports. The proposal should outline the general approach and steps to tackle the problem effectively.

[mananjadhav]

Thanks for the proposal @dukenv0307. Your RCA is correct, but I have a few questions on the proposal.

1. Why do we want to remove withReportOrNotFound page?

We should add the check to return loading page before this line

2. which line?

3. Can we simplify the proposal because the isLoadingReportingData, etc. checks are already in the withReportOrNotFound component?

[dukenv0307]

@mananjadhav

Why do we want to remove withReportOrNotFound page?

We should remove this HOC because we should show loading until both report and report action data are loaded completely.

which line?

This line we should add the condition after this line and also return NotFoundPage here because if reportAction make the get data after this line crash app

App/src/pages/iou/SplitBillDetailsPage.js

Line 66 in 031b4a3

const reportAction = props.reportActions[`${props.route.params.reportActionID.toString()}`];

Can we simplify the proposal because the isLoadingReportingData, etc. checks are already in the withReportOrNotFound component?

This HOC is used in other places and this check in this isn't enough because report data is loaded in openApp API, reportAction data is loaded in openReport API, and isLoadingReportingData will be false after openApp API complete

[mananjadhav]

Thanks for the response, @dukenv0307. I meant why not just add checks for reportAction. Why remove the withReportOrNotFound hoc?

[dukenv0307]

@mananjadhav If we add the check for reportAction and doesn't remove HOC, page is flicker when loading because we show loading from HOC and then loading in the page.

[mananjadhav]

I think we can go ahead with your proposal @dukenv0307. You've got the RCA as well as the suggestion. But I have two options of the implementation here.

1. The way you suggested, remove the HoC from the component, copy the report checks code to the component and add reportAction checks
2. Figure out a way to just add reportAction checks, without touching much of the withReportOrNotFound.
3. Create a new HoC that caters to report as well as reportAction.

I am going to confirm this from an internal engineer (@Li357) too.

🎀 👀 🎀 C+ reviewed.

[melvin-bot]

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

[dukenv0307]

I think we can create a new HOC for this because flag page also need this.

[mananjadhav]

Yeah I thought so. Let's wait for @Li357 to respond. Thanks for the patience here.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[Li357]

I agree, a new HOC would be great. I imagine future components that depend on a specific action being loaded would benefit from a central place of the logic.

[melvin-bot]

📣 @mananjadhav Please request via NewDot manual requests for the Reviewer role ($1000)

[melvin-bot]

📣 @dukenv0307 🎉 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]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[mananjadhav]

@dukenv0307 I see you've linked a PR, but I don't see the how its related to the current issue.

[dukenv0307]

@mananjadhav We have a problem here that is in mWeb when we open this page by deep link this will display not found page because in mWeb openReport doesn't call until we open this report.

[dukenv0307]

@mananjadhav Open the PR to see more detail.

[dukenv0307]

@mananjadhav Update: I updated HOC to resolve the problem above, please help to review the PR when you have a chance.

[melvin-bot]

🎯 ⚡️ Woah @mananjadhav / @dukenv0307, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @dukenv0307 got assigned: 2023-07-31 20:53:43 Z
• when the PR got merged: 2023-08-02 19:31:42 UTC

On to the next one 🚀

[mananjadhav]

@trjExpensify This is ready for payout on 08/15 but the title wasn't updated.

Also I couldn't pinpoint to a specific PR causing this. I think we should add a regression test for this one as it is was crashing the app and related to IOU. The test steps from the PR are good enough for the regression test.

@Li357 wdyt?

[trjExpensify]

Updated the title! :)

I think we should add a regression test for this one as it is was crashing the app and related to IOU. The test steps from the PR are good enough for the regression test.

Yeah cool, I'll ask Applause about what they think is best re: opening from deep links. We should do the same with request money as well.

[mananjadhav]

Makes sense.

[trjExpensify]

Okay, confirming payments as follows:

• @dhanashree-sawant $250 for the bug report (paid!)
• @mananjadhav $1,500 for C+ & #urgency bonus (awaiting a request)
• @dukenv0307 $1,500 for the fix (paid!)

[mananjadhav]

I’ve raised my request on NewDot.

[JmillsExpensify]

Reviewed the details for @mananjadhav. Approved for payment in NewDot based on BZ summary above.

[trjExpensify]

Dope, we're all set then. Closing!