Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2023-05-23] [$2000] System displays a technical error message when handling an error (Auth CreateReportAction returned an error) #16637

Closed
1 of 6 tasks
kavimuru opened this issue Mar 28, 2023 · 88 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kavimuru
Copy link

kavimuru commented Mar 28, 2023

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 existing chat
  2. Click on “Profile” avatar
  3. Click on “Preferences” link
  4. Turn on “Simualte failing network request”
  5. Click on "+" icon
  6. Click on "Add attachment" link
  7. Select an image/document/video
  8. Click on “send” button
  9. Turn off “Simualte failing network request”
  10. Open the chat
  11. Observe that system displays an error message

Expected Result:

error message should not include technical details, and should include a clear, actionable message only

Actual Result:

system displays a technical error message (Auth CreateReportAction returned an error)

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: 1.2.90-7
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

2023-03-28.11.52.39.mp4
Recording.148.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1679994447012709

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f06dab90106f3c26
  • Upwork Job ID: 1643030950824513536
  • Last Price Increase: 2023-04-19
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 28, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

MelvinBot commented Mar 28, 2023

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

@melvin-bot melvin-bot bot added the Overdue label Mar 31, 2023
@MitchExpensify
Copy link
Contributor

Can't test because site is down currently, will test once I can

@melvin-bot melvin-bot bot removed the Overdue label Mar 31, 2023
@MitchExpensify
Copy link
Contributor

I am unable to reproduce this using the steps above

@MitchExpensify MitchExpensify added the Needs Reproduction Reproducible steps needed label Mar 31, 2023
@Natnael-Guchima
Copy link

2023-04-02.20.15.34.mp4

I am able to reproduce the issue.

@melvin-bot melvin-bot bot added the Overdue label Apr 2, 2023
@MitchExpensify
Copy link
Contributor

Got it this time!

image

@melvin-bot melvin-bot bot removed the Overdue label Apr 3, 2023
@MitchExpensify MitchExpensify added the External Added to denote the issue can be worked on by a contributor label Apr 3, 2023
@melvin-bot melvin-bot bot changed the title System displays a technical error message when handling an error (Auth CreateReportAction returned an error) [$1000] System displays a technical error message when handling an error (Auth CreateReportAction returned an error) Apr 3, 2023
@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 3, 2023
@MelvinBot
Copy link

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

@alitoshmatov
Copy link
Contributor

Seems like it is making the same request even if the first request was successful
изображение

@hellohublot
Copy link
Contributor

hellohublot commented Apr 4, 2023

Proposal

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

System displays a technical error message when handling an error (Auth CreateReportAction returned an error)

What is the root cause of that problem?

The error reported by the server is an onyx id conflict, because the network request has already been sent

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

We should not send this network request

+    if (shouldFailAllRequests || shouldForceOffline) {
+        return Promise.reject(new HttpsError({ 
+               message: CONST.ERROR.FAILED_TO_FETCH 
+        })
+    }
+

if (shouldFailAllRequests || shouldForceOffline) {
throw new HttpsError({
message: CONST.ERROR.FAILED_TO_FETCH,
});
}

-        if (shouldFailAllRequests || shouldForceOffline) {
-            throw new HttpsError({
-                message: CONST.ERROR.FAILED_TO_FETCH,
-            });
-        }

What alternative solutions did you explore? (Optional)

Not Yet

@Santhosh-Sellavel
Copy link
Collaborator

@johnmlee101
This is not a valid bug, and does not need to be fixed. shouldForceOffline or shouldFailAllRequests does not really stop anything from happening. In fact, all requests get succeeded only the client does not get updated. Those options are used only for testing/dev purposes and do not affect the product. So we can close this one.

cc: @MitchExpensify

@Natnael-Guchima
Copy link

Natnael-Guchima commented Apr 4, 2023

@Santhosh-Sellavel, I thought the feature in staging is used to simulate a real life situation that would happen if the user has a failing network. If that is the case, the user is currently seeing a technical error message that talks about a code module if system throws an error. It is like seeing a console log error on the UI. The normal trend of error handling is to display clear actionable message. In this case the error message excluding the phrase: "Auth CreateReportAction returned an error"

@Natnael-Guchima
Copy link

To make things clear. When system throws an error while user is trying to attach a file, the error should be:

Expected Result: Unexpected error while posting the comment. Please try again later.

Actual Result: Unexpected error while posting the comment. Please try again later. Auth CreateReportAction returned an error

@johnmlee101
Copy link
Contributor

@Santhosh-Sellavel so you're saying this is purely a bug with our own internal flags for testing? Is there a way to better simulate failures without introducing bugs that wouldn't be visible if this happened on production?

@Natnael-Guchima
Copy link

image
The same error message is displayed in production when the system throws an error message while a user is trying to attach an image. It is a bit hard to reproduce attachment failure in prod though.

@melvin-bot melvin-bot bot added the Awaiting Payment Auto-added when associated PR is deployed to production label May 16, 2023
@melvin-bot melvin-bot bot changed the title [$2000] System displays a technical error message when handling an error (Auth CreateReportAction returned an error) [HOLD for payment 2023-05-23] [$2000] System displays a technical error message when handling an error (Auth CreateReportAction returned an error) May 16, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label May 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 16, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented May 16, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.14-14 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-05-23. 🎊

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

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot
Copy link

melvin-bot bot commented May 16, 2023

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:

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

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels May 23, 2023
@jjcoffee
Copy link
Contributor

@MitchExpensify Gentle bump for payment on this one.

@MitchExpensify
Copy link
Contributor

Offer sent for payment @jjcoffee and your "appeal" makes sense to me, I will not apply to 50% within 9 days merge penalty

@jjcoffee
Copy link
Contributor

Thanks @MitchExpensify! Offer accepted.

@Natnael-Guchima
Copy link

@MitchExpensify a gentle bump to settle payments.

Thanks

@melvin-bot melvin-bot bot added the Overdue label May 29, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 29, 2023

@johnmlee101, @jjcoffee, @MitchExpensify, @Santhosh-Sellavel Whoops! This issue is 2 days overdue. Let's get this updated quick!

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented May 29, 2023

Not overdue only Checklist & payment is left out

@melvin-bot melvin-bot bot removed the Overdue label May 29, 2023
@MitchExpensify
Copy link
Contributor

All paid and contracts ended - Thanks!

@MitchExpensify
Copy link
Contributor

Bump on these next steps @Santhosh-Sellavel #16637 (comment)

@Santhosh-Sellavel
Copy link
Collaborator

I had this on my list but caught up with priority items. Will try and get it done this week

@melvin-bot melvin-bot bot added the Overdue label Jun 1, 2023
@MitchExpensify
Copy link
Contributor

MitchExpensify commented Jun 1, 2023

Not critically overdue, @Santhosh-Sellavel has this on their radar

@Santhosh-Sellavel
Copy link
Collaborator

Santhosh-Sellavel commented Jun 3, 2023

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:

@johnmlee101 Kind of an edge case, so regression test not need here let me know your thoughts thanks!

@melvin-bot melvin-bot bot added the Overdue label Jun 5, 2023
@MitchExpensify
Copy link
Contributor

MitchExpensify commented Jun 5, 2023

@johnmlee101 Friendly bump to avoid to the overdue overlords and close this one out potentially 😄

@melvin-bot melvin-bot bot removed the Overdue label Jun 5, 2023
@johnmlee101
Copy link
Contributor

Yeah we can avoid regressions for this I think

@MitchExpensify
Copy link
Contributor

Thanks everyone, closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

9 participants