fix: guard identity lookup for root element in getDataNode (fixes #330873)
- Dominant language
- TypeScript
- Stars
- 193k
- Forks
- 42.4k
- PR merge metrics
- PR metrics pending
Description
### Summary
A `TypeError: s.getId is not a function` is thrown from the file explorer's async data tree (`asyncDataTree.getDataNode`) and surfaces via `explorerView.ts` telemetry. The crash is in the **error-enrichment path** of `getDataNode`: when a tree node cannot be found, the diagnostic branch unconditionally casts the lookup `element` to `T` and calls `identityProvider.getId(...)`. For the file explorer the tree is parameterized as `TInput = ExplorerItem[]` (an array) and `T = ExplorerItem`, so when `element` is the **root input array** the `getId` call is invoked on an array, which has no `getId` method — throwing a *different* error that masks the intended `TreeError` and pollutes telemetry.
This is a pre-existing latent bug (introduced 2024) that became newly reachable in 1.133.0 because the new **Agents / Sessions window** (`src/vs/sessions/`) bundles and exercises the file explorer, producing a fresh error bucket with 222 affected users where 1.132.1 had none.
Fixes microsoft/vscode\#330873
Recommended reviewer: `@benibenj`
### Culprit Commit
`6fc6135e6e53` — Benjamin Simmonds (`@benibenj`), PR #231843, 2024-10-21, "Improve error logging for missing tree nodes". This commit added the `getId`-based message enrichment at the throw site. It is a diagnostics/logging change; the underlying type bypass (`element as T` on a `TInput | T` value) is what allows an array to reach `getId`. The bucket is new in 1.133.0 only because the Sessions window newly exercises this code path.
### Code Flow
```mermaid
flowchart TD
A["ExplorerView.refresh(item?)"] --> B["toRefresh = item ?? tree.getInput()"]
B -->|item undefined| C["toRefresh = ExplorerItem[] (root TInput)"]
C --> D["AsyncDataTree.updateChildren(toRefresh)"]
D --> E["_updateChildren(element = array)"]
E --> F["getDataNode(element = array)"]
F --> G{"nodes.get(root ? null : element)?"}
G -->|found| H["return node (normal)"]
G -->|not found: stale/replaced input during race| I["identityProvider.getId(element as T)"]
I --> J["array.getId() -> TypeError: s.getId is not a function"]
```
### Affected Files
- `src/vs/base/browser/ui/tree/asyncDataTree.ts` — `getDataNode` (the type-system bypass and fix location).
- `src/vs/workbench/contrib/files/browser/views/explorerView.ts` — crash surfaces here via the `identityProvider.getId` (line 90) called from the tree; unchanged.
### Repro Steps
1. Open the Agents / Sessions window (new in 1.133.0), which hosts the file explorer.
2. Trigger an explorer refresh where the root input array is passed to `updateChildren` while the node map has been replaced/cleared (e.g., a parallel refresh / input reset race).
3. `getDataNode` fails to find the node and enters the error-enrichment branch, calling `getId` on the root array → `TypeError: s.getId is not a function`.
### How the Fix Works
**Chosen approach** — `src/vs/base/browser/ui/tree/asyncDataTree.ts`, `getDataNode`:
The node lookup on line 1074 already distinguishes the root element (`element === this.root.element ? null : element`), correctly recognizing that the root's key is `null` and that the root `element` is the `TInput`, not a `T`. The error-enrichment branch, however, ignored this distinction and cast `element as T` before calling `getId`. The fix computes the identity string **only when `element !== this.root.element`**:
```ts
const nodeIdentity = element !== this.root.element ? this.identityProvider?.getId(element as T).toString() : undefined;
```
This fixes the defect at the exact site of the type bypass (the invalid `as T` cast on a `TInput | T` value), guaranteeing `getId` is only ever invoked on an actual `T`. It preserves the intended `TreeError` throw and the telemetry pipeline — the real "Data tree node not found" error is still reported, no `try/catch` is added, and no `logService.error` is removed. After this change, line 1077 cannot call `getId` on the root `TInput` array, so `s.getId is not a function` can no longer be produced there; the genuine underlying error is surfaced instead of being masked.
**Alternatives considered:** wrapping the `getId` call in `try/catch` — rejected because it would swallow the diagnostic and mask the real `TreeError`, hiding the symptom rather than fixing the invalid cast at the producer.
### Recommended Owner
`@benibenj` — author of the culprit enrichment commit and owner of the tree widget area. If write-access verification fails, defaults to `bryanchen-d`.
> Generated by [errors-fix](https://github.com/microsoft/vscode-engineering/actions/runs/31814618767) · opus48 · 555.4 AIC · ⌖ 17.8 AIC · ⊞ 18.6K · [◷](https://github.com/search?q=repo%3Amicrosoft%2Fvscode+%22gh-aw-workflow-id%3A+errors-fix%22&type=pullrequests)
---
> [!NOTE]
> This was originally intended as a pull request, but PR creation failed. The changes have been pushed to the branch [`fix/async-tree-getid-typerror-330873-e11f44695d284621`](https://github.com/vscodebot-pr/vscode/tree/fix/async-tree-getid-typerror-330873-e11f44695d284621).
>
> **Original error:** ERR_API: [2026-08-14T15:50:46.625Z] create pull request in microsoft/vscode failed (attempt 1)
Original error: Validation Failed: {"resource":"PullRequest","code":"custom","field":"fork_collab","message":"fork_collab Fork collab can't be granted by someone without permission"} - https://docs.github.com/rest/pulls/pulls#create-a-pull-request
Retryable: false
Suggestion: This error cannot be resolved by retrying. Please check the error details and fix the underlying issue.
To create the pull request manually:
```sh
gh pr create --title "fix: guard identity lookup for root element in getDataNode (fixes #330873)" --base main --head vscodebot-pr:fix/async-tree-getid-typerror-330873-e11f44695d284621 --repo microsoft/vscode
```
Show patch (29 lines)
```diff
From cf33c4f68fafeda9d6920acee6602501effa58f5 Mon Sep 17 00:00:00 2001
X-GH-AW-Base-Commit: 6ebcc0a377ae04e130212f34df551c8340cf966a
From: "github-actions[bot]"
Date: Fri, 14 Aug 2026 15:43:20 +0000
Subject: [PATCH] fix: guard identity lookup for root element in getDataNode
(fixes #330873)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
src/vs/base/browser/ui/tree/asyncDataTree.ts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/vs/base/browser/ui/tree/asyncDataTree.ts b/src/vs/base/browser/ui/tree/asyncDataTree.ts
index d3c4c24efcc..533fe1969c6 100644
--- a/src/vs/base/browser/ui/tree/asyncDataTree.ts
+++ b/src/vs/base/browser/ui/tree/asyncDataTree.ts
@@ -1074,7 +1074,7 @@ export class AsyncDataTree implements IDisposable
const node: IAsyncDataTreeNode | undefined = this.nodes.get((element === this.root.element ? null : element) as T);
if (!node) {
- const nodeIdentity = this.identityProvider?.getId(element as T).toString();
+ const nodeIdentity = element !== this.root.element ? this.identityProvider?.getId(element as T).toString() : undefined;
throw new TreeError(this.user, `Data tree node not found${nodeIdentity ? `: ${nodeIdentity}` : ''}`);
}
--
2.54.0
```
Contributor guide
Assessment
This issue has not been assessed yet.