Skip to content
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

Dont lock the event loop once a promise rejection is left unhandled #2693

Merged
merged 1 commit into from
Sep 27, 2022

Conversation

mstoykov
Copy link
Contributor

Previously when a promise is rejected but not handled, the event loop would end the current iteration and then let a new one start.

Unfortuantely it also didn't clear the failure so the next time it will again fail the iteration as it still thinks a promise has been rejected without a handler.

Heavily related to grafana/xk6-browser#510

Previously when a promise is rejected but *not* handled, the event loop
would end the current iteration and then let a new one start.

Unfortuantely it also didn't clear the failure so the next time it will
again *fail* the iteration as it still thinks a promise has been
rejected without a handler.

Heavily related to grafana/xk6-browser#510
@mstoykov mstoykov requested review from imiric and removed request for oleiade and codebien September 26, 2022 13:53
@mstoykov mstoykov added this to the v0.41.0 milestone Sep 26, 2022
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

Thanks for spotting this and the fix! 🙇🎉

I spent way too long troubleshooting the xk6-browser behavior, and it turned out to be a one-line k6 fix... 😮‍💨🤦

@mstoykov
Copy link
Contributor Author

I spent way too long troubleshooting the xk6-browser behavior, and it turned out to be a one-line k6 fix... face_exhalingfacepalm

Yeah, I am really surprised we didn't notice it earlier. As I elude in the commit message this in practice makes the whole k6+promises workflow not work the moment you have any unhandled promise rejection.

I guess that I just always write my scripts with .reject somewhere in it and the majority of other people either rarely use promises or just taught it is a problem with their script. Or in this case with xk6-browser ...

Copy link
Member

@oleiade oleiade left a comment

Choose a reason for hiding this comment

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

I might have run into this without realizing in the past 👀
LGTM 👍🏻

@mstoykov mstoykov merged commit b5bbf7e into master Sep 27, 2022
@mstoykov mstoykov deleted the clearPromiseRejections branch September 27, 2022 10:10
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.

3 participants