Improvement: Introduce support for "cached" error TreeElements in the TreeView

[tnaum-ms]

Error Node Caching for Improved User Experience

Currently, an error tree node (TreeElement) will be generated, for example in case of incorrect credentials or connectivity issues. On any TreeView refresh event, the branch data provider will attempt to refresh every visible node. In the scenario mentioned, it will attempt to reconnect, and in case of incorrect credentials or connectivity issues, the user will see the same error node generated—however, after a significant delay.

Imagine this happening with a few nodes at the same time. This is a user experience issue.

A solution has been prototyped and implemented in the DocumentDB for VS Code Extension. Below are the key components of the approach:

This description focuses on caching only, but if you look closely at the referenced code base, you'll notice that we added support for additional error nodes being added at a higher level, from specific data providers.

Implementation Steps

1. When an error node is created, it needs to have the context set to 'error'

   In the ClusterItemBase.ts file, when authentication fails, we create an error node with a specific context value:

   createGenericElementWithContext({
     contextValue: 'error',
     id: `${this.id}/reconnect`, // note: keep this in sync with the `hasRetryNode` function in this file
     label: vscode.l10n.t('Click here to retry'),
     iconPath: new vscode.ThemeIcon('refresh'),
     commandId: 'vscode-documentdb.command.internal.retryAuthentication',
     commandArgs: [this],
   });

   This error node appears in the tree with a refresh icon and "Click here to retry" text. The context value 'error' is critical for identifying these nodes later.

   Note: We had to create our own createGenericElementWithContext function as the default createGenericElement from the Azure extension utils does not support setting the contextValue property.

Note: While reviewing this description, I realized that the error context is not used any longer and now I rely on hasRetryNode only.

See implementation in ClusterItemBase.ts.

2. Add an error node cache in the branch data provider

   In the ConnectionsBranchDataProvider.ts file, we added a cache to track error nodes:

   /**
    * Caches nodes whose getChildren() call has failed.
    *
    * This cache prevents repeated attempts to fetch children for nodes that have previously failed,
    * such as when a user enters invalid credentials. By storing the failed nodes, we avoid unnecessary
    * repeated calls until the error state is explicitly cleared.
    *
    * Key: Node ID (parent)
    * Value: Array of TreeElement representing the failed children (usually an error node)
    */
   private readonly errorNodeCache = new Map<string, TreeElement[]>();

   This cache stores parent node IDs as keys and their error node children as values, preventing repeated connection attempts for nodes that have already failed.

   See implementation in ConnectionsBranchDataProvider.ts.

3. In the getChildren method, detect whether to use the cache and cache elements based on the context value

   In the getChildren method of ConnectionsBranchDataProvider.ts, we first check if the element has a cached error nodes:

            // 1. Check if we have a cached error for this element
            //
            // This prevents repeated attempts to fetch children for nodes that have previously failed
            // (e.g., due to invalid credentials or connection issues).
            if (element.id && this.errorNodeCache.has(element.id)) {
                context.telemetry.properties.usedCachedErrorNode = 'true';
                return this.errorNodeCache.get(element.id);
            }

   Then, after fetching children, we check if they contain a retry node and if so, we cache them:

            // 3. Check if the returned children contain an error node
            // This means the operation failed (eg. authentication)
            if (isTreeElementWithRetryChildren(element) && element.hasRetryNode(children)) {
                // optional: append helpful nodes to the error node
                children?.push(
                    createGenericElementWithContext({
                        contextValue: 'error',
                        id: `${element.id}/updateCredentials`,
                        label: vscode.l10n.t('Click here to update credentials'),
                        iconPath: new vscode.ThemeIcon('key'),
                        commandId: 'vscode-documentdb.command.connectionsView.updateCredentials',
                        commandArgs: [element],
                    }),
                );
                // Store the error node(s) in our cache for future refreshes
                this.errorNodeCache.set(element.id, children ?? []);
                context.telemetry.properties.cachedErrorNode = 'true';
            }

   Note: Instead of checking the context value directly, we use the hasRetryNode method from ClusterItemBase which checks if any child in the array has an ID ending with '/reconnect'. The exact implementation is:

   public hasRetryNode(children: TreeElement[] | null | undefined): boolean {
       return !!(children && children.length > 0 && children.some((child) => child.id.endsWith('/reconnect')));
   }

   The way of determining error nodes (whether by context value or ID pattern) is not critical, as long as it's consistent.

   See implementation in ConnectionsBranchDataProvider.ts and ClusterItemBase.ts.

4. Provide a way to reset the error state

   We added a method to the ConnectionsBranchDataProvider class to remove a node from the error cache:

   /**
    * Removes a node's error state from the failed node cache.
    * This allows the node to be refreshed and its children to be re-fetched on the next refresh call.
    * If not reset, the cached error children will always be returned for this node.
    * @param nodeId The ID of the node to clear from the failed node cache.
    */
   resetNodeErrorState(nodeId: string): void {
       this.errorNodeCache.delete(nodeId);
   }

   This method is crucial as it allows us to clear a node's error state, enabling a proper refresh attempt when the user wants to retry the connection.

   See implementation in ConnectionsBranchDataProvider.ts.

5. Implement a dedicated command that calls resetNodeErrorState and then refresh function

   We implemented a retryAuthentication command that clears the error state and refreshes the node:

   export async function retryAuthentication(_context: IActionContext, node: ClusterItemBase): Promise<void> {
     if (!node) {
       throw new Error(l10n.t('No node selected.'));
     }
   
     if (new RegExp(`\\b${Views.ConnectionsView}\\b`, 'i').test(node.contextValue)) {
       ext.connectionsBranchDataProvider.resetNodeErrorState(node.id);
       return ext.connectionsBranchDataProvider.refresh(node);
     }
   
     if (new RegExp(`\\b${Views.DiscoveryView}\\b`, 'i').test(node.contextValue)) {
       ext.discoveryBranchDataProvider.resetNodeErrorState(node.id);
       return ext.discoveryBranchDataProvider.refresh(node);
     }
   
     throw new Error(l10n.t('Unsupported view for an authentication retry.'));
   }

   This command:

   • Takes a node as input
   • Determines which view the node belongs to (Connections or Discovery)
   • Calls the appropriate branch data provider's resetNodeErrorState method
   • Refreshes the node

   This allows users to retry connections by clicking on the "Click here to retry" node, without causing refresh delays for other nodes in the tree.

   See implementation in retryAuthentication.ts.

Conclusion

With this implementation, error nodes are cached and reused on refresh operations, preventing repeated connection attempts to failing nodes. This significantly improves the user experience by avoiding lengthy delays when multiple nodes have connection issues. The caching mechanism is only bypassed when the user explicitly chooses to retry the connection.

[tnaum-ms]

@sevoku If you added instructions stating where you'd like to see the change done, Copilot should be able to refactor it on its own.

I went quite verbose here (with help), to try it out.