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

Update sobek to fix interrupt with async code #4017

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Update sobek to fix interrupt with async code #4017

merged 1 commit into from
Oct 25, 2024

Conversation

mstoykov
Copy link
Contributor

@mstoykov mstoykov commented Oct 25, 2024

What?

Also fix it through the codebase, although experimental/streams and fs likely will need more work.

As part of this change NewPromise's resolve and reject function now return error and that error is going to handle the interrupt - but probably it should be handled as any other error.

This mostly needs that it needs to be propagated upwards.

This also needs to be changed in different extensions, although there the work seems to be less.

Why?

Fix #3983 among other things.

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

Fixes #3983

Also fix it through the codebase, although experimental/streams and fs
likely will need more work.
@mstoykov mstoykov added this to the v0.55.0 milestone Oct 25, 2024
@mstoykov mstoykov requested a review from a team as a code owner October 25, 2024 07:38
@mstoykov mstoykov requested review from oleiade and olegbespalov and removed request for a team October 25, 2024 07:38
mstoykov added a commit to grafana/xk6-websockets that referenced this pull request Oct 25, 2024
mstoykov added a commit to grafana/xk6-redis that referenced this pull request Oct 25, 2024
This "automatically" fixes grafana/k6#4017 when this is build with a k6
version after that PR is merged.

Also reduces copy-pasted code.
mstoykov added a commit to grafana/xk6-browser that referenced this pull request Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and
Interrupt will work.

AbortingPromises seems to have not been used at all and just adds
complexity. And `promises.New()` already does the remaining things
needed to happen, so this can be merged even without grafana/k6#4017.
mstoykov added a commit to grafana/xk6-browser that referenced this pull request Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and
Interrupt will work.

AbortingPromises seems to have not been used at all and just adds
complexity. And `promises.New()` already does the remaining things
needed to happen, so this can be merged even without grafana/k6#4017.
mstoykov added a commit to grafana/xk6-browser that referenced this pull request Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and
Interrupt will work.

AbortingPromises seems to have not been used at all and just adds
complexity. And `promises.New()` already does the remaining things
needed to happen, so this can be merged even without grafana/k6#4017.
@mstoykov mstoykov merged commit 192a49e into master Oct 25, 2024
23 checks passed
@mstoykov mstoykov deleted the updateSobek branch October 25, 2024 08:33
mstoykov added a commit to grafana/xk6-websockets that referenced this pull request Oct 25, 2024
ankur22 pushed a commit to grafana/xk6-browser that referenced this pull request Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and
Interrupt will work.

AbortingPromises seems to have not been used at all and just adds
complexity. And `promises.New()` already does the remaining things
needed to happen, so this can be merged even without grafana/k6#4017.
ankur22 pushed a commit to grafana/xk6-browser that referenced this pull request Oct 25, 2024
This changes the promise helpers so after grafana/k6#4017 async code and
Interrupt will work.

AbortingPromises seems to have not been used at all and just adds
complexity. And `promises.New()` already does the remaining things
needed to happen, so this can be merged even without grafana/k6#4017.
@mstoykov mstoykov mentioned this pull request Oct 25, 2024
5 tasks
mstoykov added a commit to grafana/xk6-websockets that referenced this pull request Oct 25, 2024
mstoykov added a commit to grafana/xk6-redis that referenced this pull request Oct 26, 2024
This "automatically" fixes grafana/k6#4017 when this is build with a k6
version after that PR is merged.

Also reduces copy-pasted code.
oleiade added a commit that referenced this pull request Nov 11, 2024
oleiade added a commit that referenced this pull request Nov 11, 2024
* Initialize v0.55.0 release notes

* Update release notes for browser 1448

* Update release notes for browser page.on generalization

* Update release notes for browser 1439

* Update release notes for browser 1461

* Update release notes for browser 1496

I'm not yet sure if we should add this PR as it's a part of a bigger
functionality that we expect to have in k6 v0.56.

See the issue for more details:
grafana/xk6-browser#1289

* Replace issue 1461 with PR 1462

* release notes: webcrypto RSA support

* release notes: statsd output removal

* release notes: outputs changes

* Add browser related changes to release notes

* Remove template examples from release notes

* Add browser#1457 to release notes

* release notes: updates check recommendation

* Add initial page.on('metric') release note change

* Add issues to the page.on('metric') release note

* release notes: add TLA support section

* release notes: add tracing breaking change and future open ones

* auto-assigner changelog

* check recommendation apply feedback

* fixup! release notes: add tracing breaking change and future open ones

* Apply release notes clean-ups and fillers

* Address wrong formulation of tracing removal

* Update release notes/v0.55.0.md

Co-authored-by: Oleg Bespalov <oleg.bespalov@grafana.com>

* Apply suggestions from code review

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

* Apply suggestions from code review

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

* Apply pull request review suggestions

* Update the page.on('metric') description

* Update release notes/v0.55.0.md

Co-authored-by: Ankur <ankur.agarwal@grafana.com>

* Update page.on example as an expandable detail

* Align last sections format with template

* Update ControlOrMeta description

* Update wording of page.on example

* Apply pull request review suggestions

* Document #4017 bug fix

* Update release notes/v0.55.0.md

Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>

* Bring minor improvement to #4009

---------

Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>
Co-authored-by: Oleg Bespalov <oleg.bespalov@grafana.com>
Co-authored-by: ankur22 <ankur.agarwal@grafana.com>
Co-authored-by: Mihail Stoykov <M.Stoikov@gmail.com>
Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>
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.

abort after an await doesn't abort the test.
3 participants