From 084042ad137647738a1df45522499abc270e30c7 Mon Sep 17 00:00:00 2001
From: ranasaria <41588310+ranasaria@users.noreply.github.com>
Date: Tue, 18 Sep 2018 13:41:14 -0700
Subject: [PATCH] Bug/oetimeout Fix - When timeout happens while fetching node
 children, the node becomes unusable (#2616)

This commit fixes issue when multiple OE nodes are expanded simultaneously. While the error was getting displayed the node was left in incorrect state which was leading to the node being unusable in future. This commit repairs this defect.
---
 .../viewlet/serverTreeDataSource.ts             |  6 +++++-
 src/vs/base/parts/tree/browser/treeModel.ts     | 17 ++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/src/sql/parts/objectExplorer/viewlet/serverTreeDataSource.ts b/src/sql/parts/objectExplorer/viewlet/serverTreeDataSource.ts
index 495c20c576..8392186449 100644
--- a/src/sql/parts/objectExplorer/viewlet/serverTreeDataSource.ts
+++ b/src/sql/parts/objectExplorer/viewlet/serverTreeDataSource.ts
@@ -7,7 +7,7 @@
 import { ConnectionProfileGroup } from 'sql/parts/connection/common/connectionProfileGroup';
 import { ConnectionProfile } from 'sql/parts/connection/common/connectionProfile';
 import { ITree, IDataSource } from 'vs/base/parts/tree/browser/tree';
-import { TreeNode } from 'sql/parts/objectExplorer/common/treeNode';
+import { TreeNode, TreeItemCollapsibleState } from 'sql/parts/objectExplorer/common/treeNode';
 import { IObjectExplorerService } from 'sql/parts/objectExplorer/common/objectExplorerService';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { TreeUpdateUtils } from 'sql/parts/objectExplorer/viewlet/treeUpdateUtils';
@@ -72,9 +72,13 @@ export class ServerTreeDataSource implements IDataSource {
 				if (node.children) {
 					resolve(node.children);
 				} else {
+					// These similar changes are probably needed for a ConnectionProfile group element as well. However, we do not have a repro of a failiure in that scenario so they will be tackled in a future checkin.
+					// It has been tested for connecting to the server in profile itself and things work fine there.
 					this._objectExplorerService.resolveTreeNodeChildren(node.getSession(), node).then(() => {
 						resolve(node.children);
 					}, expandError => {
+						node.setExpandedState(TreeItemCollapsibleState.Collapsed);
+						node.errorStateMessage = expandError;
 						this.showError(expandError);
 						resolve([]);
 					});
diff --git a/src/vs/base/parts/tree/browser/treeModel.ts b/src/vs/base/parts/tree/browser/treeModel.ts
index 2aa115ed62..3d5600b732 100644
--- a/src/vs/base/parts/tree/browser/treeModel.ts
+++ b/src/vs/base/parts/tree/browser/treeModel.ts
@@ -12,6 +12,8 @@ import { INavigator } from 'vs/base/common/iterator';
 import * as WinJS from 'vs/base/common/winjs.base';
 import * as _ from './tree';
 import { Event, Emitter, once, EventMultiplexer, Relay } from 'vs/base/common/event';
+// {{SQL CARBON EDIT}}
+import { TreeNode } from 'sql/parts/objectExplorer/common/treeNode';
 
 interface IMap<T> { [id: string]: T; }
 interface IItemMap extends IMap<Item> { }
@@ -359,6 +361,12 @@ export class Item {
 			return WinJS.TPromise.as(false);
 		}
 
+		// {{SQL CARBON EDIT}} - Original code does not handle the need to refresh children in case previous refreshchildren errored out.
+		//						 resetting the errorStateMessage before we refresh children so we can track if there has been error in processing.
+		if (this.element instanceof TreeNode) {
+			this.element.errorStateMessage = null;
+		}
+
 		var result = this.lock.run(this, () => {
 			var eventData: IItemExpandEvent = { item: this };
 			var result: WinJS.Promise;
@@ -373,7 +381,14 @@ export class Item {
 			return result.then(() => {
 				this._setExpanded(true);
 				this._onDidExpand.fire(eventData);
-				return true;
+				// {{SQL CARBON EDIT}} - Original code does not handle the need to refresh children in case previous refreshchildren errored out.
+				if ((this.element instanceof TreeNode) && (this.element.errorStateMessage)) {
+					this.needsChildrenRefresh = true;
+					return false;
+				} // We may need special handling for other types of this.element apart from TreeNode as well.
+				else {
+					return true;
+				}
 			});
 		});