Steps in VBA flow are resulting in a loading spinner, making it impossible to enable the card

[kevinksullivan]

Problem

Slack thread

When going through the VBA flow for the Free Plan, there are certain steps that result in a loading spinner, which makes it impossible to enable the Expensify Card. In this PR we caught it at the ValidateStep, but in my most recent testing on kevin@ottercountry.com I reached a loading spinner after entering Company Info, and have been stuck there for ~24 hours now.

According to @tgolen here, this scenario may occur in several areas/steps of this process.

This makes it impossible to enable the card on the Free Plan in NewDot.

Solution

From the thread it sounds like a certain API call is failing at this step, and could occur elsewhere. @tgolen @marcaaron not sure if either of you have the capacity to investigate this further, but tagging you in case you have any other thoughts on the best way to proceed. Thanks!

Note: as a follow up GH, I think we should never show the loading spinner in place of the button copy, and always make the button clickable. Even for a step that loads for a few seconds, it looks a bit odd to see the button turn into a loading spinner in addition to the right pane.

[MitchExpensify]

This is a launch blocker, labelling eng to expedite the investigation

[MelvinBot]

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[stitesExpensify]

As I mentioned in slack, it seems like the CORS issue is what caused this to fail. I'm not sure why we got that error though, staging-new is definitely whitelisted in api.php...

[tgolen]

The CORS is a red-herring. We need to see in the server-logs what the error was on the server for that request.

[MitchExpensify]

I encountered the spinner when logged into staging.new.expensify.com as blueberry@expensivepie.com. Interestingly, I could not however replicate the spinner when logged into both OldDot and NewDot on staging.

https://user-images.githubusercontent.com/36425901/127898454-a8982989-668d-40f6-a4fe-37d53d18bcfc.mov

Logs

[stitesExpensify]

@tgolen

The CORS is a red-herring. We need to see in the server-logs what the error was on the server for that request.

Why do you think it's a red herring? The API call causing the spinner is the exact API call that was blocked by CORS, which makes sense why we would be permanently stuck in a loading state.

[stitesExpensify]

I am unable to reproduce any issues even with the same account Mitch used (blueberry@expensivepie.com)

[tgolen]

CORS is an all-or-nothing setting on the server, so it's not possible (in our code) to have one API request throw a CORS error, but not another. It happens in e.cash triggered off of a server error. You can see additional times this was discussed in Slack here: https://expensify.slack.com/archives/C03TQ48KC/p1617979958063200?thread_ts=1617920668.034500&cid=C03TQ48KC

[stitesExpensify]

Hmm, yeah it makes sense that you shouldn't get CORS errors on some calls and not others. My thinking in this case was that it's calling secure which is not called pretty much anywhere but here. I checked the git history though and we updated secure to accept staging.new.expensify.com a long time ago.

Looking at the logs for kevin, the only error I see for that API call is 407 Unauthorized - Invalid token which makes me think that the auth token just happened to expire before the call.

Looking at the logs for Mitch is more interesting, we have

BankAccount_VerificationAPI - Bank account or identity could not be verified: unexpectedRealSearchError Expensify\Error\BankAccount\AutoVerifyFailure - 1a348233-aa75-4def-a22a-32aff17ac228 ~~ currentStep: 'CompanyStep' realSearchResult: '[status: 'unexpectedRealSearchError' apiResult: '[reason: 'Tested via external APIs' status: 'Success' data: '[responseCode: '91' responseDesc: 'ERROR communicating with web service or Request Timed Out.']']

and also this [BankAccountSetup] BankAccountAPI - External verifications failed. Asking user to confirm before calling Auth SetupWithdrawalAccount ~~ currentStep: 'CompanyStep'

Are we missing a step that we do in web for confirming info? Regardless it seems like we aren't catching any errors here

[stitesExpensify]

Okay it looks like when there is a legit error thrown such as a 404, you won't get into .then for promises. We need to add a .catch() to this call

[tgolen]

Yep, I think that will fix this particular API call. I have a feeling this will happen to others. I looked through this file and found these methods could possibly have a stuck spinner if the HTTP request fails:

• activateWallet()
• getPlaidBankAccounts()
• fetchFreePlanVerifiedBankAccount() (the first API call for getting NVP)
• validateBankAccount()
• setupWithdrawalAccount() (looks like there are several API calls that could fail and make the spinner stuck)

Most of these are because there is no .catch() on the API requests.

So, we can either choose to fix them one-by-one as we experience them, or we can proactively add better error handling to all these methods. I would be OK with fixing this single instance right now since it's an N6 blocker, and then patching up the rest in a follow-up issue, but I also think it isn't too much effort to patch all these in one go either.

[stitesExpensify]

Cool sounds good @tgolen. My plan right now is to throw a generic "oops, try again later" style error when something goes wrong enough that we get into a catch(), and bringing them back to the beginning of the process. We will also log the stack to the console. Does that sound good @kevinksullivan ?

https://user-images.githubusercontent.com/42391420/128059302-52fda595-b69f-48a4-bb86-cbaabbe45e17.mp4

[stitesExpensify]

As I mentioned on slack, WRT Mitch's error, I think we have a step in web that is not in newDot that we need to add. This specific error is when we can't auto-validate a bank account and have the user confirm themselves

[MitchExpensify]

Can you mention that here @stitesExpensify ? Sounds helpful for that issue, right?

[stitesExpensify]

Can you mention that here @stitesExpensify ? Sounds helpful for that issue, right?

Hmm I'm not sure it's related tbh. The inability to auto-validate and have the user confirm is a feature we need to add, whereas that GH is specifically about the amount of time it takes that API call to run

[tgolen]

throw a generic "oops, try again later" style error

This is fine with me. We have a growl lib that can do this.

[kevinksullivan]

My plan right now is to throw a generic "oops, try again later" style error when something goes wrong enough that we get into a catch(), and bringing them back to the beginning of the process.

@stitesExpensify does this mean if I am at the company info step and the API call fails, I'll be sent back to the very first step of the process? Or will I get dropped back in the company info step with the information I previously entered shown in the fields?

[stitesExpensify]

You will be dropped back at the beginning, your data should be saved though

https://user-images.githubusercontent.com/42391420/128268165-c8cc3a1e-a7be-4052-a573-9b7819ce7704.mp4

[MelvinBot]

@stitesExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[stitesExpensify]

This is being QA'd

[MitchExpensify]

Seeing as this (https://github.com/Expensify/App/pull/4400) is on production, can this be closed @stitesExpensify ?

[MitchExpensify]

Closing as this seems to be a GH issue not auto-closing.