lib: add aborted() utility function#46494

debadree25 · 2023-02-04T10:34:32Z

debadree25 · 2023-02-04T10:37:50Z

I think this would require further discussion since the conversation on the original issue stopped at 2021, nonetheless have tried to implement the discussed details until then and have added tests

cc @benjamingr, @jasnell

aduh95

This would need documentation, as well as tests with invalid types/values to ensure we correctly validate the input.

debadree25 · 2023-02-04T10:51:05Z

Yes adding to those tests,

Docs can I update after this is confirmed that this change is acceptable?

lib/internal/abort_controller.js

bnoordhuis · 2023-02-04T12:59:46Z

test/parallel/test-aborted-util.js

+constac=newAbortController();
+aborted(ac.signal,{}).then(common.mustNotCall());
+setImmediate(()=>{
+global.gc();


What does GC accomplish here? That the aborted() promise is collected before abort() is called?
I don't know if that kind of GC observability is a good thing. Maybe it works now but GC changes can break such patterns. Looks fragile.

basically want to cleanup the empty object to check if the listener is removed when the resource is cleaned by the gc, unsure of any other ways to test this

If it's just for testing, then it's probably fine. I mean, it could still break in the future but we can revisit if and when that happens.
I'd advise against documenting that behavior though. It's simply not very robust.

lib/internal/abort_controller.js

benjamingr

Mostly LGTM, missing docs (this should be marked as experimental, and as for where it should live I'll ping some TSC folk and ask)

benjamingr · 2023-02-04T18:11:50Z

@ronag @mcollina @jasnell where would a method for (safely, memory leak wise) getting a promise for when a signal is aborted live?

events? util?

debadree25 · 2023-02-04T18:56:10Z

Have added to the docs, @benjamingr @aduh95

doc/api/util.md

lib/internal/abort_controller.js

doc/api/util.md

test/parallel/test-aborted-util.js

Co-authored-by: Antoine du Hamel <[email protected]>

debadree25 · 2023-02-06T17:53:33Z

lib/internal/abort_controller.js

+ */
+asyncfunctionaborted(signal,resource){
+if(signal===undefined){
+thrownewERR_INVALID_ARG_TYPE('signal','AbortSignal',signal);


Have added this check since turns out changing the condition in validateAbortSignal() immediately results in the build breaking, so i assume its probably not a bug but would like a review here

It seems like a common pattern across to the codebase to check for undefined and then validate the signal so i think its probably fine?

nodejs-github-bot · 2023-02-07T10:44:50Z

CI: https://ci.nodejs.org/job/node-test-pull-request/49463/

nodejs-github-bot · 2023-02-07T17:44:00Z

Landed in 6d584ae

Fixes: #37220 Refs: #36607 PR-URL: #46494 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

Notable changes: deps: * upgrade npm to 9.5.0 (npm team) #46673 * add ada as a dependency (Yagiz Nizipli) #46410 doc: * add debadree25 to collaborators (Debadree Chatterjee) #46716 * add deokjinkim to collaborators (Deokjin Kim) #46444 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 lib: * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494 src: * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038 * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583 * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) #46491 * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) #45888 * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) #45888 * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) #45888 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368 stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273 test_runner: * add initial code coverage support (Colin Ihrig) #46017 url: * replace url-parser with ada (Yagiz Nizipli) #46410 PR-URL: TODO

Notable changes: deps: * upgrade npm to 9.5.0 (npm team) #46673 * add ada as a dependency (Yagiz Nizipli) #46410 doc: * add debadree25 to collaborators (Debadree Chatterjee) #46716 * add deokjinkim to collaborators (Deokjin Kim) #46444 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 lib: * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494 src: * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038 * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583 * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) #46491 * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) #45888 * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) #45888 * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) #45888 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368 stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273 test_runner: * add initial code coverage support (Colin Ihrig) #46017 url: * replace url-parser with ada (Yagiz Nizipli) #46410 PR-URL: #46725

Notable changes: deps: * upgrade npm to 9.5.0 (npm team) nodejs#46673 * add ada as a dependency (Yagiz Nizipli) nodejs#46410 doc: * add debadree25 to collaborators (Debadree Chatterjee) nodejs#46716 * add deokjinkim to collaborators (Deokjin Kim) nodejs#46444 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) nodejs#46017 lib: * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) nodejs#46494 src: * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) nodejs#45038 * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) nodejs#46583 * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) nodejs#46491 * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) nodejs#45888 * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) nodejs#45888 * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) nodejs#45888 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) nodejs#46368 stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) nodejs#46273 test_runner: * add initial code coverage support (Colin Ihrig) nodejs#46017 url: * replace url-parser with ada (Yagiz Nizipli) nodejs#46410 PR-URL: nodejs#46725

Notable changes: deps: * upgrade npm to 9.5.0 (npm team) #46673 * add ada as a dependency (Yagiz Nizipli) #46410 doc: * add debadree25 to collaborators (Debadree Chatterjee) #46716 * add deokjinkim to collaborators (Deokjin Kim) #46444 doc,lib,src,test: * rename --test-coverage (Colin Ihrig) #46017 lib: * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494 src: * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038 * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583 * (SEMVER-MINOR) allow blobs in addition to `FILE*`s in embedder snapshot API (Anna Henningsen) #46491 * (SEMVER-MINOR) allow snapshotting from the embedder API (Anna Henningsen) #45888 * (SEMVER-MINOR) make build_snapshot a per-Isolate option, rather than a global one (Anna Henningsen) #45888 * (SEMVER-MINOR) add snapshot support for embedder API (Anna Henningsen) #45888 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368 stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273 test_runner: * add initial code coverage support (Colin Ihrig) #46017 url: * replace url-parser with ada (Yagiz Nizipli) #46410 PR-URL: #46725

Fixes: #37220 Refs: #36607 PR-URL: #46494 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

Notable changes: * buffer: * (SEMVER-MINOR) add Buffer.copyBytesFrom(...) (James M Snell) #46500 * doc: * add marco-ippolito to collaborators (Marco Ippolito) #46816 * add debadree25 to collaborators (Debadree Chatterjee) #46716 * add deokjinkim to collaborators (Deokjin Kim) #46444 * events: * (SEMVER-MINOR) add listener argument to listenerCount (Paolo Insogna) #46523 * lib: * (SEMVER-MINOR) add AsyncLocalStorage.bind() and .snapshot() (flakey5) #46387 * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494 * src: * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368 * stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273 * tls: * (SEMVER-MINOR) support automatic DHE (Tobias Nießen) #46978 * url: * (SEMVER-MINOR) implement URLSearchParams size getter (James M Snell) #46308 * worker: * (SEMVER-MINOR) add support for worker name in inspector and trace_events (Debadree Chatterjee) #46832 PR-URL: TBD

Notable changes: * buffer: * (SEMVER-MINOR) add Buffer.copyBytesFrom(...) (James M Snell) #46500 * doc: * add marco-ippolito to collaborators (Marco Ippolito) #46816 * add debadree25 to collaborators (Debadree Chatterjee) #46716 * add deokjinkim to collaborators (Deokjin Kim) #46444 * events: * (SEMVER-MINOR) add listener argument to listenerCount (Paolo Insogna) #46523 * lib: * (SEMVER-MINOR) add AsyncLocalStorage.bind() and .snapshot() (flakey5) #46387 * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494 * src: * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368 * stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273 * tls: * (SEMVER-MINOR) support automatic DHE (Tobias Nießen) #46978 * url: * (SEMVER-MINOR) implement URLSearchParams size getter (James M Snell) #46308 * worker: * (SEMVER-MINOR) add support for worker name in inspector and trace_events (Debadree Chatterjee) #46832 PR-URL: #47502

Notable changes: * buffer: * (SEMVER-MINOR) add Buffer.copyBytesFrom(...) (James M Snell) #46500 * doc: * add marco-ippolito to collaborators (Marco Ippolito) #46816 * add debadree25 to collaborators (Debadree Chatterjee) #46716 * add deokjinkim to collaborators (Deokjin Kim) #46444 * events: * (SEMVER-MINOR) add listener argument to listenerCount (Paolo Insogna) #46523 * lib: * (SEMVER-MINOR) add AsyncLocalStorage.bind() and .snapshot() (flakey5) #46387 * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494 * src: * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368 * (SEMVER-MINOR) add initial support for single executable applications (Darshan Sen) #45038 * stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273 * tls: * (SEMVER-MINOR) support automatic DHE (Tobias Nießen) #46978 * url: * (SEMVER-MINOR) implement URLSearchParams size getter (James M Snell) #46308 * worker: * (SEMVER-MINOR) add support for worker name in inspector and trace_events (Debadree Chatterjee) #46832 PR-URL: #47502

Notable changes: Add initial support for single executable applications Compile a JavaScript file into a single executable application: ```console $ echo 'console.log(`Hello, ${process.argv[2]}!`);' > hello.js $ cp $(command -v node) hello $ npx postject hello NODE_JS_CODE hello.js \ --sentinel-fuse NODE_JS_FUSE_fce680ab2cc467b6e072b8b5df1996b2 $ npx postject hello NODE_JS_CODE hello.js \ --sentinel-fuse NODE_JS_FUSE_fce680ab2cc467b6e072b8b5df1996b2 \ --macho-segment-name NODE_JS $ ./hello world Hello, world! ``` Contributed by Darshan Sen in #45038 Replace url parser with Ada Node.js gets a new URL parser called Ada that is compliant with the WHATWG URL Specification and provides more than 100% performance improvement to the existing implementation. Contributed by Yagiz Nizipli in #46410 Other notable changes: * buffer: * (SEMVER-MINOR) add Buffer.copyBytesFrom(...) (James M Snell) #46500 * doc: * add marco-ippolito to collaborators (Marco Ippolito) #46816 * add debadree25 to collaborators (Debadree Chatterjee) #46716 * add deokjinkim to collaborators (Deokjin Kim) #46444 * events: * (SEMVER-MINOR) add listener argument to listenerCount (Paolo Insogna) #46523 * lib: * (SEMVER-MINOR) add AsyncLocalStorage.bind() and .snapshot() (flakey5) #46387 * (SEMVER-MINOR) add aborted() utility function (Debadree Chatterjee) #46494 * src: * (SEMVER-MINOR) allow optional Isolate termination in node::Stop() (Shelley Vohr) #46583 * (SEMVER-MINOR) allow embedder control of code generation policy (Shelley Vohr) #46368 * stream: * (SEMVER-MINOR) add abort signal for ReadableStream and WritableStream (Debadree Chatterjee) #46273 * tls: * (SEMVER-MINOR) support automatic DHE (Tobias Nießen) #46978 * url: * (SEMVER-MINOR) implement URLSearchParams size getter (James M Snell) #46308 * worker: * (SEMVER-MINOR) add support for worker name in inspector and trace_events (Debadree Chatterjee) #46832 PR-URL: #47502

lib: add aborted() utility function
6e797f5
Fixes: nodejs#37220 Refs: nodejs#36607

nodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Feb 4, 2023

aduh95 reviewed Feb 4, 2023
View reviewed changes

aduh95 added the semver-minor PRs that contain new features and should be released in the next minor version. label Feb 4, 2023

fixup! update param validation
3834e9f

bnoordhuis reviewed Feb 4, 2023
View reviewed changes

benjamingr reviewed Feb 4, 2023
View reviewed changes

lib/internal/abort_controller.js Outdated Show resolvedHide resolved

benjamingr reviewed Feb 4, 2023
View reviewed changes

lib/internal/abort_controller.jsShow resolvedHide resolved

benjamingr reviewed Feb 4, 2023
View reviewed changes

debadree25 added 3 commits February 4, 2023 23:47

fixup! make async func
1cab0ca

fixup! make resource compulsory
2e41a14

fixup! add doc
0a58ffc

debadree25 added 2 commits February 5, 2023 00:31

fixup! doc fixup
f3621fd

fixup! doc fixup
3bfeec0

benjamingr reviewed Feb 4, 2023
View reviewed changes

doc/api/util.md Outdated Show resolvedHide resolved

jasnell reviewed Feb 4, 2023
View reviewed changes

doc/api/util.md Outdated Show resolvedHide resolved

jasnell reviewed Feb 4, 2023
View reviewed changes

doc/api/util.md Outdated Show resolvedHide resolved

jasnell reviewed Feb 4, 2023
View reviewed changes

doc/api/util.md Outdated Show resolvedHide resolved

fixup! doc
a2f2c56

debadree25 commented Feb 4, 2023
View reviewed changes

lib/internal/abort_controller.jsShow resolvedHide resolved

aduh95 reviewed Feb 4, 2023
View reviewed changes

doc/api/util.md Outdated Show resolvedHide resolved
doc/api/util.md Outdated Show resolvedHide resolved
test/parallel/test-aborted-util.js Outdated Show resolvedHide resolved
test/parallel/test-aborted-util.js Outdated Show resolvedHide resolved

aduh95 reviewed Feb 4, 2023
View reviewed changes

test/parallel/test-aborted-util.jsShow resolvedHide resolved

debadree25and others added 3 commits February 5, 2023 11:07

Update doc/api/util.md
f076729
Co-authored-by: Antoine du Hamel <[email protected]>

Update doc/api/util.md
6334317
Co-authored-by: Antoine du Hamel <[email protected]>

Update test/parallel/test-aborted-util.js
7ff30c8
Co-authored-by: Antoine du Hamel <[email protected]>

fixup! add undefined check
f106f87

debadree25 commented Feb 6, 2023
View reviewed changes

github-actionsbot mentioned this pull request Feb 7, 2023
CI Reliability 2023-02-07 nodejs/reliability#498
Open
18 tasks

aduh95 approved these changes Feb 7, 2023
View reviewed changes

aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 7, 2023

github-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 7, 2023

aduh95 added experimental Issues and PRs related to experimental features. commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Feb 7, 2023

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 7, 2023

nodejs-github-bot merged commit 6d584ae into nodejs:mainFeb 7, 2023

MylesBorins mentioned this pull request Feb 19, 2023
v19.7.0 proposal #46725
Merged

debadree25 mentioned this pull request Mar 9, 2023
fetch() AbortSignal.timeout() does not work as expected nodejs/undici#1926
Closed

danielleadams mentioned this pull request Apr 11, 2023
v18.16.0 release proposal #47502
Merged

Uh oh!

lib: add aborted() utility function#46494

lib: add aborted() utility function #46494

Uh oh!

Conversation

debadree25 commented Feb 4, 2023

Uh oh!

debadree25 commented Feb 4, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

debadree25 commented Feb 4, 2023

Uh oh!

Uh oh!

bnoordhuisFeb 4, 2023

Choose a reason for hiding this comment

Uh oh!

debadree25Feb 4, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bnoordhuisFeb 4, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benjamingr left a comment

Choose a reason for hiding this comment

Uh oh!

benjamingr commented Feb 4, 2023

Uh oh!

debadree25 commented Feb 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

debadree25Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

debadree25Feb 6, 2023

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Feb 7, 2023

Uh oh!

nodejs-github-bot commented Feb 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

debadree25 commented Feb 4, 2023•
edited
Loading

debadree25Feb 4, 2023•
edited
Loading