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

Change how group() calls async functions #2848

Open
mstoykov opened this issue Jan 12, 2023 · 1 comment
Open

Change how group() calls async functions #2848

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

Comments

@mstoykov
Copy link
Contributor

As of #2830 k6 has support async functions.

A such the following code is now valid

group("somename", async () => {
    console.log("A");
    await 5;
    console.log("B");
})
console.log("C")

And will print A, C, B (with new lines in between).

This is okay but after an internal poll most people expect C, A, B - running the remaining parts before calling the async function.

Given that irregardless of what people expect the returned value of group will be a promise so in order to get it the stack will need to be empty.

It seems more reasonable to not have some parts of the group finish before continuing and some to finish.

Proposed solution:

Have group recognise that it got an async function. And queue it's calling (with the appropriate setting of group tag) on the event loop. Or create another promise that immediately is resolved and do the actual group call in the resolution callback.

The difference between the two is that using a promise this will be called before anything else that will be called from the event loop. And as such also before any other immediately resolved promise created after the call to group.

So the difference in example is:

group("somename", async () => {
    console.log("A");
    await 5;
    console.log("B");
})
console.log("C")
Promise.resolve("D").then((s) => {console.log(s)}); // (Promise.resolve returns a resolved promise with the provided value)

Wil in the one case write C, D, A, B in the other C, A, D, B.
C, D, A, B as the resolved promise will be executed before anything from the k6 event loop can be executed.
C, A, D, B as the promise resolution of the group will be queued before the D one but then the await will once again

Recognising it seems to be possible if we take the callback argument as goja.Value and get its Prototype.

FAQ:

Why not A, B, C?

This is currently possible if the code is

await group("somename", async () => {
    console.log("A");
    await 5;
    console.log("B");
})
console.log("C")

Which also requires that the code is within an async function in order for await to not be a syntax error.

This will still be possible after that as well.

Is this a breaking change?

Given that async function was not supported up until this point - I would argue not. AFAIK the ways to polyfill this does not return the same Prototype so that will still work the same way as before.

@mstoykov mstoykov added the ux label Jan 12, 2023
@mstoykov mstoykov added this to the v0.43.0 milestone Jan 12, 2023
mstoykov added a commit that referenced this issue Jan 19, 2023
A group provided with an async function will now:
1. Treat the whole time it take for the async function promise to finisher
   the duration of the group.
2. It will also use goja's AsyncContextTracker to make it so that the code
   using `await` within the group will still continue to be tagged with the
   group after `await` returns.
3. Instead of directly calling the async function it schedules it to be
   called the next time a promise job will work.

The current AsyncContextTracker is only used for this and as such is
directly changed in the `k6` module. In the future there likely will be
API so that multiple modules can use it simultaneously, but that seems
way too involved to be included in this change and also currently only
`group` needs this.

fixes #2848 #2728
mstoykov added a commit that referenced this issue Jan 19, 2023
A group provided with an async function will now:
1. Treat the whole time it take for the async function promise to finisher
   the duration of the group.
2. It will also use goja's AsyncContextTracker to make it so that the code
   using `await` within the group will still continue to be tagged with the
   group after `await` returns.
3. Instead of directly calling the async function it schedules it to be
   called the next time a promise job will work.

The current AsyncContextTracker is only used for this and as such is
directly changed in the `k6` module. In the future there likely will be
API so that multiple modules can use it simultaneously, but that seems
way too involved to be included in this change and also currently only
`group` needs this.

fixes #2848 #2728
@mstoykov mstoykov added the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Jan 26, 2023
@mstoykov
Copy link
Contributor Author

Blocked on #2728 (comment)

@mstoykov mstoykov removed this from the v0.43.0 milestone Jan 27, 2023
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 ux
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant