lib: initial experimental AbortController implementation #33527

jasnell · 2020-05-23T03:08:10Z

AbortController impl based very closely on:
https://github.com/mysticatea/abort-controller

Marked experimental.
Global (writable, configurable)
Not currently used by any of the existing promise apis.
AbortSignal extends from EventEmitter instead of EventTarget.

const ac = new AbortController();

ac.onabort = () => {};

// Or ...

ac.on('abort', () => {});

ac.abort();

console.log(ac.aborted);

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

benjamingr · 2020-05-23T13:58:21Z

That's awesome, I'm going to check this out and play with it asap.

Also post it on the summit issue.

benjamingr · 2020-05-23T13:59:14Z

lib/internal/abort_controller.js

+'use strict';
+
+// Modeled very closely on the AbortController implementation
+// in https://github.com/mysticatea/abort-controller (MIT license)


cc @mysticatea <3

lib/internal/abort_controller.js

.eslintrc.js

benjamingr · 2020-05-23T14:24:29Z

Other than test and review this is there anything I can do to help?

jasnell · 2020-05-23T14:36:51Z

Other than test and review this is there anything I can do to help?

One of the next steps is to identify a list of the existing Promise APIs in core that could by updated to make use of this. Not all of them will make sense because not all of them are abortable (e.g. libuv only allows us to cancel file system requests if they have not already been started, for instance). So while we may be able to use AbortController for fs.promises.readFile() (which performs a sequence of discreet read operations) it won't for fs.promises.read() (which performs a single discreet read operation). Once we have a list of APIs for which use of AbortController makes sense, then we need to figure out the best way to incorporate it. Each will have their own set of considerations to address.

Additionally, we should make it possible for custom util.promisify implementations to support AbortController. For instance, I'm playing around with the promisification of setTimeout such that it allows:

const sleep = promisify(setTimeout);
const ac = new AbortController();

sleep(10000, ac);

ac.abort();  // Calls clearTimeout on abort

benjamingr · 2020-05-23T16:43:04Z

@jasnell I've split those tasks off into an issue here: #33528 - please feel free to edit that issue as you see fit.

I will start working on these one by one (on weekends mostly) and add tasks as I find them. Feel free to edit it.

I want to be clear that I want to be helpful here so since you took the initiative if at any point any of the things I'm doing are frustrating just tell me and I'll stop and do whatever is helpful to the effort instead. These sorts of ongoing efforts tend to become frustrating in Node.js at times, I want to be sure that I'm not doing any toe-stepping if I can help it.

jasnell · 2020-05-25T16:08:09Z

Initial implementation of EventTarget in #33556. Assuming that one goes through, this PR will be modified such that AbortController.prototype.signal is an instance of EventTarget rather than EventEmitter.

test/parallel/test-abortcontroller.js

lib/internal/abort_controller.js

jasnell · 2020-05-26T22:47:13Z

Updated based on the #33556

jasnell · 2020-05-26T22:54:18Z

@benjamingr ... quick note... this is currently extending from NodeEventTarget ... in this, however, the remove listener functions are not used and the only ones we definitely need here are the on/once/addListener in order to achieve the use cases we need for AbortSignal

doc/api/globals.md

lib/internal/abort_controller.js

doc/api/globals.md

See documentation changes for details Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #33556 Refs: #33527 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com>

jasnell · 2020-05-28T14:33:55Z

Marking this ready for review. This does create a new global so technically may be semver-major but I have tested it with existing ecosystem polyfills and haven't seen any breakage at all. Unless there are objections from the @nodejs/tsc, I'd like to handle this as a semver-minor.

It is still experimental and an experimental warning will be emitted the first time an AbortController is created. It is not behind a flag.

Notable changes: Deprecations and Removals: - **build**: remove --build-v8-with-gn configure option (Yang Guo) (#27576) - **build**: drop support for VS2017 (Michaël Zasso) (#33694) - **doc**: move DEP0018 to End-of-Life (Rich Trott) (#35316) - **fs**: deprecation warning on recursive rmdir (Ian Sutherland) (#35562) - **lib**: add EventTarget-related browser globals (Anna Henningsen) (#35496) - **net**: remove long deprecated server.connections property (James M Snell) (#33647) - **repl**: remove deprecated repl.memory function (Ruben Bridgewater) (#33286) - **repl**: remove deprecated repl.turnOffEditorMode() function (Ruben Bridgewater) (#33286) - **repl**: remove deprecated repl.parseREPLKeyword() function (Ruben Bridgewater) (#33286) - **repl**: remove deprecated bufferedCommand property (Ruben Bridgewater) (#33286) - **repl**: remove deprecated .rli (Ruben Bridgewater) (#33286) - **src**: remove deprecated node debug command (James M Snell) (#33648) - **timers**: introduce timers/promises (James M Snell) (#33950) - **util**: change default value of `maxStringLength` to 10000 (unknown) (#32744) - **wasi**: drop --experimental-wasm-bigint requirement (Colin Ihrig) (#35415) npm 7 (#35631): Node.js 15 comes with a new major release of npm, npm 7. npm 7 comes with many new features - including npm workspaces and a new package-lock.json format. npm 7 also includes yarn.lock file support. One of the big changes in npm 7 is that peer dependencies are now installed by default. Throw On Unhandled Rejections (#33021): As of Node.js 15, the default mode for `unhandledRejection` is changed to `throw` (from `warn`). In `throw` mode, if an `unhandledRejection` hook is not set, the `unhandledRejection` is raised as an uncaught exception. Users that have an `unhandledRejection` hook should see no change in behavior, and it’s still possible to switch modes using the `--unhandled-rejections=mode` process flag. QUIC (#32379): Node.js 15 comes with experimental support QUIC, which can be enabled by compiling Node.js with the `--experimental-quic` configuration flag. The Node.js QUIC implementation is exposed by the core `net` module. V8 8.6 (#35415): The V8 JavaScript engine has been updated to V8 8.6 (V8 8.4 is the latest available in Node.js 14). Along with performance tweaks and improvements the V8 update also brings the following language features: * `Promise.any()` (from V8 8.5) * `AggregateError` (from V8 8.5) * `String.prototype.replaceAll()` (from V8 8.5) * Logical assignment operators `&&=`, `||=`, and `??=` (from V8 8.5) Other Notable Changes: - **assert**: add `assert/strict` alias module (ExE Boss) (#34001) - **dns**: add dns/promises alias (shisama) (#32953) - **fs**: reimplement read and write streams using stream.construct (Robert Nagy) (#29656) - **http2**: allow Host in HTTP/2 requests (Alba Mendez) (#34664) - **lib**: add EventTarget-related browser globals (Anna Henningsen) (#35496) - **lib**: unflag AbortController (James M Snell) (#33527) - **lib**: initial experimental AbortController implementation (James M Snell) (#33527) - **net**: autoDestroy Socket (Robert Nagy) (#31806) - **src**: disallow JS execution inside FreeEnvironment (Anna Henningsen) (#33874) - **stream**: construct (Robert Nagy) (#29656) - **worker**: make MessageEvent class more Web-compatible (Anna Henningsen) (#35496) Semver-Major Commits: - **assert**: add `assert/strict` alias module (ExE Boss) (#34001) - **build**: reset embedder string to "-node.0" (Michaël Zasso) (#35415) - **build**: remove --build-v8-with-gn configure option (Yang Guo) (#27576) - **build**: drop support for VS2017 (Michaël Zasso) (#33694) - **crypto**: refactoring internals, add WebCrypto (James M Snell) (#35093) - **crypto**: move node\_crypto files to src/crypto (James M Snell) (#35093) - **deps**: V8: cherry-pick d76abfed3512 (Michaël Zasso) (#35415) - **deps**: V8: cherry-pick 717543bbf0ef (Michaël Zasso) (#35415) - **deps**: V8: cherry-pick 6be2f6e26e8d (Michaël Zasso) (#35415) - **deps**: fix V8 build issue with inline methods (Jiawen Geng) (#35415) - **deps**: fix platform-embedded-file-writer-win for ARM64 (Michaël Zasso) (#35415) - **deps**: update V8 postmortem metadata script (Colin Ihrig) (#35415) - **deps**: update V8 to 8.6.395 (Michaël Zasso) (#35415) - **deps**: upgrade npm to 7.0.0 (Myles Borins) (#35631) - **deps**: update npm to 7.0.0-rc.3 (Myles Borins) (#35474) - **deps**: V8: cherry-pick 0d6debcc5f08 (Gus Caplan) (#33600) - **dns**: add dns/promises alias (shisama) (#32953) - **doc**: move DEP0018 to End-of-Life (Rich Trott) (#35316) - **doc**: update support macos version for 15.x (Ash Cripps) (#35022) - **fs**: deprecation warning on recursive rmdir (Ian Sutherland) (#35562) - **fs**: reimplement read and write streams using stream.construct (Robert Nagy) (#29656) - **http**: fixed socket.setEncoding fatal error (iskore) (#33405) - **http**: emit 'error' on aborted server request (Robert Nagy) (#33172) - **http**: cleanup end argument handling (Robert Nagy) (#31818) - **http2**: allow Host in HTTP/2 requests (Alba Mendez) (#34664) - **http2**: add `invalidheaders` test (Pranshu Srivastava) (#33161) - **http2**: refactor state code validation for the http2Stream class (rickyes) (#33535) - **http2**: header field valid checks (Pranshu Srivastava) (#33193) - **lib**: add EventTarget-related browser globals (Anna Henningsen) (#35496) - **lib**: remove ERR\_INVALID\_OPT\_VALUE and ERR\_INVALID\_OPT\_VALUE\_ENCODING (Denys Otrishko) (#34682) - **lib**: handle one of args case in ERR\_MISSING\_ARGS (Denys Otrishko) (#34022) - **lib**: remove NodeError from the prototype of errors with code (Michaël Zasso) (#33857) - **lib**: unflag AbortController (James M Snell) (#33527) - **lib**: initial experimental AbortController implementation (James M Snell) (#33527) - **net**: check args in net.connect() and socket.connect() calls (Denys Otrishko) (#34022) - **net**: remove long deprecated server.connections property (James M Snell) (#33647) - **net**: autoDestroy Socket (Robert Nagy) (#31806) - **process**: update v8 fast api calls usage (Maya Lekova) (#35415) - **process**: change default --unhandled-rejections=throw (Dan Fabulich) (#33021) - **process**: use v8 fast api calls for hrtime (Gus Caplan) (#33600) - **process**: delay throwing an error using `throwDeprecation` (Ruben Bridgewater) (#32312) - **repl**: remove deprecated repl.memory function (Ruben Bridgewater) (#33286) - **repl**: remove deprecated repl.turnOffEditorMode() function (Ruben Bridgewater) (#33286) - **repl**: remove deprecated repl.parseREPLKeyword() function (Ruben Bridgewater) (#33286) - **repl**: remove deprecated bufferedCommand property (Ruben Bridgewater) (#33286) - **repl**: remove deprecated .rli (Ruben Bridgewater) (#33286) - **src**: implement NodePlatform::PostJob (Clemens Backes) (#35415) - **src**: update NODE\_MODULE\_VERSION to 88 (Michaël Zasso) (#35415) - **src**: error reporting on CPUUsage (Yash Ladha) (#34762) - **src**: use node:moduleName as builtin module filename (Michaël Zasso) (#35498) - **src**: enable wasm trap handler on windows (Gus Caplan) (#35033) - **src**: update NODE\_MODULE\_VERSION to 86 (Michaël Zasso) (#33579) - **src**: disallow JS execution inside FreeEnvironment (Anna Henningsen) (#33874) - **src**: remove \_third\_party\_main support (Anna Henningsen) (#33971) - **src**: remove deprecated node debug command (James M Snell) (#33648) - **src**: remove unused CancelPendingDelayedTasks (Anna Henningsen) (#32859) - **stream**: try to wait for flush to complete before 'finish' (Robert Nagy) (#34314) - **stream**: cleanup and fix Readable.wrap (Robert Nagy) (#34204) - **stream**: add promises version to utility functions (rickyes) (#33991) - **stream**: fix writable.end callback behavior (Robert Nagy) (#34101) - **stream**: construct (Robert Nagy) (#29656) - **stream**: write should throw on unknown encoding (Robert Nagy) (#33075) - **stream**: fix \_final and 'prefinish' timing (Robert Nagy) (#32780) - **stream**: simplify Transform stream implementation (Robert Nagy) (#32763) - **stream**: use callback to properly propagate error (Robert Nagy) (#29179) - **test**: update tests after increasing typed array size to 4GB (Kim-Anh Tran) (#35415) - **test**: fix tests for npm 7.0.0 (Myles Borins) (#35631) - **test**: fix test suite to work with npm 7 (Myles Borins) (#35474) - **test**: update WPT harness and tests (Michaël Zasso) (#33770) - **timers**: introduce timers/promises (James M Snell) (#33950) - **tools**: disable x86 safe exception handlers in V8 (Michaël Zasso) (#35415) - **tools**: update V8 gypfiles for 8.6 (Ujjwal Sharma) (#35415) - **tools**: update V8 gypfiles for 8.5 (Ujjwal Sharma) (#35415) - **url**: file URL path normalization (Daijiro Wachi) (#35477) - **url**: verify domain is not empty after "ToASCII" (Michaël Zasso) (#33770) - **url**: remove U+0000 case in the fragment state (Michaël Zasso) (#33770) - **url**: remove gopher from special schemes (Michaël Zasso) (#33325) - **url**: forbid lt and gt in url host code point (Yash Ladha) (#33328) - **util**: change default value of `maxStringLength` to 10000 (unknown) (#32744) - **wasi**: drop --experimental-wasm-bigint requirement (Colin Ihrig) (#35415) - **win, child_process**: sanitize env variables (Bartosz Sosnowski) (#35210) - **worker**: make MessageEvent class more Web-compatible (Anna Henningsen) (#35496) - **worker**: set trackUnmanagedFds to true by default (Anna Henningsen) (#34394) - **worker**: rename error code to be more accurate (Anna Henningsen) (#33872) PR-URL: #35014

zcbenz · 2020-10-29T01:14:53Z

@nodejs/embedders This is going to conflict with DOM's AbortController.

mhdawson · 2020-10-29T13:38:03Z

@zcbenz can you expand on your comment?

jasnell · 2020-10-29T14:00:54Z

@mhdawson ... I can explain a bit: Environment's like Electron have both Node.js and Chromium available. Chromium brings along its own implementation of certain globals (like URL, TextEncoder, and AbortController. In some situations, it's possible for objects from one environment to interfere with the others. @codebytere recently opened an issue (for instance) about conflicts between the two environment's and the URL global and opened a PR that changes how we detect if a URL really is a URL (instanceof does not work reliably in this case). We may run in to similar issues here.

mhdawson · 2020-10-29T21:57:51Z

@jasnell thanks that helps me understand better. Next question is it something we need to do something about on the Node.js side or was the comment an FYI for awareness?

jasnell · 2020-10-29T22:08:46Z

I assume more FYI at the moment. There may be something we can do down the road to make it easier

addaleax · 2020-10-29T22:57:51Z

Fwiw, there is the option to use ./configure --no-browser-globals that should affect AbortController as well.

zcbenz · 2020-10-30T09:58:50Z

@jasnell Thanks for explaining it.

On the embedder's side, there are 2 challenges with Node's AbortController global variable:

DOM's APIs can not take Node's AbortSignal as argument, and I assume Node's APIs would not be able to take DOM's as well. So no matter which AbortController implementation we choose to use as the global variable, there will be a part of APIs refusing to work with it.
The TypeScript definitions of Node's AbortSignal must be exactly the same with DOM's one, otherwise issues like node-fetch/node-fetch#741 will happen.

But if Node's promise APIs are able to take arbitrary implementation of AbortSignal, then we would be able to replace Node's AbortController with DOM's one completely, and all of the above issues would be solved.

benjamingr · 2020-10-30T10:57:43Z

@zcbenz

I assume Node's APIs would not be able to take DOM's as well.

Why not? We do not perform the same sort of branding checks the DOM does. We just check that it has a property called "aborted" on it at the moment.

I cannot speak about the other way around though - I would assume node's AbortSignals won't work in DOM APIs.

The TypeScript definitions of Node's AbortSignal must be exactly the same with DOM's one,

I am not sure who maintains the node type definitions - but if the API is different please open an issue and we will fix it. A goal is that developers should have an easier time cancelling things in a universal way and API deltas run counter to that.

But if Node's promise APIs are able to take arbitrary implementation of AbortSignal, then we would be able to replace Node's AbortController with DOM's one completely, and all of the above issues would be solved.

This sounds like a reasonable approach to me.

AbortController impl based very closely on: https://github.com/mysticatea/abort-controller Marked experimental. Not currently used by any of the existing promise apis. Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: nodejs#33527 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

AbortController impl based very closely on: https://github.com/mysticatea/abort-controller Marked experimental. Not currently used by any of the existing promise apis. Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #33527 Backport-PR-URL: #38386 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Refs: - nodejs/node#34186 - nodejs/node#34895 - nodejs/node#33527 - nodejs/node#33950

nodejs-github-bot added the lib / src label May 23, 2020

jasnell force-pushed the abort-controller branch 2 times, most recently from 7544a2d to 29b247c Compare May 23, 2020

benjamingr mentioned this pull request May 23, 2020

Session Proposal: Support Cancellation (AbortController) in Node.js core openjs-foundation/summit#273

Closed

11 tasks

benjamingr reviewed May 23, 2020

View changes

lib/internal/abort_controller.js Outdated Show resolved Hide resolved

benjamingr reviewed May 23, 2020

View changes

.eslintrc.js Show resolved Hide resolved

benjamingr mentioned this pull request May 23, 2020

🚀 AbortController in Node task list #33528

Closed

5 tasks

jasnell mentioned this pull request May 25, 2020

events: initial implementation of experimental EventTarget #33556

Closed

4 tasks

ptomato reviewed May 25, 2020

View changes

test/parallel/test-abortcontroller.js Outdated Show resolved Hide resolved

lib/internal/abort_controller.js Outdated Show resolved Hide resolved

arthurschreiber mentioned this pull request May 26, 2020

Rework connect timeout handling tediousjs/tedious#1100

Open

jasnell force-pushed the abort-controller branch 2 times, most recently from 02953f1 to 7290192 Compare May 26, 2020

jasnell force-pushed the abort-controller branch from 95b6541 to 98d96d5 Compare May 26, 2020

felixfbecker reviewed May 27, 2020

View changes

doc/api/globals.md Show resolved Hide resolved

lib/internal/abort_controller.js Outdated Show resolved Hide resolved

targos reviewed May 27, 2020

View changes

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

felixfbecker reviewed May 28, 2020

View changes

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

jasnell force-pushed the abort-controller branch 2 times, most recently from a7b3dd8 to 890a585 Compare May 28, 2020

jasnell marked this pull request as ready for review May 28, 2020

jasnell added the semver-minor label May 28, 2020

zcbenz mentioned this pull request Oct 29, 2020

feat: add signal option to dialog.showMessageBox electron/electron#26102

Merged

6 tasks

targos added the backport-open-v14.x label Apr 24, 2021

targos mentioned this pull request Apr 24, 2021

[v14.x] Backport AbortController and friends #38386

Closed

targos added backported-to-v14.x and removed backport-open-v14.x labels Apr 30, 2021

danielleadams mentioned this pull request May 3, 2021

v14.17.0 proposal #38507

Merged

codebytere added a commit to electron/electron that referenced this issue May 14, 2021

chore: update filenames list

85480f8

Refs: - nodejs/node#34186 - nodejs/node#34895 - nodejs/node#33527 - nodejs/node#33950

codebytere added a commit to electron/electron that referenced this issue May 18, 2021

chore: update filenames list

aa420bd

Refs: - nodejs/node#34186 - nodejs/node#34895 - nodejs/node#33527 - nodejs/node#33950

snyk-bot mentioned this pull request Jan 25, 2022

[Snyk] Security upgrade gatsby from 2.13.73 to 4.0.0 kissofire/node#53

Open

snyk-bot mentioned this pull request Jan 25, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 ron-a20/node#52

Open

snyk-bot mentioned this pull request Jan 25, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 Rednickle/node-1#39

Open

snyk-bot mentioned this pull request Jan 25, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 137717unity/node#44

Open

snyk-bot mentioned this pull request Jan 25, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 aliceUnhinged613/node#42

Open

snyk-bot mentioned this pull request Jan 25, 2022

[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 Dmarch28/node#42

Open

snyk-bot mentioned this pull request Jan 26, 2022

[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 baby636/node#36

Open

snyk-bot mentioned this pull request Jan 26, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 molllyn1/node#34

Open

snyk-bot mentioned this pull request Jan 26, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 wedataintelligence/node#20

Open

nodejs / node Public

lib: initial experimental AbortController implementation #33527

lib: initial experimental AbortController implementation #33527

jasnell commented May 23, 2020 •

edited

benjamingr commented May 23, 2020

benjamingr May 23, 2020

benjamingr commented May 23, 2020

jasnell commented May 23, 2020

benjamingr commented May 23, 2020

jasnell commented May 25, 2020

jasnell commented May 26, 2020

jasnell commented May 26, 2020

jasnell commented May 28, 2020

zcbenz commented Oct 29, 2020

mhdawson commented Oct 29, 2020

jasnell commented Oct 29, 2020

mhdawson commented Oct 29, 2020

jasnell commented Oct 29, 2020

addaleax commented Oct 29, 2020

zcbenz commented Oct 30, 2020

benjamingr commented Oct 30, 2020 •

edited

Mar	APR	May
	23
2021	2022	2023

nodejs / node Public

lib: initial experimental AbortController implementation #33527

lib: initial experimental AbortController implementation #33527

Conversation

jasnell commented May 23, 2020 • edited

Checklist

benjamingr commented May 23, 2020

benjamingr May 23, 2020

Choose a reason for hiding this comment

benjamingr commented May 23, 2020

jasnell commented May 23, 2020

benjamingr commented May 23, 2020

jasnell commented May 25, 2020

jasnell commented May 26, 2020

jasnell commented May 26, 2020

jasnell commented May 28, 2020

zcbenz commented Oct 29, 2020

mhdawson commented Oct 29, 2020

jasnell commented Oct 29, 2020

mhdawson commented Oct 29, 2020

jasnell commented Oct 29, 2020

addaleax commented Oct 29, 2020

zcbenz commented Oct 30, 2020

benjamingr commented Oct 30, 2020 • edited

jasnell commented May 23, 2020 •

edited

benjamingr commented Oct 30, 2020 •

edited