diff --git a/src/sql/workbench/contrib/objectExplorer/common/serverGroup.contribution.ts b/src/sql/workbench/contrib/objectExplorer/common/serverGroup.contribution.ts index 3a20b8ea82..465d4e0f6c 100644 --- a/src/sql/workbench/contrib/objectExplorer/common/serverGroup.contribution.ts +++ b/src/sql/workbench/contrib/objectExplorer/common/serverGroup.contribution.ts @@ -41,7 +41,6 @@ const serverGroupConfig: IConfigurationNode = { } }; -export const NODE_EXPANSION_CONFIG = 'serverTree.nodeExpansionTimeout'; export const USE_ASYNC_SERVER_TREE_CONFIG = 'serverTree.useAsyncServerTree'; const serverTreeConfig: IConfigurationNode = { 'id': 'serverTree', @@ -52,12 +51,6 @@ const serverTreeConfig: IConfigurationNode = { 'type': 'boolean', 'default': true, 'description': localize('serverTree.useAsyncServerTree', "Use the new async server tree for the Servers view and Connection Dialog with support for new features such as dynamic node filtering. Requires a restart to take effect.") - }, - 'serverTree.nodeExpansionTimeout': { - 'type': 'number', - 'default': '45', - 'description': localize('serverTree.nodeExpansionTimeout', "The timeout in seconds for expanding a node in the Servers view"), - 'minimum': 1 } } }; diff --git a/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts b/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts index da45e66b8f..73a015694b 100644 --- a/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts +++ b/src/sql/workbench/services/objectExplorer/browser/asyncServerTree.ts @@ -151,3 +151,10 @@ export class AsyncServerTree extends WorkbenchAsyncDataTree(); this._activeObjectExplorerNodes = {}; @@ -298,27 +293,8 @@ export class ObjectExplorerService implements IObjectExplorerService { this._connectionsWaitingForSession.delete(connection.id); } createNewSessionListener.dispose(); - clearTimeout(timeoutHandle); } - const onTimeout = () => { - if (!this._sessions[sessionId]) { - this.logService.error(`Timed out waiting for session ${sessionId} to be created. - This has probably happened because OE service did not recieve a response for createNewSession from the provider.`); - reject(new Error( - nls.localize('objectExplorerMissingSession', - 'Timed out waiting for session {0} to be created. This has probably happened because OE service did not recieve a response for createNewSession from the provider.', sessionId))); - } else { - this.logService.error(`Timeout waiting for session ${sessionId} to be created for connection "${connection.title}". - This has probably happened because OE service did not recieve a response for createNewSession from the provider for connection."${connection.title}`); - reject(new Error(nls.localize( - 'objectExplorerMissingConnectionForSession', - 'Timeout waiting for session {0} to be created for connection "{1}". This has probably happened because OE service did not recieve a response for createNewSession from the provider for connection "{1}"', sessionId, connection.title - ))); - } - cleanup(); - } - const timeoutHandle = setTimeout(onTimeout, this.getObjectExplorerTimeout() * 1000); const createNewSessionListener = this._onCreateNewSession.event((response) => { checkSessionAndConnection(); }); @@ -510,7 +486,6 @@ export class ObjectExplorerService implements IObjectExplorerService { const resolveExpansion = () => { resolve(self.mergeResults(allProviders, resultMap, node.nodePath)); // Have to delete it after get all responses otherwise couldn't find session for not the first response - clearTimeout(expansionTimeout); if (newRequest) { delete self._sessions[session.sessionId!].nodes[node.nodePath]; this.logService.trace(`Deleted node ${node.nodePath} from session ${session.sessionId}`); @@ -529,19 +504,6 @@ export class ObjectExplorerService implements IObjectExplorerService { } }); - const expansionTimeout = setTimeout(() => { - /** - * If we don't get a response back from all the providers in specified expansion timeout seconds then we assume - * it's not going to respond and resolve the promise with the results we have so far - */ - if (resultMap.size !== allProviders.length) { - const missingProviders = allProviders.filter(p => !resultMap.has(p.providerId)); - this.logService.warn(`${session.sessionId}: Node expansion timed out for node ${node.nodePath} for providers ${missingProviders.map(p => p.providerId).join(', ')}`); - this._notificationService.error(nls.localize('nodeExpansionTimeout', "Node expansion timed out for node {0} for providers {1}", node.nodePath, missingProviders.map(p => p.providerId).join(', '))); - } - resolveExpansion(); - }, this.getObjectExplorerTimeout() * 1000); - self._sessions[session.sessionId!].nodes[node.nodePath].expandEmitter.event((expandResult: NodeExpandInfoWithProviderId) => { if (expandResult && expandResult.providerId) { this.logService.trace(`${session.sessionId}: Received expand result for node ${node.nodePath} from provider ${expandResult.providerId}`); @@ -600,8 +562,12 @@ export class ObjectExplorerService implements IObjectExplorerService { }); } - private mergeResults(allProviders: azdata.ObjectExplorerProviderBase[], resultMap: Map, nodePath: string): azdata.ObjectExplorerExpandInfo | undefined { - let finalResult: azdata.ObjectExplorerExpandInfo | undefined = undefined; + private mergeResults(allProviders: azdata.ObjectExplorerProviderBase[], resultMap: Map, nodePath: string): azdata.ObjectExplorerExpandInfo { + let finalResult: azdata.ObjectExplorerExpandInfo | undefined = { + sessionId: undefined, + nodePath: nodePath, + nodes: [] + }; let allNodes: azdata.NodeInfo[] = []; let errorNode: azdata.NodeInfo = { nodePath: nodePath, @@ -620,12 +586,14 @@ export class ObjectExplorerService implements IObjectExplorerService { if (resultMap.has(provider.providerId)) { let result = resultMap.get(provider.providerId); if (result) { + if (!result.errorMessage) { finalResult = result; if (result.nodes !== undefined && result.nodes) { allNodes = allNodes.concat(result.nodes); } } else { + finalResult.sessionId = result.sessionId; errorMessages.push(result.errorMessage); } } @@ -639,8 +607,11 @@ export class ObjectExplorerService implements IObjectExplorerService { errorNode.errorMessage = errorMessages.join('\n'); errorNode.label = errorNode.errorMessage; allNodes = [errorNode].concat(allNodes); + this._onUpdateObjectExplorerNodes.fire({ + connection: undefined, + errorMessage: errorNode.errorMessage + }); } - finalResult.nodes = allNodes; } return finalResult; @@ -1049,13 +1020,6 @@ export class ObjectExplorerService implements IObjectExplorerService { return currentNode; } - /** - * returns object explorer timeout in seconds. - */ - public getObjectExplorerTimeout(): number { - return this._configurationService.getValue(NODE_EXPANSION_CONFIG); - } - private getTreeNodeCacheKey(node: azdata.NodeInfo | TreeNode): string { return node.nodePath; } diff --git a/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts b/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts index e8f4d24228..55eb922196 100644 --- a/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts +++ b/src/sql/workbench/services/objectExplorer/browser/treeUpdateUtils.ts @@ -12,11 +12,9 @@ import { NodeType } from 'sql/workbench/services/objectExplorer/common/nodeType' import { TreeNode } from 'sql/workbench/services/objectExplorer/common/treeNode'; import { Disposable, isDisposable } from 'vs/base/common/lifecycle'; -import { onUnexpectedError } from 'vs/base/common/errors'; -import { AsyncServerTree, ServerTreeElement } from 'sql/workbench/services/objectExplorer/browser/asyncServerTree'; +import { getErrorMessage, onUnexpectedError } from 'vs/base/common/errors'; +import { AsyncServerTree, ConnectionError as AsyncTreeConnectionError, ServerTreeElement } from 'sql/workbench/services/objectExplorer/browser/asyncServerTree'; import { ObjectExplorerRequestStatus } from 'sql/workbench/services/objectExplorer/browser/treeSelectionHandler'; -import * as nls from 'vs/nls'; -import { NODE_EXPANSION_CONFIG } from 'sql/workbench/contrib/objectExplorer/common/serverGroup.contribution'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; export interface IExpandableTree extends ITree { @@ -184,52 +182,58 @@ export class TreeUpdateUtils { options: IConnectionCompletionOptions, connectionManagementService: IConnectionManagementService, tree: AsyncServerTree | ITree | undefined): Promise { - if (!connectionManagementService.isProfileConnected(connection)) { - // don't try to reconnect if currently connecting - if (connectionManagementService.isProfileConnecting(connection)) { - return undefined; - // else if we aren't connected or connecting then try to connect - } else { - let callbacks: IConnectionCallbacks | undefined; - if (tree instanceof AsyncServerTree) { - callbacks = { - onConnectStart: () => { }, - onConnectReject: () => { }, - onConnectSuccess: () => { }, - onDisconnect: () => { }, - onConnectCanceled: () => { }, - }; - } else if (tree) { - // Show the spinner in OE by adding the 'loading' trait to the connection, and set up callbacks to hide the spinner - tree.addTraits('loading', [connection]); - let rejectOrCancelCallback = () => { - tree.collapse(connection); - tree.removeTraits('loading', [connection]); - }; - callbacks = { - onConnectStart: () => { }, - onConnectReject: rejectOrCancelCallback, - onConnectSuccess: () => tree.removeTraits('loading', [connection]), - onDisconnect: () => { }, - onConnectCanceled: rejectOrCancelCallback, - }; - } + try { + if (!connectionManagementService.isProfileConnected(connection)) { + // don't try to reconnect if currently connecting + if (connectionManagementService.isProfileConnecting(connection)) { + return undefined; - const result = await connectionManagementService.connect(connection, undefined, options, callbacks); - if (result.connected) { - let existingConnection = connectionManagementService.findExistingConnection(connection); - return existingConnection; + // else if we aren't connected or connecting then try to connect } else { - throw new Error(result.errorMessage); + let callbacks: IConnectionCallbacks | undefined; + if (tree instanceof AsyncServerTree) { + callbacks = { + onConnectStart: () => { }, + onConnectReject: () => { }, + onConnectSuccess: () => { }, + onDisconnect: () => { }, + onConnectCanceled: () => { }, + }; + } else if (tree) { + // Show the spinner in OE by adding the 'loading' trait to the connection, and set up callbacks to hide the spinner + tree.addTraits('loading', [connection]); + let rejectOrCancelCallback = () => { + tree.collapse(connection); + tree.removeTraits('loading', [connection]); + }; + callbacks = { + onConnectStart: () => { }, + onConnectReject: rejectOrCancelCallback, + onConnectSuccess: () => tree.removeTraits('loading', [connection]), + onDisconnect: () => { }, + onConnectCanceled: rejectOrCancelCallback, + }; + } + + const result = await connectionManagementService.connect(connection, undefined, options, callbacks); + if (result.connected) { + let existingConnection = connectionManagementService.findExistingConnection(connection); + return existingConnection; + } else { + throw new Error(result.errorMessage); + } } + } else { + let existingConnection = connectionManagementService.findExistingConnection(connection); + if (options && options.showDashboard) { + await connectionManagementService.showDashboard(connection); + } + return existingConnection; } - } else { - let existingConnection = connectionManagementService.findExistingConnection(connection); - if (options && options.showDashboard) { - await connectionManagementService.showDashboard(connection); - } - return existingConnection; + } catch (e) { + // Since the connection dialog handles connection errors, we don't need to do anything here + throw new AsyncTreeConnectionError(getErrorMessage(e), connection); } } @@ -249,6 +253,7 @@ export class TreeUpdateUtils { objectExplorerService: IObjectExplorerService, tree: AsyncServerTree | ITree | undefined, requestStatus?: ObjectExplorerRequestStatus | undefined): Promise { + const connectedConnection = await TreeUpdateUtils.connectIfNotConnected(connection, options, connectionManagementService, tree); if (connectedConnection) { // append group ID and original display name to build unique OE session ID @@ -305,28 +310,18 @@ export class TreeUpdateUtils { return rootNode.children ?? []; } else { const options: IConnectionCompletionOptions = { + params: undefined, saveTheConnection: true, showConnectionDialogOnError: true, showFirewallRuleOnError: true, showDashboard: false }; - const expansionTimeoutValueSec = configurationService.getValue(NODE_EXPANSION_CONFIG); // Need to wait for the OE service to update its nodes in order to resolve the children const nodesUpdatedPromise = new Promise((resolve, reject) => { // Clean up timeout and listener const cleanup = () => { - clearTimeout(nodeUpdateTimer); nodesUpdatedListener.dispose(); } - - // If the node update takes too long, reject the promise - const nodeUpdateTimeout = () => { - cleanup(); - reject(new Error(nls.localize('objectExplorerTimeout', "Object Explorer expansion timed out for '{0}'", connection.databaseName))); - } - const nodeUpdateTimer = setTimeout(nodeUpdateTimeout, expansionTimeoutValueSec * 1000); - - const nodesUpdatedListener = objectExplorerService.onUpdateObjectExplorerNodes(e => { if (e.connection && e.connection.id === connection.id) { if (e.errorMessage) { diff --git a/src/sql/workbench/services/objectExplorer/test/browser/objectExplorerService.test.ts b/src/sql/workbench/services/objectExplorer/test/browser/objectExplorerService.test.ts index 0d878c7df0..3f2d307a6d 100644 --- a/src/sql/workbench/services/objectExplorer/test/browser/objectExplorerService.test.ts +++ b/src/sql/workbench/services/objectExplorer/test/browser/objectExplorerService.test.ts @@ -288,9 +288,7 @@ suite('SQL Object Explorer Service tests', () => { connectionManagementService.object, new NullAdsTelemetryService(), capabilitiesService, - logService, - configurationService.object, - notificationService.object); + logService); objectExplorerService.registerProvider(mssqlProviderName, sqlOEProvider.object); sqlOEProvider.setup(x => x.createNewSession(TypeMoq.It.is(x => x.options['serverName'] === connection.serverName))).returns(() => new Promise((resolve) => {