Skip to content

Fixup assertion message in AsyncGenerator.p.return with broken promises #4486

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

legendecas
Copy link
Member

@@ -53,7 +53,7 @@ it.return(brokenPromise)
throw new Test262Error("Expected rejection");
},
err => {
assert(unblocked, false, 'return should be rejected before generator is resumed');
assert(unblocked, 'return should be rejected when the generator is completed');
Copy link
Member

@Jack-Works Jack-Works May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert(unblocked, 'return should be rejected when the generator is completed');
assert(unblocked, 'return should be rejected after the generator is resumed');

tc39/ecma262#3596 (comment)

Copy link
Member Author

@legendecas legendecas May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is describing the state of the async generator, which is completed though. See the info section:

  AsyncGenerator.prototype.return ( value )
  ...
  8. If state is either suspendedStart or completed, then
    a. Set generator.[[AsyncGeneratorState]] to awaiting-return.
    b. Perform ! AsyncGeneratorAwaitReturn(generator).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@legendecas In this test, the state of the generator when .return is called is ~executing~, not ~completed~, because at the time .return is called the generator is paused at await blocking. So the quoted step doesn't apply.

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.

4 participants