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

+  const ac = new AbortController();
+  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

+ */
+async function aborted(signal, resource) {
+  if (signal === undefined) {
+    throw new ERR_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 resolved Hide resolved

benjamingr reviewed Feb 4, 2023

View reviewed changes

lib/internal/abort_controller.js Show resolved Hide 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 resolved Hide resolved

jasnell reviewed Feb 4, 2023

View reviewed changes

doc/api/util.md Outdated Show resolved Hide resolved

jasnell reviewed Feb 4, 2023

View reviewed changes

doc/api/util.md Outdated Show resolved Hide resolved

jasnell reviewed Feb 4, 2023

View reviewed changes

doc/api/util.md Outdated Show resolved Hide resolved

fixup! doc

a2f2c56

debadree25 commented Feb 4, 2023

View reviewed changes

lib/internal/abort_controller.js Show resolved Hide resolved

aduh95 reviewed Feb 4, 2023

View reviewed changes

doc/api/util.md Outdated Show resolved Hide resolved

doc/api/util.md Outdated Show resolved Hide resolved

test/parallel/test-aborted-util.js Outdated Show resolved Hide resolved

test/parallel/test-aborted-util.js Outdated Show resolved Hide resolved

aduh95 reviewed Feb 4, 2023

View reviewed changes

test/parallel/test-aborted-util.js Show resolved Hide resolved

debadree25 and 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-actions bot 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-actions bot 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:main Feb 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: add aborted() utility function #46494

lib: add aborted() utility function #46494

debadree25 commented Feb 4, 2023

debadree25 commented Feb 4, 2023 •

edited

Loading

aduh95 left a comment

debadree25 commented Feb 4, 2023

bnoordhuis Feb 4, 2023

debadree25 Feb 4, 2023 •

edited

Loading

bnoordhuis Feb 4, 2023

benjamingr left a comment

benjamingr commented Feb 4, 2023

debadree25 commented Feb 4, 2023

debadree25 Feb 6, 2023

debadree25 Feb 6, 2023

nodejs-github-bot commented Feb 7, 2023

nodejs-github-bot commented Feb 7, 2023

lib: add aborted() utility function #46494

lib: add aborted() utility function #46494

Conversation

debadree25 commented Feb 4, 2023

debadree25 commented Feb 4, 2023 • edited Loading

aduh95 left a comment

Choose a reason for hiding this comment

debadree25 commented Feb 4, 2023

bnoordhuis Feb 4, 2023

Choose a reason for hiding this comment

debadree25 Feb 4, 2023 • edited Loading

Choose a reason for hiding this comment

bnoordhuis Feb 4, 2023

Choose a reason for hiding this comment

benjamingr left a comment

Choose a reason for hiding this comment

benjamingr commented Feb 4, 2023

debadree25 commented Feb 4, 2023

debadree25 Feb 6, 2023

Choose a reason for hiding this comment

debadree25 Feb 6, 2023

Choose a reason for hiding this comment

nodejs-github-bot commented Feb 7, 2023

nodejs-github-bot commented Feb 7, 2023

debadree25 commented Feb 4, 2023 •

edited

Loading

debadree25 Feb 4, 2023 •

edited

Loading