fix(explorer): guard file-explorer scroll handler against transient tree-map desync (#188365)#310833
Merged
dmitrivMS merged 2 commits intoJun 5, 2026
Conversation
lramos15
approved these changes
Jun 5, 2026
dmitrivMS
approved these changes
Jun 5, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the File Explorer (ExplorerView) scroll handler against a transient AsyncDataTree internal state desync that can throw TreeError [FileExplorer] Tree element not found during async refreshes, reducing unhandled error telemetry for #188365.
Changes:
- Replaces an unguarded
tree.getRelativeTop(editable.stat)call in theonDidScrollhandler with a guarded helper. - Introduces
tryGetRelativeTop(element)which returnsnullwhengetRelativeTopfails, preserving the “not visible” semantic for callers.
Show a summary per file
| File | Description |
|---|---|
src/vs/workbench/contrib/files/browser/views/explorerView.ts |
Guards the explorer scroll/editable-finish path against getRelativeTop() throwing during async refresh desyncs. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
Comment on lines
+734
to
+738
| try { | ||
| return this.tree.getRelativeTop(element); | ||
| } catch { | ||
| return null; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What —
ExplorerView's scroll handler no longer throwsTreeError [FileExplorer] Tree element not foundwhen an editable element's tree node is in a brief desync during an async refresh.Why —
WorkbenchCompressibleAsyncDataTreehas two internal node maps that are updated across anawaitduring async refreshes (file-system-provider changes, file-nesting updates, etc.).hasNode()andgetRelativeTop()consult different maps, so there is a microtask window wheregetRelativeTop()throws even when the element is still present from the caller's perspective. TheonDidScrollhandler atexplorerView.ts:564calledgetRelativeTop()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 wrapsthis.tree.getRelativeTopin try/catch and returnsnullon failure (semantic: "not currently visible"). Replaced the single unguarded call inonDidScroll; the fall-back brancheditable.data.onFinish('', false)is safe to call when the element is in the data source even if the view hasn't caught up. The othergetRelativeTopcall at line 867 (inselectResource) 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
TreeErrortelemetry bucket; post-fix the error is absorbed and the editable'sonFinishfires. No unit test added — reproducing the race deterministically would require heavy mocking ofAsyncDataTreeinternals.Fixes #188365