Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Close sync iterables when they yield rejected promises #49

Open
3 tasks done
js-choi opened this issue Apr 1, 2025 · 3 comments · May be fixed by tc39/test262#4450
Open
3 tasks done

Close sync iterables when they yield rejected promises #49

js-choi opened this issue Apr 1, 2025 · 3 comments · May be fixed by tc39/test262#4450
Assignees

Comments

@js-choi
Copy link
Collaborator

js-choi commented Apr 1, 2025

Previously, for await did not close sync iterables when they yield rejected promises (tc39/ecma262#1849):

Code example
function * createIter() {
  try {
    yield Promise.resolve(console.log("a"));
    yield Promise.reject("x");
  } finally {
    console.log("finalized");
  }
}

// Prints "a" and then prints "finalized".
// There is an uncaught "x" rejection.
for (const x of createIter()) {
  console.log(await x);
}

// Prints "a" and then prints "finalized".
// There is an uncaught "x" rejection.
Array.from(createIter());

A month ago, the editors merged tc39/ecma262#2600 into the language. Now, for await closes sync iterables when they yield rejected promises. Array.fromAsync needs to change to do the same. See #39.

@js-choi js-choi changed the title Close Close sync iterables when they yield rejected promises Apr 1, 2025
@bakkot
Copy link
Contributor

bakkot commented Apr 1, 2025

It actually doesn't need any changes to the spec: Array.fromAsync uses CreateAsyncFromSyncIterator, and that's where the change happened.

Specifically, the Let nextResult be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]]). step will call %AsyncFromSyncIteratorPrototype%.next, which calls AsyncFromSyncIteratorContinuation passing true for closeOnRejection, which causes it to close the underlying sync iterable when a rejected promises is yielded (as part of the rejection handler specified in step 13.a).

However, the explainer does indeed need to be updated. Also tests should be updated (or written).

js-choi added a commit that referenced this issue Apr 5, 2025
@js-choi js-choi mentioned this issue Apr 5, 2025
5 tasks
js-choi added a commit to js-choi/test262 that referenced this issue Apr 8, 2025
Adds
built-ins/Array/fromAsync/sync-iterable-with-rejecting-thenable-closes.js.

This is
related to tc39/ecma262#1849 and tc39/ecma262#2600.
This closes tc39/proposal-array-from-async#49.

Renames
built-ins/Array/fromAsync/sync-iterable-with-thenable-element-rejects.js
to
built-ins/Array/fromAsync/sync-iterable-with-rejecting-thenable-closes.js
for consistency with the new test.
@js-choi
Copy link
Collaborator Author

js-choi commented Apr 8, 2025

A test262 pull request is pending (tc39/test262#4450).

@bakkot: I see a “Note about changed behavior of Array.fromAsync after landing #2600” item in the 2025-04 plenary agenda. Is there anything I need to do on my end regarding this agenda item?

@bakkot
Copy link
Contributor

bakkot commented Apr 8, 2025

I had genuinely forgotten I'd added that! But no, nothing from you; that item is just to inform engines that this change to fromAsync had happened as a result of the 262 editors merging the old normative PR to 262.

@js-choi js-choi self-assigned this Apr 10, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants