From 749c42ce41533f93037953ed2d84ef2f941266a2 Mon Sep 17 00:00:00 2001 From: Charles Gagnon Date: Mon, 29 Jun 2020 19:50:27 -0700 Subject: [PATCH] Improve handling of connection errors for arc controllers (#11139) * some fixes for failed connections * cleanup * add back in prompt --- extensions/arc/src/localizedConstants.ts | 1 + extensions/arc/src/models/controllerModel.ts | 16 ++++++++------- extensions/arc/src/models/miaaModel.ts | 2 +- .../src/ui/dialogs/connectControllerDialog.ts | 3 ++- .../arc/src/ui/tree/controllerTreeNode.ts | 20 ++++++++++++++----- 5 files changed, 28 insertions(+), 14 deletions(-) diff --git a/extensions/arc/src/localizedConstants.ts b/extensions/arc/src/localizedConstants.ts index 45bba03021..059cd349b6 100644 --- a/extensions/arc/src/localizedConstants.ts +++ b/extensions/arc/src/localizedConstants.ts @@ -146,3 +146,4 @@ export function resourceDeletionWarning(namespace: string, name: string): string export function invalidResourceDeletionName(name: string): string { return localize('arc.invalidResourceDeletionName', "The value '{0}' does not match the instance name. Try again or press escape to exit", name); } export function couldNotFindAzureResource(name: string): string { return localize('arc.couldNotFindAzureResource', "Could not find Azure resource for {0}", name); } export function passwordResetFailed(error: any): string { return localize('arc.passwordResetFailed', "Failed to reset password. {0}", getErrorMessage(error)); } +export function errorConnectingToController(error: any): string { return localize('arc.errorConnectingToController', "Error connecting to controller. {0}", getErrorMessage(error)); } diff --git a/extensions/arc/src/models/controllerModel.ts b/extensions/arc/src/models/controllerModel.ts index 6c5acf572e..16f0e4f970 100644 --- a/extensions/arc/src/models/controllerModel.ts +++ b/extensions/arc/src/models/controllerModel.ts @@ -6,7 +6,7 @@ import * as vscode from 'vscode'; import { Authentication, BasicAuth } from '../controller/auth'; import { EndpointsRouterApi, EndpointModel, RegistrationRouterApi, RegistrationResponse, TokenRouterApi, SqlInstanceRouterApi } from '../controller/generated/v1/api'; -import { getAzurecoreApi, parseEndpoint, parseInstanceName } from '../common/utils'; +import { getAzurecoreApi, parseEndpoint, parseInstanceName, UserCancelledError } from '../common/utils'; import { ResourceType } from '../constants'; import { ConnectToControllerDialog } from '../ui/dialogs/connectControllerDialog'; import { AzureArcTreeDataProvider } from '../ui/tree/azureArcTreeDataProvider'; @@ -63,24 +63,26 @@ export class ControllerModel { } } - public async refresh(showErrors: boolean = true): Promise { - // We haven't gotten our password yet, fetch it now - if (!this._auth) { + public async refresh(showErrors: boolean = true, promptReconnect: boolean = false): Promise { + // We haven't gotten our password yet or we want to prompt for a reconnect + if (!this._auth || promptReconnect) { let password = ''; if (this.info.rememberPassword) { // It should be in the credentials store, get it from there password = await this.treeDataProvider.getPassword(this.info); } - if (password) { + if (password && !promptReconnect) { this.setAuthentication(new BasicAuth(this.info.username, password)); } else { - // No password yet so prompt for it from the user + // No password yet or we want to reprompt for credentials so prompt for it from the user const dialog = new ConnectToControllerDialog(this.treeDataProvider); - dialog.showDialog(this.info); + dialog.showDialog(this.info, password); const model = await dialog.waitForClose(); if (model) { this.treeDataProvider.addOrUpdateController(model.controllerModel, model.password, false); this.setAuthentication(new BasicAuth(this.info.username, model.password)); + } else { + throw new UserCancelledError(); } } diff --git a/extensions/arc/src/models/miaaModel.ts b/extensions/arc/src/models/miaaModel.ts index 9a54d46ce4..3209dc4dee 100644 --- a/extensions/arc/src/models/miaaModel.ts +++ b/extensions/arc/src/models/miaaModel.ts @@ -123,7 +123,7 @@ export class MiaaModel extends ResourceModel { // fire the event so callers can know to update (e.g. so dashboards don't show the // loading icon forever) if (err instanceof UserCancelledError) { - vscode.window.showErrorMessage(loc.connectionRequired); + vscode.window.showWarningMessage(loc.connectionRequired); } else { vscode.window.showErrorMessage(loc.fetchStatusFailed(this.info.name, err)); } diff --git a/extensions/arc/src/ui/dialogs/connectControllerDialog.ts b/extensions/arc/src/ui/dialogs/connectControllerDialog.ts index 3dad3816a1..2ba733f485 100644 --- a/extensions/arc/src/ui/dialogs/connectControllerDialog.ts +++ b/extensions/arc/src/ui/dialogs/connectControllerDialog.ts @@ -24,7 +24,7 @@ export class ConnectToControllerDialog { constructor(private _treeDataProvider: AzureArcTreeDataProvider) { } - public showDialog(controllerInfo?: ControllerInfo): void { + public showDialog(controllerInfo?: ControllerInfo, password?: string): void { const dialog = azdata.window.createModelViewDialog(loc.connectToController); dialog.cancelButton.onClick(() => this.handleCancel()); dialog.registerContent(async view => { @@ -43,6 +43,7 @@ export class ConnectToControllerDialog { this.passwordInputBox = this.modelBuilder.inputBox() .withProperties({ inputType: 'password', + value: password }) .component(); this.rememberPwCheckBox = this.modelBuilder.checkBox() diff --git a/extensions/arc/src/ui/tree/controllerTreeNode.ts b/extensions/arc/src/ui/tree/controllerTreeNode.ts index ed87f249cd..7c3b5e9080 100644 --- a/extensions/arc/src/ui/tree/controllerTreeNode.ts +++ b/extensions/arc/src/ui/tree/controllerTreeNode.ts @@ -11,12 +11,13 @@ import { PostgresTreeNode } from './postgresTreeNode'; import { ControllerModel, Registration, ResourceInfo } from '../../models/controllerModel'; import { ControllerDashboard } from '../dashboards/controller/controllerDashboard'; import { PostgresModel } from '../../models/postgresModel'; -import { parseInstanceName } from '../../common/utils'; +import { parseInstanceName, UserCancelledError } from '../../common/utils'; import { MiaaModel } from '../../models/miaaModel'; import { Deferred } from '../../common/promise'; import { RefreshTreeNode } from './refreshTreeNode'; import { ResourceTreeNode } from './resourceTreeNode'; import { AzureArcTreeDataProvider } from './azureArcTreeDataProvider'; +import * as loc from '../../localizedConstants'; /** * The TreeNode for displaying an Azure Arc Controller @@ -38,10 +39,19 @@ export class ControllerTreeNode extends TreeNode { await this.model.refresh(false); await this._childrenRefreshPromise.promise; } catch (err) { - // Couldn't get the children and TreeView doesn't have a way to collapse a node - // in a way that will refetch its children when expanded again so instead we - // display a tempory node that will prompt the user to re-enter credentials - return [new RefreshTreeNode(this)]; + vscode.window.showErrorMessage(loc.errorConnectingToController(err)); + try { + await this.model.refresh(false, true); + await this._childrenRefreshPromise.promise; + } catch (err) { + if (!(err instanceof UserCancelledError)) { + vscode.window.showErrorMessage(loc.errorConnectingToController(err)); + } + // Couldn't get the children and TreeView doesn't have a way to collapse a node + // in a way that will refetch its children when expanded again so instead we + // display a tempory node that will prompt the user to re-enter credentials + return [new RefreshTreeNode(this)]; + } } return this._children;