readline: fix .question still called after closed #42464

meixg · 2022-03-25T09:26:18Z

aduh95

Can you add a test for node:readline/promises as well please?

aduh95

I don't like the inconsistency: for node:readline, the callback is never called, but for node:readline/promises the promise gets resolved with undefined?? I think we should implement the same behavior for both (so either the callback gets called with no arguments, either we let the promise in a never settling state). We should also need to document the new behavior.

Also would it make more sense to make the change in the super class instead of two changes? (not sure, I haven't looked at the implementation closely)

node/lib/internal/readline/interface.js

Line 400 in a066857

question(query, cb) {

test/parallel/test-readline-interface.js

doc/api/readline.md

test/parallel/test-readline-promises-interface.js

aduh95 · 2022-03-25T17:35:10Z

I wonder if there are other Node.js APIs that never call the callback (or return a never-settling promise), that seems a bit counter-intuitive tbh. @nodejs/tsc thoughts?

//cc @nodejs/readline

RaisinTen · 2022-03-26T13:18:09Z

I think we should throw an exception if the user attempts to use the readline object after readline.close() gets called. We already do that in a number of places. You'll be able to find some examples if you search for _CLOSED in https://nodejs.org/api/errors.html.

meixg · 2022-03-28T05:38:21Z

changed to throw an error.

aduh95 · 2022-03-28T08:01:19Z

test/parallel/test-readline-interface.js

+        .then(common.mustNotCall())
+        .catch(common.expectsError({


Suggested change

.then(common.mustNotCall())

.catch(common.expectsError({

.then(common.mustNotCall(), common.expectsError({

or

Suggested change

.then(common.mustNotCall())

.catch(common.expectsError({

.catch(common.expectsError({

mcollina

lgtm

RaisinTen

LGTM

RaisinTen · 2022-03-28T15:33:20Z

doc/api/errors.md

@@ -2358,6 +2358,14 @@ Accessing `Object.prototype.__proto__` has been forbidden using
 [`Object.setPrototypeOf`][] should be used to get and set the prototype of an
 object.

+<a id="ERR_READLINE_CLOSED"></a>
+
+### `ERR_READLINE_CLOSED`


nit: I think it would be better if we rename this to something more generic like ERR_USE_AFTER_CLOSE and replace all the other error codes with the same meaning with this one in a separate PR. Refs: #41660 (comment).

RaisinTen · 2022-04-05T14:55:41Z

@meixg this has a git conflict btw, could you please rebase and resolve it?

resolve: #42450

handle promises version

let the promise in a never settling state and make the change in the super class

add promisified question test case

change doc

throw an error

change according to review

meixg · 2022-04-06T03:12:54Z

rebase and changed according to CR

change to a more generically error code name

test/parallel/test-readline-promises-interface.js

aduh95 · 2022-04-06T14:04:39Z

This is semver-major PRs that contain breaking changes and should be released in the next major version. as it's throwing an new error.

change according to cr

nodejs-github-bot · 2022-04-08T03:14:49Z

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

nodejs-github-bot · 2022-04-08T04:54:04Z

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

nodejs-github-bot · 2022-04-08T07:26:08Z

Commit Queue failed

- Loading data for nodejs/node/pull/42464
✔  Done loading data for nodejs/node/pull/42464
----------------------------------- PR info ------------------------------------
Title      readline: fix .question still called after closed (#42464)
Author     Xuguang Mei  (@meixg)
Branch     meixg:readline-question-meixg -> nodejs:master
Labels     readline, semver-major, needs-ci, commit-queue-squash
Commits    9
 - readline: fix question still called after closed
 - readline: fix question still called after closed
 - readline: fix question still called after closed
 - readline: fix question still called after closed
 - readline: fix question still called after closed
 - readline: fix question still called after closed
 - readline: fix question still called after closed
 - readline: fix question still called after closed
 - readline: fix question still called after closed
Committers 1
 - meixg 
PR-URL: https://github.com/nodejs/node/pull/42464
Reviewed-By: Antoine du Hamel 
Reviewed-By: Matteo Collina 
Reviewed-By: Darshan Sen 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42464
Reviewed-By: Antoine du Hamel 
Reviewed-By: Matteo Collina 
Reviewed-By: Darshan Sen 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - readline: fix question still called after closed
   ℹ  This PR was created on Fri, 25 Mar 2022 09:26:18 GMT
   ✔  Approvals: 3
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/42464#pullrequestreview-933552817
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/42464#pullrequestreview-922903993
   ✔  - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/42464#pullrequestreview-923344674
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-04-08T04:54:04Z: https://ci.nodejs.org/job/node-test-pull-request/43387/
- Querying data for job/node-test-pull-request/43387/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/2113718557

nodejs-github-bot · 2022-04-08T10:17:05Z

Landed in d36b60e

Notable Changes: Deprecations and Removals: - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (#42607) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (#41896) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (#40773) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (#41479) fetch (experimental): An experimental fetch API is available on the global scope by default. The implementation is based upon https://undici.nodejs.org/#/, an HTTP/1.1 client written for Node.js by contributors to the project. Through this addition, the following globals are made available: `fetch` , `FormData`, `Headers`, `Request`, `Response`. Disable this API with the `--no-experimental-fetch` command-line flag. Contributed by Michaël Zasso in #41811. HTTP Timeouts: `server.headersTimeout`, which limits the amount of time the parser will wait to receive the complete HTTP headers, is now set to `60000` (60 seconds) by default. `server.requestTimeout`, which sets the timeout value in milliseconds for receiving the entire request from the client, is now set to `300000` (5 minutes) by default. If these timeouts expire, the server responds with status 408 without forwarding the request to the request listener and then closes the connection. Both timeouts must be set to a non-zero value to protect against potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. Contributed by Paolo Insogna in #41263. Test Runner module (experimental): The `node:test` module facilitates the creation of JavaScript tests that report results in TAP format. This module is only available under the `node:` scheme. Contributed by Colin Ihrig in #42325. Toolchain and Compiler Upgrades: - Prebuilt binaries for Linux are now built on Red Hat Enterprise Linux (RHEL) 8 and are compatible with Linux distributions based on glibc 2.28 or later, for example, Debian 10, RHEL 8, Ubuntu 20.04. - Prebuilt binaries for macOS now require macOS 10.15 or later. - For AIX the minimum supported architecture has been raised from Power 7 to Power 8. Prebuilt binaries for 32-bit Windows will initially not be available due to issues building the V8 dependency in Node.js. We hope to restore 32-bit Windows binaries for Node.js 18 with a future V8 update. Node.js does not support running on operating systems that are no longer supported by their vendor. For operating systems where their vendor has planned to end support earlier than April 2025, such as Windows 8.1 (January 2023) and Windows Server 2012 R2 (October 2023), support for Node.js 18 will end at the earlier date. Full details about the supported toolchains and compilers are documented in the Node.js `BUILDING.md` file. Contributed by Richard Lau in #42292, #42604 and #42659 , and Michaël Zasso in #42105 and #42666. V8 10.1: The V8 engine is updated to version 10.1, which is part of Chromium 101. Compared to the version included in Node.js 17.9.0, the following new features are included: - The `findLast` and `findLastIndex` array methods. - Improvements to the `Intl.Locale` API. - The `Intl.supportedValuesOf` function. - Improved performance of class fields and private class methods (the initialization of them is now as fast as ordinary property stores). The data format returned by the serialization API (`v8.serialize(value)`) has changed, and cannot be deserialized by earlier versions of Node.js. On the other hand, it is still possible to deserialize the previous format, as the API is backwards-compatible. Contributed by Michaël Zasso in #42657. Web Streams API (experimental): Node.js now exposes the experimental implementation of the Web Streams API on the global scope. This means the following APIs are now globally available: - `ReadableStream`, `ReadableStreamDefaultReader`, `ReadableStreamBYOBReader`, `ReadableStreamBYOBRequest`, `ReadableByteStreamController`, `ReadableStreamDefaultController`, `TransformStream`, `TransformStreamDefaultController`, `WritableStream`, `WritableStreamDefaultWriter`, `WritableStreamDefaultController`, `ByteLengthQueuingStrategy`, `CountQueuingStrategy`, `TextEncoderStream`, `TextDecoderStream`, `CompressionStream`, `DecompressionStream`. Contributed James Snell in #39062, and Antoine du Hamel in #42225. Other Notable Changes: - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (#41270) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (#41305) - doc: add RafaelGSS to collaborators (RafaelGSS) (#42718) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (#42163) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (#41271) Semver-Major Commits: - (SEMVER-MAJOR) assert,util: compare RegExp.lastIndex while using deep equal checks (Ruben Bridgewater) (#41020) - (SEMVER-MAJOR) buffer: refactor `byteLength` to remove outdated optimizations (Rongjian Zhang) (#38545) - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (#41270) - (SEMVER-MAJOR) buffer: graduate Blob from experimental (James M Snell) (#41270) - (SEMVER-MAJOR) build: make x86 Windows support temporarily experimental (Michaël Zasso) (#42666) - (SEMVER-MAJOR) build: bump macOS deployment target to 10.15 (Richard Lau) (#42292) - (SEMVER-MAJOR) build: downgrade Windows 8.1 and server 2012 R2 to experimental (Michaël Zasso) (#42105) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (#41305) - (SEMVER-MAJOR) cluster: make `kill` to be just `process.kill` (Bar Admoni) (#34312) - (SEMVER-MAJOR) crypto: cleanup validation (Mohammed Keyvanzadeh) (#39841) - (SEMVER-MAJOR) crypto: prettify othername in PrintGeneralName (Tobias Nießen) (#42123) - (SEMVER-MAJOR) crypto: fix X509Certificate toLegacyObject (Tobias Nießen) (#42124) - (SEMVER-MAJOR) crypto: use RFC2253 format in PrintGeneralName (Tobias Nießen) (#42002) - (SEMVER-MAJOR) crypto: change default check(Host|Email) behavior (Tobias Nießen) (#41600) - (SEMVER-MAJOR) deps: V8: cherry-pick semver-major commits from 10.2 (Michaël Zasso) (#42657) - (SEMVER-MAJOR) deps: update V8 to 10.1.124.6 (Michaël Zasso) (#42657) - (SEMVER-MAJOR) deps: update V8 to 9.8.177.9 (Michaël Zasso) (#41610) - (SEMVER-MAJOR) deps: update V8 to 9.7.106.18 (Michaël Zasso) (#40907) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) doc: update minimum glibc requirements for Linux (Richard Lau) (#42659) - (SEMVER-MAJOR) doc: update AIX minimum supported arch (Richard Lau) (#42604) - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (#42607) - (SEMVER-MAJOR) http: refactor headersTimeout and requestTimeout logic (Paolo Insogna) (#41263) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (#42163) - (SEMVER-MAJOR) lib: enable fetch by default (Michaël Zasso) (#41811) - (SEMVER-MAJOR) lib: replace validator and error (Mohammed Keyvanzadeh) (#41678) - (SEMVER-MAJOR) module,repl: support 'node:'-only core modules (Colin Ihrig) (#42325) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (#41431) - (SEMVER-MAJOR) process: disallow some uses of Object.defineProperty() on process.env (Himself65) (#28006) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (#41896) - (SEMVER-MAJOR) readline: fix question still called after closed (Xuguang Mei) (#42464) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (#40773) - (SEMVER-MAJOR) stream: expose web streams globals, remove runtime experimental warning (Antoine du Hamel) (#42225) - (SEMVER-MAJOR) stream: need to cleanup event listeners if last stream is readable (Xuguang Mei) (#41954) - (SEMVER-MAJOR) stream: revert revert `map` spec compliance (Benjamin Gruenbaum) (#41933) - (SEMVER-MAJOR) stream: throw invalid arg type from End Of Stream (Jithil P Ponnan) (#41766) - (SEMVER-MAJOR) stream: don't emit finish after destroy (Robert Nagy) (#40852) - (SEMVER-MAJOR) stream: add errored and closed props (Robert Nagy) (#40696) - (SEMVER-MAJOR) test: add initial test module (Colin Ihrig) (#42325) - (SEMVER-MAJOR) timers: refactor internal classes to ES2015 syntax (Rabbit) (#37408) - (SEMVER-MAJOR) tls: represent registeredID numerically always (Tobias Nießen) (#41561) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (#41479) - (SEMVER-MAJOR) url: throw on NULL in IPv6 hostname (Rich Trott) (#42313) - (SEMVER-MAJOR) v8: make v8.writeHeapSnapshot() error codes consistent (Darshan Sen) (#42577) - (SEMVER-MAJOR) v8: make writeHeapSnapshot throw if fopen fails (Antonio Román) (#41373) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (#41271) PR-URL: #42262

resolve: nodejs#42450 PR-URL: nodejs#42464 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Darshan Sen <[email protected]>

Notable Changes: Deprecations and Removals: - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (nodejs#42607) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (nodejs#41896) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (nodejs#40773) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (nodejs#41479) fetch (experimental): An experimental fetch API is available on the global scope by default. The implementation is based upon https://undici.nodejs.org/#/, an HTTP/1.1 client written for Node.js by contributors to the project. Through this addition, the following globals are made available: `fetch` , `FormData`, `Headers`, `Request`, `Response`. Disable this API with the `--no-experimental-fetch` command-line flag. Contributed by Michaël Zasso in nodejs#41811. HTTP Timeouts: `server.headersTimeout`, which limits the amount of time the parser will wait to receive the complete HTTP headers, is now set to `60000` (60 seconds) by default. `server.requestTimeout`, which sets the timeout value in milliseconds for receiving the entire request from the client, is now set to `300000` (5 minutes) by default. If these timeouts expire, the server responds with status 408 without forwarding the request to the request listener and then closes the connection. Both timeouts must be set to a non-zero value to protect against potential Denial-of-Service attacks in case the server is deployed without a reverse proxy in front. Contributed by Paolo Insogna in nodejs#41263. Test Runner module (experimental): The `node:test` module facilitates the creation of JavaScript tests that report results in TAP format. This module is only available under the `node:` scheme. Contributed by Colin Ihrig in nodejs#42325. Toolchain and Compiler Upgrades: - Prebuilt binaries for Linux are now built on Red Hat Enterprise Linux (RHEL) 8 and are compatible with Linux distributions based on glibc 2.28 or later, for example, Debian 10, RHEL 8, Ubuntu 20.04. - Prebuilt binaries for macOS now require macOS 10.15 or later. - For AIX the minimum supported architecture has been raised from Power 7 to Power 8. Prebuilt binaries for 32-bit Windows will initially not be available due to issues building the V8 dependency in Node.js. We hope to restore 32-bit Windows binaries for Node.js 18 with a future V8 update. Node.js does not support running on operating systems that are no longer supported by their vendor. For operating systems where their vendor has planned to end support earlier than April 2025, such as Windows 8.1 (January 2023) and Windows Server 2012 R2 (October 2023), support for Node.js 18 will end at the earlier date. Full details about the supported toolchains and compilers are documented in the Node.js `BUILDING.md` file. Contributed by Richard Lau in nodejs#42292, nodejs#42604 and nodejs#42659 , and Michaël Zasso in nodejs#42105 and nodejs#42666. V8 10.1: The V8 engine is updated to version 10.1, which is part of Chromium 101. Compared to the version included in Node.js 17.9.0, the following new features are included: - The `findLast` and `findLastIndex` array methods. - Improvements to the `Intl.Locale` API. - The `Intl.supportedValuesOf` function. - Improved performance of class fields and private class methods (the initialization of them is now as fast as ordinary property stores). The data format returned by the serialization API (`v8.serialize(value)`) has changed, and cannot be deserialized by earlier versions of Node.js. On the other hand, it is still possible to deserialize the previous format, as the API is backwards-compatible. Contributed by Michaël Zasso in nodejs#42657. Web Streams API (experimental): Node.js now exposes the experimental implementation of the Web Streams API on the global scope. This means the following APIs are now globally available: - `ReadableStream`, `ReadableStreamDefaultReader`, `ReadableStreamBYOBReader`, `ReadableStreamBYOBRequest`, `ReadableByteStreamController`, `ReadableStreamDefaultController`, `TransformStream`, `TransformStreamDefaultController`, `WritableStream`, `WritableStreamDefaultWriter`, `WritableStreamDefaultController`, `ByteLengthQueuingStrategy`, `CountQueuingStrategy`, `TextEncoderStream`, `TextDecoderStream`, `CompressionStream`, `DecompressionStream`. Contributed James Snell in nodejs#39062, and Antoine du Hamel in nodejs#42225. Other Notable Changes: - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (nodejs#41270) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (nodejs#41305) - doc: add RafaelGSS to collaborators (RafaelGSS) (nodejs#42718) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (nodejs#42163) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (nodejs#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (nodejs#41271) Semver-Major Commits: - (SEMVER-MAJOR) assert,util: compare RegExp.lastIndex while using deep equal checks (Ruben Bridgewater) (nodejs#41020) - (SEMVER-MAJOR) buffer: refactor `byteLength` to remove outdated optimizations (Rongjian Zhang) (nodejs#38545) - (SEMVER-MAJOR) buffer: expose Blob as a global (James M Snell) (nodejs#41270) - (SEMVER-MAJOR) buffer: graduate Blob from experimental (James M Snell) (nodejs#41270) - (SEMVER-MAJOR) build: make x86 Windows support temporarily experimental (Michaël Zasso) (nodejs#42666) - (SEMVER-MAJOR) build: bump macOS deployment target to 10.15 (Richard Lau) (nodejs#42292) - (SEMVER-MAJOR) build: downgrade Windows 8.1 and server 2012 R2 to experimental (Michaël Zasso) (nodejs#42105) - (SEMVER-MAJOR) child\_process: improve argument validation (Rich Trott) (nodejs#41305) - (SEMVER-MAJOR) cluster: make `kill` to be just `process.kill` (Bar Admoni) (nodejs#34312) - (SEMVER-MAJOR) crypto: cleanup validation (Mohammed Keyvanzadeh) (nodejs#39841) - (SEMVER-MAJOR) crypto: prettify othername in PrintGeneralName (Tobias Nießen) (nodejs#42123) - (SEMVER-MAJOR) crypto: fix X509Certificate toLegacyObject (Tobias Nießen) (nodejs#42124) - (SEMVER-MAJOR) crypto: use RFC2253 format in PrintGeneralName (Tobias Nießen) (nodejs#42002) - (SEMVER-MAJOR) crypto: change default check(Host|Email) behavior (Tobias Nießen) (nodejs#41600) - (SEMVER-MAJOR) deps: V8: cherry-pick semver-major commits from 10.2 (Michaël Zasso) (nodejs#42657) - (SEMVER-MAJOR) deps: update V8 to 10.1.124.6 (Michaël Zasso) (nodejs#42657) - (SEMVER-MAJOR) deps: update V8 to 9.8.177.9 (Michaël Zasso) (nodejs#41610) - (SEMVER-MAJOR) deps: update V8 to 9.7.106.18 (Michaël Zasso) (nodejs#40907) - (SEMVER-MAJOR) dns: remove `dns.lookup` and `dnsPromises.lookup` options type coercion (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) doc: update minimum glibc requirements for Linux (Richard Lau) (nodejs#42659) - (SEMVER-MAJOR) doc: update AIX minimum supported arch (Richard Lau) (nodejs#42604) - (SEMVER-MAJOR) fs: runtime deprecate string coercion in `fs.write`, `fs.writeFileSync` (Livia Medeiros) (nodejs#42607) - (SEMVER-MAJOR) http: refactor headersTimeout and requestTimeout logic (Paolo Insogna) (nodejs#41263) - (SEMVER-MAJOR) http: make TCP noDelay enabled by default (Paolo Insogna) (nodejs#42163) - (SEMVER-MAJOR) lib: enable fetch by default (Michaël Zasso) (nodejs#41811) - (SEMVER-MAJOR) lib: replace validator and error (Mohammed Keyvanzadeh) (nodejs#41678) - (SEMVER-MAJOR) module,repl: support 'node:'-only core modules (Colin Ihrig) (nodejs#42325) - (SEMVER-MAJOR) net: make `server.address()` return an integer for `family` (Antoine du Hamel) (nodejs#41431) - (SEMVER-MAJOR) process: disallow some uses of Object.defineProperty() on process.env (Himself65) (nodejs#28006) - (SEMVER-MAJOR) process: runtime deprecate multipleResolves (Benjamin Gruenbaum) (nodejs#41896) - (SEMVER-MAJOR) readline: fix question still called after closed (Xuguang Mei) (nodejs#42464) - (SEMVER-MAJOR) stream: remove thenable support (Robert Nagy) (nodejs#40773) - (SEMVER-MAJOR) stream: expose web streams globals, remove runtime experimental warning (Antoine du Hamel) (nodejs#42225) - (SEMVER-MAJOR) stream: need to cleanup event listeners if last stream is readable (Xuguang Mei) (nodejs#41954) - (SEMVER-MAJOR) stream: revert revert `map` spec compliance (Benjamin Gruenbaum) (nodejs#41933) - (SEMVER-MAJOR) stream: throw invalid arg type from End Of Stream (Jithil P Ponnan) (nodejs#41766) - (SEMVER-MAJOR) stream: don't emit finish after destroy (Robert Nagy) (nodejs#40852) - (SEMVER-MAJOR) stream: add errored and closed props (Robert Nagy) (nodejs#40696) - (SEMVER-MAJOR) test: add initial test module (Colin Ihrig) (nodejs#42325) - (SEMVER-MAJOR) timers: refactor internal classes to ES2015 syntax (Rabbit) (nodejs#37408) - (SEMVER-MAJOR) tls: represent registeredID numerically always (Tobias Nießen) (nodejs#41561) - (SEMVER-MAJOR) tls: move tls.parseCertString to end-of-life (Tobias Nießen) (nodejs#41479) - (SEMVER-MAJOR) url: throw on NULL in IPv6 hostname (Rich Trott) (nodejs#42313) - (SEMVER-MAJOR) v8: make v8.writeHeapSnapshot() error codes consistent (Darshan Sen) (nodejs#42577) - (SEMVER-MAJOR) v8: make writeHeapSnapshot throw if fopen fails (Antonio Román) (nodejs#41373) - (SEMVER-MAJOR) worker: expose BroadcastChannel as a global (James M Snell) (nodejs#41271) - (SEMVER-MAJOR) worker: graduate BroadcastChannel to supported (James M Snell) (nodejs#41271) PR-URL: nodejs#42262

nodejs-github-bot added needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. labels Mar 25, 2022

meixg changed the title ~~readline: fix question still called after closed.~~ readline: fix question still called after closed. Mar 25, 2022

meixg changed the title ~~readline: fix question still called after closed.~~ readline: fix .question still called after closed. Mar 25, 2022

meixg changed the title ~~readline: fix .question still called after closed.~~ readline: fix .question still called after closed Mar 25, 2022

aduh95 requested changes Mar 25, 2022

View reviewed changes

meixg requested a review from aduh95 March 25, 2022 11:34

aduh95 requested changes Mar 25, 2022

View reviewed changes

meixg requested a review from aduh95 March 25, 2022 13:39

aduh95 reviewed Mar 25, 2022

View reviewed changes

test/parallel/test-readline-interface.js Outdated Show resolved Hide resolved

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

test/parallel/test-readline-promises-interface.js Outdated Show resolved Hide resolved

aduh95 reviewed Mar 28, 2022

View reviewed changes

mcollina approved these changes Mar 28, 2022

View reviewed changes

RaisinTen approved these changes Mar 28, 2022

View reviewed changes

meixg added 7 commits April 6, 2022 10:46

readline: fix question still called after closed

75e0355

resolve: #42450

readline: fix question still called after closed

0e4bd7b

handle promises version

readline: fix question still called after closed

75c38f3

let the promise in a never settling state and make the change in the super class

readline: fix question still called after closed

e57fe91

add promisified question test case

readline: fix question still called after closed

88088f4

change doc

readline: fix question still called after closed

ee388bd

throw an error

readline: fix question still called after closed

6db8686

change according to review

readline: fix question still called after closed

cb80a3a

change to a more generically error code name

RaisinTen requested a review from aduh95 April 6, 2022 04:28

aduh95 approved these changes Apr 6, 2022

View reviewed changes

test/parallel/test-readline-promises-interface.js Outdated Show resolved Hide resolved

aduh95 added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 6, 2022

readline: fix question still called after closed

ac7c846

change according to cr

meixg added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 8, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 8, 2022

meixg added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 8, 2022

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 8, 2022

meixg requested a review from aduh95 April 8, 2022 07:28

meixg removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Apr 8, 2022

aduh95 approved these changes Apr 8, 2022

View reviewed changes

aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 8, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 8, 2022

nodejs-github-bot merged commit d36b60e into nodejs:master Apr 8, 2022

github-actions bot mentioned this pull request Apr 9, 2022

CI Reliability 2022-04-09 nodejs/reliability#246

Open

24 tasks

BethGriggs mentioned this pull request Apr 9, 2022

2022-04-19, Version 18.0.0 (Current) #42262

Merged

This was referenced Apr 10, 2022

CI Reliability 2022-04-10 nodejs/reliability#247

Open

CI Reliability 2022-04-11 nodejs/reliability#248

Open

CI Reliability 2022-04-12 nodejs/reliability#249

Open

CI Reliability 2022-04-13 nodejs/reliability#250

Open

uwilken mentioned this pull request Feb 16, 2023

CLI: sst dev throws error, when committing MFA on Node18 sst/sst#2500

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

readline: fix .question still called after closed #42464

readline: fix .question still called after closed #42464

meixg commented Mar 25, 2022

aduh95 left a comment

aduh95 left a comment

aduh95 commented Mar 25, 2022

RaisinTen commented Mar 26, 2022

meixg commented Mar 28, 2022

aduh95 Mar 28, 2022

mcollina left a comment

RaisinTen left a comment

RaisinTen Mar 28, 2022 •

edited

Loading

RaisinTen commented Apr 5, 2022

meixg commented Apr 6, 2022

aduh95 commented Apr 6, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

	.then(common.mustNotCall())
	.catch(common.expectsError({
	.then(common.mustNotCall(), common.expectsError({

readline: fix .question still called after closed #42464

readline: fix .question still called after closed #42464

Conversation

meixg commented Mar 25, 2022

aduh95 left a comment

Choose a reason for hiding this comment

aduh95 left a comment

Choose a reason for hiding this comment

aduh95 commented Mar 25, 2022

RaisinTen commented Mar 26, 2022

meixg commented Mar 28, 2022

aduh95 Mar 28, 2022

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

RaisinTen left a comment

Choose a reason for hiding this comment

RaisinTen Mar 28, 2022 • edited Loading

Choose a reason for hiding this comment

RaisinTen commented Apr 5, 2022

meixg commented Apr 6, 2022

aduh95 commented Apr 6, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

nodejs-github-bot commented Apr 8, 2022

RaisinTen Mar 28, 2022 •

edited

Loading