[HOLD for payment 2023-11-21] [$250] Add error handling to readFilyAsync

[luacmartins]

Problem

We added the ability to request money with a scanned receipt in this PR, but there's no error handling if we fail to read the file before sending the request

Why is this important

Informs users that there was something wrong so it's not confusing for them.

Solution

Add error handling

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0116eda5bd8fef8746
• Upwork Job ID: 1716521658283470848
• Last Price Increase: 2023-10-30
• Automatic offers:
• DylanDylann | Contributor | 27491365

[luacmartins]

Will work on this this week.

[luacmartins]

Not a priority. I'll try to work on it next week.

[luacmartins]

Not a priority. I'll try to work on it next week.

[luacmartins]

Not a priority

[luacmartins]

I'll try to get to this this week. We also have an additional consideration that sometimes selecting a file in iOS just fails.

[luacmartins]

Not a priority

[luacmartins]

Not a priority

[luacmartins]

I think we can clean up the method by adding onSuccess and onFailure callback params to it and then just calling onFailure in the catch instead of resolving the promise. That also cleans up the need to have promise chaining in other files. Gonna make this external.

[luacmartins]

Here's an example. When the user uploaded a receipt and refreshed the page, we'd fail to load the file because the browser would get rid of the temp url. We have since fixed that issue, but I think we can clean it up with the solution I described here

[DylanDylann]

@luacmartins Thanks for your explanation.
Currently, If user reload page, the browser gets rid of the temp URL and the App will redirect to Scan screen (upload receipt screen)
Do you mean when the browser gets rid of the temp URL, we need to redirect the user to Scan screen and display a message like: "Please help to post receipt again"

[luacmartins]

No, we should still reroute the user. This is just a refactor of the current solution.

[DylanDylann]

@robertKozik Updated proposal for more detail
cc @luacmartins

[robertKozik]

Great! Thanks @DylanDylann for updating your proposal. I think we are GTG
🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @luacmartins is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @DylanDylann 🎉 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 📖

[DylanDylann]

@robertKozik The PR is ready for review

[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 1.3.98-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix/24141: Add error handling to readFilyAsync #30832

If no regressions arise, payment will be issued on 2023-11-21. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @robertKozik does not require payment (Contractor)
• @DylanDylann requires payment offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@robertKozik] The PR that introduced the bug has been identified. Link to the PR:
• [@robertKozik] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@robertKozik] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@robertKozik] Determine if we should create a regression test for this bug.
• [@robertKozik] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@conorpendergrast] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[conorpendergrast]

Payouts due:

• Contributor: $250 @DylanDylann Offer

• Contributor+: @robertKozik N/A - contractor

Upwork job is here.

[conorpendergrast]

This looks like a known uncaught error in a new feature rather than a bug itself, so I don't think we need the C+ checklist here. Closing out, reopen or comment if you disagree!