Fix IteratorClose omission in IteratorRecord (IteratorStep / IteratorStepValue)

[mrhapile]

Fix IteratorClose omission in IteratorRecord (IteratorStep / IteratorStepValue)

Background

ECMA-262 requires that iterators are properly closed when an abrupt completion occurs during iteration.

Specifically:

• §7.4.6 IteratorStep
• §7.4.7 IteratorStepValue

Both require the use of IfAbruptCloseIterator, which ensures that the iterator’s return method is invoked if accessing:

• done (IteratorComplete)
• value (IteratorValue)

throws an exception.

---

Problem

The current implementation of:

• IteratorRecord::step
• IteratorRecord::step_value

propagates errors using ? without invoking IteratorClose, which can leave iterators in an unclosed state when:

• done getter throws
• value getter throws

This violates ECMAScript semantics and may lead to incorrect iterator behavior.

---

Changes

• Replaced ? with explicit error handling in both methods
• On abrupt completion:
  • Call iterator.close(Err(err.clone()), context)
  • Explicitly drop the result using drop(...)
  • Return the original error (preserving completion semantics)

---

Example Scenario

let closed = false;

let iter = {
  next() {
    return {
      get done() {
        throw new Error("boom");
      }
    };
  },
  return() {
    closed = true;
    return {};
  }
};

try {
  iter.next();
} catch {}

console.log(closed); // should be true

[github-actions]

Test262 conformance changes

Test result	main count	PR count	difference
Total	52,963	52,963	0
Passed	50,545	50,533	-12
Ignored	1,426	1,426	0
Failed	992	1,004	+12
Panics	2	2	0
Conformance	95.43%	95.41%	-0.02%

Broken tests (12):

test/built-ins/Promise/all/iter-step-err-no-close.js (previously Passed)
test/built-ins/Promise/all/iter-next-val-err-no-close.js (previously Passed)
test/built-ins/Promise/any/iter-step-err-no-close.js (previously Passed)
test/built-ins/Promise/any/iter-next-val-err-no-close.js (previously Passed)
test/built-ins/Promise/allSettled/iter-step-err-no-close.js (previously Passed)
test/built-ins/Promise/allSettled/iter-next-val-err-no-close.js (previously Passed)
test/built-ins/Promise/race/iter-step-err-no-close.js (previously Passed)
test/built-ins/Promise/race/iter-next-val-err-no-close.js (previously Passed)
test/staging/sm/Iterator/prototype/lazy-methods-throw-next-done-throws.js (previously Passed)
test/staging/sm/Iterator/prototype/lazy-methods-iterator-not-closed-on-value-throws.js (previously Passed)
test/staging/sm/Map/constructor-iterator-close.js (previously Passed)
test/staging/sm/Array/from-iterator-close.js (previously Passed)

Tested main commit: 5b0f62ad860a2f5e2a4631ac2f280642cf7e21ea
Tested PR commit: 947e67ea34e14391807f4a46bee03277b752534a
Compare commits: 5b0f62a...947e67e

[mrhapile]

[mrhapile]

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.87%. Comparing base (6ddc2b4) to head (947e67e).
⚠️ Report is 917 commits behind head on main.

Files with missing lines	Patch %	Lines
core/engine/src/builtins/iterable/mod.rs	50.00%	6 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5245       +/-   ##
===========================================
+ Coverage   47.24%   59.87%   +12.63%     
===========================================
  Files         476      582      +106     
  Lines       46892    63469    +16577     
===========================================
+ Hits        22154    38002    +15848     
- Misses      24738    25467      +729     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jedel1043]

Printing false in your example is actually the correct behaviour. ECMAScript semantics don't ensure that all iterators are closed, it just ensures all iterators that are iterated using a for [await] of loop are closed.

You can also compare with node:

node <<EOF
let closed = false;

let iter = {
  next() {
    return {
      get done() {
        throw new Error("boom");
      },
    };
  },
  return() {
    closed = true;
    return {};
  },
};

try {
  iter.next();
} catch {}

console.log(closed); // should be true
EOF

false