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

Invoices - App crashes after sending an invoice from FAB #47914

Closed
2 of 6 tasks
lanitochka17 opened this issue Aug 23, 2024 · 21 comments
Closed
2 of 6 tasks

Invoices - App crashes after sending an invoice from FAB #47914

lanitochka17 opened this issue Aug 23, 2024 · 21 comments
Assignees
Labels
Engineering Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Aug 23, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.24-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: N/A
Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch New Expensify app
  2. Go to FAB > Send invoice
  3. Enter amount > Next
  4. Select a user > Next
  5. Send the invoice

Expected Result:

App will not crash

Actual Result:

App crashes after sending an invoice from FAB

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

Add any screenshot/video evidence

Bug6580116_1724401370446.ScreenRecording_08-23-2024_17.mp4

logs (2).txt

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @tgolen
@lanitochka17 lanitochka17 added the DeployBlockerCash This issue or pull request should block deployment label Aug 23, 2024
Copy link

melvin-bot bot commented Aug 23, 2024

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

Copy link
Contributor

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

@VickyStash
Copy link
Contributor

Hi, I'm Viktoryia from Callstack - expert contributor group - and I would like to work on this issue.

@tgolen
Copy link
Contributor

tgolen commented Aug 23, 2024

Thank you @VickyStash!

@VickyStash
Copy link
Contributor

I've tried to reproduce the crash on different platforms and accounts and had no luck.

no_crash_11.mp4
no_crash_21.mp4

@lanitochka17 Do I miss anything?

@lanitochka17
Copy link
Author

Issue is still reproducible on IOS

ScreenRecording_08-24-2024.01-41-06_1.MP4

@VickyStash
Copy link
Contributor

@lanitochka17 It also works on iOS for me. Could you try to re-login to the account, will it be the same for you?

ios_recording1.mp4

@Nodebrute
Copy link
Contributor

@tgolen Can I submit a proposal for this issue? I am able to repro.

@melvin-bot melvin-bot bot added the Overdue label Aug 26, 2024
@nkdengineer
Copy link
Contributor

Proposal

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

App crashes after sending an invoice from FAB

What is the root cause of that problem?

After we create a send invoice, IOURequestStepConfirmation can be unmounted after the draft transaction is reset then report?.policyID ?? policy?.id to be undefined

policyID={report?.policyID ?? policy?.id}

Then policyID prop here is an empty string by default that causes the app crash here since the OnyxKey change from policyDrafts_id to policyDrafts_

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

We should use getIOURequestPolicyID function to get the policyID as we do here

policyID={report?.policyID ?? policy?.id}

OPTIONAL: We can also fallback -1 here to prevent the same bug if we pass the empty string policyID to MoneyRequestConfirmationList

const [mileageRatesDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID || -1}`, { 
   selector: (selectedPolicy: OnyxEntry<OnyxTypes.Policy>) => DistanceRequestUtils.getMileageRates(selectedPolicy), 
}); 

const [mileageRatesDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${policyID}`, {
selector: (selectedPolicy: OnyxEntry<OnyxTypes.Policy>) => DistanceRequestUtils.getMileageRates(selectedPolicy),
});
const mileageRates = isEmptyObject(mileageRatesReal) ? mileageRatesDraft : mileageRatesReal;

What alternative solutions did you explore? (Optional)

NA

@nkdengineer
Copy link
Contributor

@VickyStash @tgolen I can reproduce this issue on the latest main on mWeb safari.

Screen.Recording.2024-08-26.at.15.29.38.mov

@VickyStash
Copy link
Contributor

Good catch @nkdengineer! So it looks like this PR caused this regression, right?

@nkdengineer
Copy link
Contributor

@VickyStash Yes.

@tgolen
Copy link
Contributor

tgolen commented Aug 26, 2024

@dominictb Looks like #46859 caused the regression here so I will assign it to you. If you cannot write a fix today, then we'll need to revert that PR until a fix can be made. Are you able to submit a fix today?

@Nodebrute
Copy link
Contributor

@tgolen We are encountering another crash on iOS that is easily reproducible when navigating between different chats. The crash occurs due to an existing iOS bug where setIsInputInitialized(true) is being called in a loop, resulting in the 'Maximum update depth exceeded' error.

setIsInputInitialized(true);

Proposal:#47946 (comment)

Screenshot 2024-08-24 at 12 18 22 AM

@tgolen
Copy link
Contributor

tgolen commented Aug 26, 2024

@Nodebrute OK, thank you for letting me know. Unless I'm missing something though, your issue is unrelated to what is causing the crash in the original description of this issue, right?

@Nodebrute
Copy link
Contributor

@tgolen right, but I don’t think there are any open issues for this crash.

@dominictb
Copy link
Contributor

@tgolen I will raise the PR in a few hours.

@tgolen
Copy link
Contributor

tgolen commented Aug 26, 2024

@Nodebrute Thank you for clarifying! Please use our normal bug reporting process for the crash you found: https://github.com/Expensify/App/issues/new?assignees=&labels=Bug%2C+Daily&projects=&template=Standard.md

@dominictb dominictb mentioned this issue Aug 26, 2024
50 tasks
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Hourly KSv2 labels Aug 26, 2024
@dominictb
Copy link
Contributor

@tgolen The PR is ready.

@puneetlath puneetlath removed the DeployBlockerCash This issue or pull request should block deployment label Aug 26, 2024
@puneetlath
Copy link
Contributor

Demoting since the fix has been CP'd.

@tgolen
Copy link
Contributor

tgolen commented Aug 30, 2024

Weekly Update

Next Steps

  • @tgolen Close this issue since the fix has been deployed to production

ETA

  • Today, Aug 30

@tgolen tgolen closed this as completed Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants