Could we continue the command 'azure-account.askForLogin' when creating cloud shell

[jdneo]

Hi @chrmarti

Just find that when we are creating the cloud shell, and the user is not logged in, the extension will execute the code below: https://github.com/Microsoft/vscode-azure-account/blob/master/src/cloudConsole.ts#L125

Then if the user is not logged in previously, he needs to trigger the command again.

Could we await this command instead of return here?

for example:

...
if (!(await api.waitForLogin())) {
    await commands.executeCommand("azure-account.askForLogin");
    if (!(await api.waitForLogin())) {
        // handle login failure
    }
}
...

[jdneo]

Hi @chrmarti , I sent out a PR about this issue: #48. 😄

[chrmarti]

I have so far taken the view that when a user wants to do something (e.g., Open Cloud Shell) that requires the user to sign in first, it makes sense to let the user repeat the initial action (Open Cloud Shell) because there is such a long time between taking that initial action and completing the sign in process. The user might simply have forgotten what s/he was up to by the time the sign in completes.

Could you describe the scenario you are looking at?

[jdneo]

Yep, the scenario is just like you said.

1. User clicks "Open Cloud Shell".
2. User is told that he needs to sign in first.
3. User signs in.
4. Cloud Shell is provisioned.
5. Cloud Shell is shown to the user.

I agree that this is not a problem if users just forget their intention after signing in. But what if the user still remember? He might keep waiting but nothing happens.

I think continue the execution might improve the experience for the latter users, as long as this change won't introduce regression. 😄

[jdneo]

Em, actually this feedback is from our testers. After they click open cloud shell, nothing happens after signing in, which makes them confused.

BTW, an idea just came up in my mind.

Maybe I can check the login status through the API, and if the user is not signed in, tell him that he needs to sign in first, and then trigger the login command from the account extension.

After login, next time user trigger cloud shell related command, he can pass the login status check and do whatever he wants to do.

But I still much prefer the one click experience, since less action is needed.

What do you think?

[jdneo]

@chrmarti I saw your code change and tested on my side. I like the experience that telling user sign in needed in the terminal panel. Thanks!

But when user hasn't signed in at begining, I got Entry not found in cache. error when calling acquireToken(). Is that becaus I missed something?

[chrmarti]

I cannot reproduce the "Entry not found in cache" error. Do you see it every time you start without being signed in?

[jdneo]

Yes it is. The problem only occurs if I am not signed in.

My code looks like this:

const accountAPI: AzureAccount = vscode.extensions
    .getExtension<AzureAccount>("ms-vscode.azure-account")!.exports;
this.cloudShell =  accountAPI.createCloudShell("Linux");
this.terminal = await this.cloudShell.terminal;
this.terminal.show();

const session: AzureSession = await cloudShell.session;
const token: IToken = await acquireToken(session);

The method acquireToken() is just copied from the code in azure account extension

[chrmarti]

I do just that before resolving the cloudShell.session promise and there it seems to work: https://github.com/Microsoft/vscode-azure-account/blob/f0846fc601554ddb3f945e34e6a85551b705fca7/src/cloudConsole.ts#L310

[jdneo]

Do you mean I need to pass the Promise to the method? But the acquireToken() is declared to accept AzureSession type.

[chrmarti]

The way you have it should work. I'm surprised it does not work because my code is doing the same shortly before your code does it. Maybe put breakpoints in your code and here to step through: https://github.com/Microsoft/vscode-azure-account/blob/f0846fc601554ddb3f945e34e6a85551b705fca7/src/cloudConsole.ts#L310

[jdneo]

Hi @chrmarti , I did some tests and here is my observation:

1. Open cloud shell command in the Azure Account extension works fine no matter I'm signed in or not.
2. If I have signed in, open cloud shell using my above code snippet works fine.
3. If I haven't signed in at beginning, I will get "Entry not found in cache" error.

I find there is a difference is in the session.credentials.context.cache.

When there is no error, there are two entries in the cache, one for the common tenant, the other for the Microsoft AD
When there occurs error, there are three entries in the cache, one for the common tenant, the other two are the same, both of them are the Microsoft AD. See my screenshot below.

[jdneo]

Hi @chrmarti, I have a question for the method createCloudConsole(): https://github.com/Microsoft/vscode-azure-account/blob/master/src/cloudConsole.ts#L156

Can we resolve the Promise for AzureSession before we calling the acquireToken()? for example:

	...
	deferredSession!.resolve(pick.session);	
	token = await acquireToken(pick.session);
} else {
	deferredSession!.resolve(api.sessions[0]);
	token = await acquireToken(api.sessions[0]);
}

I tested locally and seems this can resolve the error for Entry not found in cache

[chrmarti]

I'm not opposed to shuffle things around, but I'd rather first understand why it sometimes works and sometimes doesn't. It looks suspiciously like a timing issue at the moment and I'm not sure the reshuffling will fix it or just make it less likely to happen.

[jdneo]

It looks like not a timing issue.

api.sessions[0] is different from result.token.session from my observation.

[jdneo]

Hi @chrmarti I found out a way to reproduce Entry not found in cache error. Just trigger the Azure: Open bash in clould shell twice.

More details can be found in the gif below:

[chrmarti]

Continued in #53. Thanks @jdneo.