fix: guard against non-iterable value in mapObservableArrayCached (fixes #314122)#314215
Draft
vs-code-engineering[bot] wants to merge 1 commit intomainfrom
Draft
Conversation
#314122) The observable passed to mapObservableArrayCached may transiently produce undefined before initialization completes. Add a guard to skip setItems when the read value is falsy, preventing 'e is not iterable' TypeError. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vs-code-engineering
Bot
requested review from
Copilot and
hediet
and removed request for
Copilot
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the Copilot extension’s observable utility mapObservableArrayCached when the upstream observable produces an invalid (non-iterable) value during initialization, by adding a guard before iterating/mapping.
Changes:
- Added an early-return guard in
mapObservableArrayCachedto avoid callingArrayMap.setItemswhen the read value is falsy. - Preserved the previously cached mapped items when the guard triggers.
Show a summary per file
| File | Description |
|---|---|
| extensions/copilot/src/util/vs/base/common/observableInternal/utils/utils.ts | Adds a guard in mapObservableArrayCached to avoid crashing when items.read(reader) is falsy. |
Copilot's findings
Comments suppressed due to low confidence (1)
extensions/copilot/src/util/vs/base/common/observableInternal/utils/utils.ts:253
- This file is marked as a copy of the upstream observable utils, but this change introduces a behavior difference from
src/vs/base/common/observableInternal/utils/utils.ts’smapObservableArrayCached. To avoid the fix being lost the next time the Copilot copy is resynced, consider applying the same guard upstream (or otherwise documenting/centralizing the divergence).
const i = items.read(reader);
if (!i) { return m.getItems(); }
m.setItems(i);
- Files reviewed: 1/1 changed files
- Comments generated: 2
| } | ||
| }, (reader) => { | ||
| const i = items.read(reader); | ||
| if (!i) { return m.getItems(); } |
| } | ||
| }, (reader) => { | ||
| const i = items.read(reader); | ||
| if (!i) { return m.getItems(); } |
Contributor
Author
Follow-up: Review Comment AnalysisTrigger: cron_check_failed Review Comments Addressed (fix ready but push blocked)
Suggested Change- if (!i) { return m.getItems(); }
- m.setItems(i);
+ if (!Array.isArray(i)) {
+ return m.getItems();
+ }
+ m.setItems(i);CI Status
Remaining Items
|
Contributor
Author
Follow-up: Review Comments AddressedTrigger: Changes Made
Suggested Diff const i = items.read(reader);
- if (!i) { return m.getItems(); }
+ if (!Array.isArray(i)) {
+ return m.getItems();
+ }
m.setItems(i);CI Status
StatusPush was blocked due to infrastructure constraints (subdirectory checkout). The fix above addresses both review comments and is ready to be applied manually or by a maintainer.
|
Contributor
Author
Follow-up: Review Comments Fix AppliedTrigger: Changes Made
Commit
Applied Diff }, (reader) => {
const i = items.read(reader);
- if (!i) { return m.getItems(); }
+ if (!Array.isArray(i)) {
+ return m.getItems();
+ }
m.setItems(i);
return m.getItems();CI Status
Status
|
Contributor
Author
Follow-up: Review Comments Fix AppliedTrigger: Changes Made
Applied Diff }, (reader) => {
const i = items.read(reader);
- if (!i) { return m.getItems(); }
+ if (!Array.isArray(i)) {
+ return m.getItems();
+ }
m.setItems(i);
return m.getItems();CI Status
Status
Remaining Items
|
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.
Summary
The
mapObservableArrayCachedutility in the Copilot extension observable framework crashes with "e is not iterable" when the source observable transiently producesundefinedor a non-array value during initialization. This occurs insetItems()which attemptsfor (const item of items)on the non-iterable value. The error affects 1,346+ users on the Agents product across win32 and darwin platforms.Fixes #314122
Recommended reviewer:
@hedietCulprit Commit
No specific culprit commit identified. The bug is a latent type-safety gap in
mapObservableArrayCached. The function signature declaresitems: IObservable<readonly TIn[]>but callers can pass observables whose initial value isundefinedbefore initialization completes. The bucket first appeared in v0.47.2026043003, likely triggered by a caller change that introduced a transiently-undefined observable. The utility code itself was last synced from VS Code sources in January 2026 (commit 311d7fa).Code Flow
sequenceDiagram participant Caller as "rP.init" participant MapObs as "mapObservableArrayCached" participant ArrMap as "ArrayMap.setItems" Caller->>MapObs: creates derived observable with items observable MapObs->>MapObs: items.read(reader) returns undefined MapObs->>ArrMap: setItems(undefined) Note over ArrMap: Error thrown:<br/>for-of fails because items is not iterableAffected Files
extensions/copilot/src/util/vs/base/common/observableInternal/utils/utils.tsRepro Steps
mapObservableArrayCachedhas not yet been assigned a value (still undefined), the derived observable recomputes and crashesHow the Fix Works
Chosen approach (
extensions/copilot/src/util/vs/base/common/observableInternal/utils/utils.tsline 251):Added a guard clause
if (!i) { return m.getItems(); }before callingm.setItems(i). This prevents the not-iterable error by returning the previously cached items when the source observable produces a falsy value. This fixes at the data-flow boundary where the observable runtime value diverges from its declared type — following the data-producer principle of fixing where invalid data enters rather than wrapping the crash site in try-catch.Recommended Owner
@hediet— original author of the observable utilities (committed the source copy from VS Code core)