Stop a closing iterator from swallowing the error that closed it

[lahma]

Every %Iterator.prototype% and %AsyncIterator.prototype% helper that validates an argument does so before building the iterator record, and closes the receiver when validation fails — map/filter/flatMap/forEach/some/every/find/reduce on a non-callable, take/drop on a NaN or negative limit or an argument whose ToNumber throws, and (since #2867) join on a separator whose ToString throws.

IteratorClose reaches step 4 — "If completion is a throw completion, return ? completion" — before it ever inspects innerResult, so nothing raised while closing may replace the error that triggered the close:

2. Let innerResult be Completion(GetMethod(iterator, "return")).
3. If innerResult is a normal completion, then
   a. Let return be innerResult.[[Value]].
   b. If return is undefined, return ? completion.
   c. Set innerResult to Completion(Call(return, iterator)).
4. If completion is a throw completion, return ? completion.   <-- before step 5
5. If innerResult is a throw completion, return ? innerResult.

The private IteratorClose helper on each of those two prototypes honoured that for the call in step 3.c and not for the GetMethod in step 2 — the lookup sat outside the try/catch when, so it escaped.

Reproduction

const closable = {
  __proto__: Iterator.prototype,
  get return() { throw new Error('mine'); },
};

closable.map();        // Error: mine        — must be TypeError
closable.take(NaN);    // Error: mine        — must be RangeError

const weird = { __proto__: Iterator.prototype, return: 42 };
weird.take(NaN);       // TypeError          — must be RangeError

The second shape is the sharper one: GetMethod throws a TypeError of its own for a return that is present and not callable, so the error a caller catches changes type, not just message. Both shapes reproduce identically on AsyncIterator.prototype.

test262 does not cover either — its get-return-method-throws.js tests exercise the helper's own return(), a different path — which is why this went unnoticed. All 23 affected entry points (12 sync + 11 async) are wrong; the receiver's error wins every time.

Fix

Both helpers now wrap the lookup in the same catch when (completionType == CompletionType.Throw) the call already had, which is exactly what IteratorInstance.ObjectIterator.Close — the record-based close used once iteration is under way — has always done. The close still happens; only its own error is suppressed, and only when the completion being carried is already a throw.

JintYieldExpression.AsyncIteratorClose has a similar shape and is deliberately left alone: both of its call sites pass CompletionType.Normal, so the branch that would carry the same bug is unreachable.

Verification

The new test cases in IteratorHelpersTests were run against the unfixed engine first, on this exact rebased base — 23 of the file's 32 cases fail without the change, all 32 pass with it. The 9 that pass either way are the pre-existing toArray/join tests plus ClosingTheReceiverStillHappens, which pins that the close is still performed — the fix suppresses the close's own error, not the close. Each theory also folds in an already-correct throwing-return() control, so a fix that over-suppressed would be caught too.

Full Release run on this branch:

suite	result
Jint.Tests.Test262	99484 passed, 0 failed, 157 skipped
Jint.Tests (net10.0 / net472)	4660 / 4579 passed, 0 failed
Jint.Tests.PublicInterface (net10.0 / net472)	1240 / 1239 passed, 0 failed

Rebased onto main after #2867 merged; join closes through the same helper, so it joined the coercion-error coverage.

🤖 Generated with Claude Code