Skip to content

fix(explorer): guard file-explorer scroll handler against transient tree-map desync (#188365)#310833

Open
maruthang wants to merge 1 commit intomicrosoft:mainfrom
maruthang:fix/issue-188365-file-explorer-tree-error
Open

fix(explorer): guard file-explorer scroll handler against transient tree-map desync (#188365)#310833
maruthang wants to merge 1 commit intomicrosoft:mainfrom
maruthang:fix/issue-188365-file-explorer-tree-error

Conversation

@maruthang
Copy link
Copy Markdown
Contributor

WhatExplorerView's scroll handler no longer throws TreeError [FileExplorer] Tree element not found when an editable element's tree node is in a brief desync during an async refresh.

WhyWorkbenchCompressibleAsyncDataTree has two internal node maps that are updated across an await during async refreshes (file-system-provider changes, file-nesting updates, etc.). hasNode() and getRelativeTop() consult different maps, so there is a microtask window where getRelativeTop() throws even when the element is still present from the caller's perspective. The onDidScroll handler at explorerView.ts:564 called getRelativeTop() with no guard — scroll events fire frequently during refresh, so this race reliably produced telemetry hits (#188365, 774 hits / 274 users in recent bot bucket).

How — Added a private tryGetRelativeTop(element) helper that wraps this.tree.getRelativeTop in try/catch and returns null on failure (semantic: "not currently visible"). Replaced the single unguarded call in onDidScroll; the fall-back branch editable.data.onFinish('', false) is safe to call when the element is in the data source even if the view hasn't caught up. The other getRelativeTop call at line 867 (in selectResource) already has its own try/catch that retries, so it doesn't leak this error.

Same defensive pattern applied in the companion fix for AgentSessionsView (#309891 / PR #310831).

Test plan — Manual: trigger the race by scrolling the file explorer while a large-directory refresh is in flight (file-system-provider change, file-nesting settings toggle). Pre-fix this produces the TreeError telemetry bucket; post-fix the error is absorbed and the editable's onFinish fires. No unit test added — reproducing the race deterministically would require heavy mocking of AsyncDataTree internals.

Fixes #188365

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TreeError [FileExplorer] Tree element not found: [object Object]

2 participants