[HOLD for payment 2024-06-03] [$500] IOU - Creating IOU Distance offline with incomplete addresses results in incorrect error message

[izarutskaya]

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

---

Version Number: 1.4.51-0
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/4418303
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Preconditions:
Set up an OldDot admin account, invite the employee to the policy https://sites.google.com/applausemail.com/applause-expensifyproject/wiki-guides/newdot-categories?authuser=0

Steps:

1. Open https://staging.new.expensify.com/
2. Log in with the account of the employee added to the policy
3. Navigate to the WS room
4. Compose Box > Request Money
5. Select Distance option
6. Disable the internet connection
7. Enter a vague start location like "Fake"
8. Enter a vague finish location like "Blah blah"
9. Finish the request creation flow

Expected Result:

The error "No route found through the waypoints. Please edit the waypoints and try again." Should be shown in the UI.

Actual Result:

The error "Unexpected error requesting money, please try again later" is shown in the UI.

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

Bug6412058_1710321526582.Recording__1419.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01df4bc3e1ba142b9b
• Upwork Job ID: 1767941481061400576
• Last Price Increase: 2024-04-24
• Automatic offers:
• abdulrahuman5196 | Reviewer | 0
• bernhardoj | Contributor | 0

Issue Owner

Current Issue Owner: @laurenreidexpensify

[melvin-bot]

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

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

[melvin-bot]

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

[izarutskaya]

@lakchote 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 that this bug might be related to #wave-collect - Release 1
CC @trjExpensify

[izarutskaya]

Production video

bandicam.2024-03-13.12-56-20-421.mp4

[trjExpensify]

Looks like a deploy blocker to me!

[lakchote]

I can't reproduce on dev (BE won't let me do it).

Might be related to these changes

[lakchote]

@lakchote 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.

It's a frontend issue, and as such I'm opening it to external contributions.

[melvin-bot]

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

[melvin-bot]

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

[luacmartins]

@lakchote this is a deploy blocker. Since this is the end of your day, can we find an internal engineer to keep pushing this forward?

[tgolen]

I don't think this is a deploy blocker or a backend issue.

If I use the addresses "SF" and "LA", then the backend is able to come up with a route that goes from San Francisco to Lousiana (which is feasible). So, I think those are valid addresses.

When I use the addresses "fake" and "blah blah", the server returns an error No route found through the waypoints. Please edit the waypoints and try again.

Full API response

{
    "code": 666,
    "jsonCode": 666,
    "type": "Expensify\\Libs\\Error\\ExpError",
    "UUID": "5C7EDD2F-FAC9-4795-AA2D-383D7E053F9D",
    "message": "No route found through the waypoints. Please edit the waypoints and try again.",
    "title": "",
    "data": {
        "onyxData": [
            {
                "onyxMethod": "merge",
                "key": "transactions_1747048417427960048",
                "value": {
                    "errorFields": {
                        "route": {
                            "1710350682438627": "No route found through the waypoints. Please edit the waypoints and try again."
                        }
                    }
                }
            }
        ]
    },
    "htmlMessage": "",
    "onyxData": [
        {
            "onyxMethod": "merge",
            "key": "transactions_1747048417427960048",
            "value": {
                "errorFields": {
                    "route": {
                        "1710350682438627": "No route found through the waypoints. Please edit the waypoints and try again."
                    }
                }
            }
        }
    ],
    "requestID": "Zxpu95"
}

What I think needs to be fixed here is that the UI is not displaying the error that is returned from the server, and instead, it's displaying some generic error.

I'm going to remove the deploy blocker label, and update the description of the issue.

[melvin-bot]

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

[bernhardoj]

PR is ready

cc: @abdulrahuman5196

[tienifr]

@abdulrahuman5196 Could you give some feedback on my proposal?

Without the proper error clearing (in report action, IOU report createChat) here, when clearing the route error in the transaction thread, the user will see an undismissable RBR error in the report preview.

@bernhardoj's solution will have the above problem (mentioned and solved in my proposal)

[laurenreidexpensify]

@abdulrahuman5196 do you still have capacity for C+ on this one or can I reassign?

[abdulrahuman5196]

@abdulrahuman5196 Could you give some feedback on my proposal?

Without the proper error clearing (in report action, IOU report createChat) here, when clearing the route error in the transaction thread, the user will see an undismissable RBR error in the report preview.

bernhardoj's solution will have the above problem (mentioned and solved in my proposal)

Given the expectation change, your proposal seemed to be an extended fix of Bernhardoj's solution. That why went with the solution which came first.

Regarding the error clearing issue, it is an extended fix of the core issue(showing proper error in this case) which would be mostly handled in PR state.

[tienifr]

Regarding the error clearing issue, it is an extended fix of the core issue(showing proper error in this case) which would be mostly handled in PR state.

@abdulrahuman5196 It's a bug in the same flow so I'm not sure why it's only considered an "extended fix", it's part of the final solution. So it could be said that the selected proposal only fixes a part of the issue (only shows the error correctly, but not able to clear the errors) and my proposal fixes it fully (shows the error correctly and able to clear the errors).

Another way to think about this is, would we be fine for the error to show correctly but the user is unable to clear it? If yes, my solution is an "extended fix", if no, it's a "required fix" and should be factored in the proposal evaluation.

@tgolen What do you think?

[tgolen]

I can understand and agree with both sides. Ultimately, I'd like to see the most complete fix (the right error message AND being able to clear it) and I'm less interested about who does it or how it gets done :D

@tienifr If you'd like to open a new bug report for not being able to clear the error, I'd support just assigning it to you and have you fix it. Would that work?

[abdulrahuman5196]

Hi @tienifr , to mention we are clearing the error in the PR raised. In the PR, the error is shown properly and cleared properly AFAIK.

But basically the fix to clear the error is considered as an extended solution or improvement to the core fix which is showing the proper error(as per OP). Afaik we select the first original proposal which suggested the core fix if the improvements could/would have handled in the PR state. Because multiple improvements can be made on any selected proposal on any issue. In this case, clearing of error would have been mostly handled in the PR state.

Another way to think about this is, would we be fine for the error to show correctly but the user is unable to clear it? If yes, my solution is an "extended fix", if no, it's a "required fix" and should be factored in the proposal evaluation

We would want to implement any improvements along with the original fix. Contributors are encouraged to add improvements whenever possible. But proposals with minor/PR kind of improvements to the core issue might not be selected if the proposal is not the first posted compared to the proposal which has the core fix.

Cc @tgolen

[tgolen]

Yeah, I agree with that too. It's not always a given that stuff like that gets caught in the PR which is why I can see both sides to it. If it is caught and addressed in this PR, then let's just move forward with the single PR like it is.

[laurenreidexpensify]

@abdulrahuman5196 can you give an update of status of this issue? thank you

[bernhardoj]

The PR is merged 4 days ago, waiting for the deploy to staging and prod.

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

• Show invalid waypoint error message for an invalid distance request route #41481

If no regressions arise, payment will be issued on 2024-06-03. 🎊

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

• @abdulrahuman5196 requires payment automatic offer (Reviewer)
• @bernhardoj requires payment automatic 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:

• [@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR:
• [@abdulrahuman5196] 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:
• [@abdulrahuman5196] 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:
• [@abdulrahuman5196] Determine if we should create a regression test for this bug.
• [@abdulrahuman5196] 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.
• [@laurenreidexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[laurenreidexpensify]

Payment Summary:

C+ @abdulrahuman5196 $500 paid in upwork
Contributor @bernhardoj $500 paid in upwork

[laurenreidexpensify]

@abdulrahuman5196 please confirm checklist above so we can close this out thanks

[abdulrahuman5196]

The PR that introduced the bug has been identified. Link to the PR:
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:
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:

Not a regression.

Determine if we should create a regression test for this bug.

Yes.

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.

1. Open a workspace chat
2. Have at least 1 request
3. Go offline
4. Create a new distance request with invalid waypoints
5. Go online
6. Open the expense report
7. Verify there is an invalid waypoint error message below the money request preview
8. Open the transaction thread
9. Verify there is an invalid waypoint error message below the distance map image

@laurenreidexpensify