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

Log unhandled Promise rejections #2318

Closed
mstoykov opened this issue Jan 6, 2022 · 1 comment
Closed

Log unhandled Promise rejections #2318

mstoykov opened this issue Jan 6, 2022 · 1 comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature

Comments

@mstoykov
Copy link
Contributor

mstoykov commented Jan 6, 2022

Feature Description

Currently if a promise gets rejected and there is no reject handle this will not be logged or noticed in any way, for example:

export default function() {
        new Promise((resolve, reject) => {
                setTimeout(resolve, 1000);
        }).then(()=> console.log("yey"));
}

will not print anything at all
but if you write

export default function() {
        new Promise((resolve, reject) => {
                setTimeout(resolve, 1000);
        }).then(()=> console.log("yey")).catch((e)=>console.log("oh", e));
}

you will get oh ReferenceError: setTimeout is not defined

Suggested Solution (optional)

The ECMAScript standard has a defined "callback" to handle this called HostPromiseRejectionTracker as noted there it gets called with the promise and one of two operations:

  1. "reject" - a Promise is rejected and it doesn't have rejection handler at this point.
  2. "handle" - a Promise that previously got called with the "reject" handler just got a rejection handler added.

Here the idea is that the host implementation (this is k6 ;) ) will hopefully have some logic that if it gets a "reject" it won't immediately just log it but wait for sometime, until some event?, both or a third option, before logging if a corresponding "handle" hasn't also arrived.

I found this out while working on #2228, and I quickly found out that a really good implementation is ... going to take some time. So instead I just logged it each time I got "reject" and did nothing on "handle".

This unfortunately means that at least for the moment, before an event loop and some API from k6 to return promises which will rejected/resolved through it, this will happen for all create Promises. As in the above example, the Promise "body" gets executed before new Promise returns so by the time .then or .catch is called on it, it's already too late - it was already rejected.

Now the current main usage is to polyfill async/await and as I noted in #779 (comment) this isn't really ... supported. Given that and the fact that otherwise it gets a bit too hard to debug even that I am kind of for the simplest solution, possibly even added to v0.36.0 and then with event loop PR(or after/before it) a more complicated and full implementation can be though of.

Implementation ideas/problems:

  1. Just keeping them until "handle" doesn't get called or the iteration ends means that it will be really confusing when things get logged.
  2. waiting some time or end of iteration seems like a good idea but it probably needs an "OOM" prevention to not wait on ... 1milltion unhandled rejections or something like that.
  3. Maybe we should have the above+ until the next time something gets logged, on the console.log or even at all ... This will really help with debugging, but also will be kind of convoluted to do IMO.

Already existing or connected issues / PRs (optional)

@mstoykov mstoykov added feature evaluation needed proposal needs to be validated or tested before fully implementing it in k6 labels Jan 6, 2022
mstoykov added a commit that referenced this issue Jan 6, 2022
This just logs every time a promise gets rejected but has not reject
handler (yet).

The message is basically the one firefox gives.

The simplest variant of #2318
@mstoykov
Copy link
Contributor Author

This was actually fixed in #2228 - unhandled failures also abort the iteration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature
Projects
None yet
Development

No branches or pull requests

1 participant