1806731 - Anti-pattern of async/await in Promise constructor executor functions

Status: NEW

(Core :: General, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

There are 158 hits in the tree of the async/await inside Promise constructor executor function anti-pattern. This pattern has demonstrably poor error handling characteristics, leading to uncaught errors, as described in the link.

Bad patterns tend to spread unless weeded, so the sooner we root this out the better.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:marco, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Qijia Liu]

Attachment: Bug 1806731 - Convert async in Promise constructor to async function call. r?jib

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Comment 4

[Lata]

Hey! I want to give it a try. Can you please assign me this or I should go ahead and submit a patch?

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Attachment: WIP: Bug 1806731: Remove anti-pattern of async/await in Promise constructor executor functions.

Comment 6

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Hi Lata, that would be great!

Note I briefly started on this around the time I filed the issue, but got side-tracked. So if you have cycles to pick it up that would be great!

I've uploaded a patch of how far I got. I didn't get very far, as you can see, but hopefully it is helpful to get a sense of what is needed.

Challenges I ran into:

1. Each fix turned out to be more demanding and less rote than I thought, often requiring insight into what was going on. I.e. not sure I would trust GPT-4 with this.
2. A lot of the hits appear to be for tests that aren't passing. I suggest only touching tests that pass, because that way we have some measure of success.
3. I wasn't sure who would review it all, since it cuts across many folders, e.g. if some areas require review by certain people, but I'm happy to be the reviewer!

Comment 7

[Lata]

Hey! Sorry for the late reply. I have been working on other bug. I will look into your patch and try to figure out what needs to be done next. Thanks.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

This good-first-bug hasn't had any activity for 2 months, it is automatically unassigned.
For more information, please visit BugBot documentation.

Comment 9

[ram]

Hello!
I can work on this bug if it is still relevant.

Comment 10

[Konrad Kügler [:kkuegler]]

Attachment: Bug 1806731 - Replace async Promise constructor executor functions r=jib

This changeset only includes the somewhat straight-forward cases.

Approaches:

• Removing unused 'async' keywords.
• Removing the promise constructor altogether/reducing its scope
  by lifting all/some executor code up into the surrounding async
  function.

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 12

[Ali Yousef]

Hello there!
I can work on this bug if it is still available.

Comment 13

[Ali Yousef]

(In reply to Ali Yousef from comment #12)

Hello there!
I want to give it a try . Would you please assign me !

Comment 14

[Ali Yousef]

(In reply to Ali Yousef from comment #13)

(In reply to Ali Yousef from comment #12)

Hello there!
I want to give it a try . Would you please assign me !

Sorry for this mistake. I am new to Bugzilla, and what I wanted is to edit my first comment not to add second one . I did not realized that editing not allowed !

I want to give this bug a try . Would you please assign me !

Comment 15

[Mayank Bansal]

Usually the bug is assigned once the contributor attaches a patch :)

Comment 16

[Ali Yousef]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #6)

2. A lot of the hits appear to be for tests that aren't passing. I suggest only touching tests that pass, because that way we have some measure of success.

what commands I should run to see what tests aren't passing !

Comment 17

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

If we focus on the WPT tests (inside testing/web-platform/tests/), then use mach wpt <path/folder/file.html> to run tests locally.

See also testing/web-platform/meta/ where known failing tests are annotated/turned off.

For quick view there's also https://wpt.fyi/results (but that's outside our tree)