Skip to content

iterator helpers: work around IteratorClose swallowing errors - #5152

Open
bakkot wants to merge 1 commit into
mainfrom
not-mask-return-throw
Open

bakkot wants to merge 1 commit into
mainfrom
not-mask-return-throw

Conversation

@bakkot

@bakkot bakkot commented Oct 19, 2026

Copy link
Copy Markdown
Member

These tests all involve an iterator that violates the iterator protocol by having .next() either throw or return an object with a throwy .done or .value getter. In these case the .return() method is not called, which the tests were supposed to be catching by having it throw an error and asserting a different error was thrown.

But, IteratorClose in the case of an error swallows any errors thrown by .return(), so they'd never see the one from .return().

Work around this by having .return() set a flag, then asserting it wasn't set.

@bakkot
bakkot requested a review from a team as a code owner October 19, 2026 04:58
@github-actions

Copy link
Copy Markdown

39 new or modified tests were run on 7 engines.

View workflow run

Test results
Test Engine262 GraalJS JavaScriptCore QuickJS SpiderMonkey V8 XS
built-ins/Iterator/concat/next-method-returns-throwing-done.js ✅ ❌ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/concat/next-method-returns-throwing-value.js ✅ ❌ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/chunks/get-next-method-throws.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/chunks/next-method-returns-non-object.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/chunks/next-method-returns-throwing-done.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/chunks/next-method-returns-throwing-value.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/chunks/next-method-throws.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/drop/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/drop/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/every/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/every/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/filter/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/filter/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/find/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/find/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/flatMap/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/flatMap/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/forEach/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/forEach/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/includes/get-next-method-throws.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/includes/next-method-returns-non-object.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/includes/next-method-returns-throwing-done.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/includes/next-method-returns-throwing-value.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/includes/next-method-throws.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/map/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/map/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/reduce/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/reduce/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/some/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/some/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/take/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/take/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/toArray/next-method-returns-throwing-done.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/toArray/next-method-returns-throwing-value.js ✅ ✅ ✅ ✅ ✅ ❌ ✅
built-ins/Iterator/prototype/windows/get-next-method-throws.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/windows/next-method-returns-non-object.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/windows/next-method-returns-throwing-done.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/windows/next-method-returns-throwing-value.js ✅ ✅ ✅ ❌ ✅ ❌ ❌
built-ins/Iterator/prototype/windows/next-method-throws.js ✅ ✅ ✅ ❌ ✅ ❌ ❌

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants