diff --git a/doc/api/child_process.md b/doc/api/child_process.md index ec0dac6e45248a..d3a23a163bb2dc 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -1454,12 +1454,13 @@ added: REPLACEME --> * `options` {Object} - * `shouldIgnoreErrors` {boolean} **Default:** `false` - * `useStdErr` {boolean} **Default:** `false` + * `rejectIfNonZeroExitCode` {boolean} **Default:** `false` + * `listenTo` {string} Can be one of `'stdout'`, or `'stderr'`. + **Default:** `'stdout'` * Returns: {readline.InterfaceConstructor} -Convenience method to create a `readline` interface and stream over the stdout -(or stderr if `useStdErr` is `true`) of the process. +Convenience method to create a `node:readline` interface and stream over the +output of the child process. ```mjs import { spawn } from 'node:child_process'; diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index ac52a24b681cb4..880a71005d5e70 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -4,12 +4,15 @@ const { ArrayIsArray, ArrayPrototypePush, ArrayPrototypeReduce, + ArrayPrototypeShift, ArrayPrototypeSlice, FunctionPrototype, FunctionPrototypeCall, ObjectDefineProperty, ObjectSetPrototypeOf, Promise, + PromiseResolve, + PromisePrototypeThen, ReflectApply, SafePromiseRace, StringPrototypeSlice, @@ -541,33 +544,71 @@ ChildProcess.prototype.unref = function() { }; ChildProcess.prototype.readLines = function readLines(options = undefined) { - const { useStdErr = false, ignoreErrors = false } = options ?? kEmptyObject; + const { listenTo, rejectIfNonZeroExitCode = false } = options ?? kEmptyObject; if (options != null) { - validateBoolean(useStdErr, 'options.useStdErr'); - validateBoolean(ignoreErrors, 'options.ignoreErrors'); + if (listenTo != null) validateOneOf(listenTo, 'options.listenTo', ['stdout', 'stderr']); + validateBoolean(rejectIfNonZeroExitCode, 'options.rejectIfNonZeroExitCode'); + } + let input; + switch (listenTo) { + // TODO(aduh95): add case 'both' + + case 'stderr': + input = this.stderr; + break; + + default: + input = this.stdout; } const rlInterface = new Interface({ __proto__: null, signal: this[kSignal], - input: useStdErr ? this.stderr : this.stdout, + input, }); - if (ignoreErrors) { - this.on('error', () => {}); - } else { - const errorPromise = new Promise((_, reject) => this.once('error', reject)); - const exitPromise = new Promise((resolve, reject) => this.once('close', (code) => { - if (code === 0) resolve({ done: true, value: undefined }); - else reject(genericNodeError('Command failed', { pid: this.pid, signal: this.signalCode, status: code })); - })); - rlInterface[SymbolAsyncIterator] = function() { - return { __proto__: null, - next: () => SafePromiseRace([ - errorPromise, - new Promise((resolve) => this.on('line', (value) => resolve({ done: false, value }))), - exitPromise]), - [SymbolAsyncIterator]() { return this; } }; - }; - } + const errorPromise = new Promise((_, reject) => this.once('error', reject)); + const exitPromise = new Promise((resolve, reject) => this.once('close', rejectIfNonZeroExitCode ? (code) => { + if (code === 0) resolve({ done: true, value: undefined }); + else reject(genericNodeError('Command failed', { pid: this.pid, signal: this.signalCode, status: code })); + } : () => resolve({ done: true, value: undefined }))); + rlInterface[SymbolAsyncIterator] = function() { + let resolveNext; + let nextPromise = new Promise((resolve) => { + resolveNext = resolve; + }); + const buffer = ObjectSetPrototypeOf([], null); + this.on('line', (value) => { + if (resolveNext != null) { + resolveNext(value); + resolveNext = null; + } else { + ArrayPrototypePush(buffer, value); + } + }, + ); + return { __proto__: null, + next: () => SafePromiseRace([ + errorPromise, + (() => { + if (nextPromise != null) { + const p = nextPromise; + nextPromise = buffer.length === 0 ? new Promise((resolve) => { + PromisePrototypeThen(p, () => { + resolveNext = resolve; + }); + }) : null; + return p; + } + const iteratorObject = { value: ArrayPrototypeShift(buffer), done: false }; + if (buffer.length === 0) { + nextPromise = new Promise((resolve) => { + resolveNext = resolve; + }); + } + return PromiseResolve(iteratorObject); + })(), + exitPromise]), + [SymbolAsyncIterator]() { return this; } }; + }; return rlInterface; }; diff --git a/test/parallel/test-child-process-readLines.mjs b/test/parallel/test-child-process-readLines.mjs index c613938117978c..5d673984a63f9a 100644 --- a/test/parallel/test-child-process-readLines.mjs +++ b/test/parallel/test-child-process-readLines.mjs @@ -1,13 +1,16 @@ import * as common from '../common/index.mjs'; import assert from 'node:assert'; -import { spawn, exec, execSync } from 'node:child_process'; +import { spawn, exec } from 'node:child_process'; { const lines = exec(`${process.execPath} -p 42`).readLines(); - lines.on('line', common.mustCall((result) => assert.strictEqual(result, '42'))); + lines.on('line', common.mustCall((result) => { + assert.strictEqual(result, '42'); + lines.on('line', common.mustNotCall('We expect only one line event')); + })); } { @@ -21,12 +24,12 @@ import { spawn, exec, execSync } from 'node:child_process'; { const cp = spawn(process.execPath, ['-p', 42]); - [0, 1, '', 'a', 0n, 1n, Symbol(), () => {}, {}, []].forEach((useStdErr) => assert.throws( - () => cp.readLines({ useStdErr }), - { code: 'ERR_INVALID_ARG_TYPE' })); + [0, 1, '', 'a', 0n, 1n, Symbol(), () => {}, {}, []].forEach((listenTo) => assert.throws( + () => cp.readLines({ listenTo }), + { code: 'ERR_INVALID_ARG_VALUE' })); - [0, 1, '', 'a', 0n, 1n, Symbol(), () => {}, {}, []].forEach((ignoreErrors) => assert.throws( - () => cp.readLines({ ignoreErrors }), + [0, 1, '', 'a', 0n, 1n, Symbol(), () => {}, {}, []].forEach((rejectIfNonZeroExitCode) => assert.throws( + () => cp.readLines({ rejectIfNonZeroExitCode }), { code: 'ERR_INVALID_ARG_TYPE' })); } @@ -38,7 +41,7 @@ await assert.rejects(async () => { await assert.rejects(async () => { // eslint-disable-next-line no-unused-vars - for await (const _ of spawn(process.execPath, { signal: AbortSignal.abort() }).readLines({ ignoreErrors: true })); + for await (const _ of spawn(process.execPath, { signal: AbortSignal.abort() }).readLines()); }, { name: 'AbortError' }); { @@ -48,7 +51,7 @@ await assert.rejects(async () => { ['-e', 'setTimeout(()=>console.log("line 2"), 10);setImmediate(()=>console.log("line 1"));'], { signal: ac.signal }); await assert.rejects(async () => { - for await (const line of cp.readLines({ ignoreErrors: true })) { + for await (const line of cp.readLines()) { assert.strictEqual(line, 'line 1'); ac.abort(); } @@ -60,30 +63,26 @@ await assert.rejects(async () => { const cp = spawn(process.execPath, ['-e', 'throw null']); await assert.rejects(async () => { // eslint-disable-next-line no-unused-vars - for await (const _ of cp.readLines()); + for await (const _ of cp.readLines({ rejectIfNonZeroExitCode: true })); }, { pid: cp.pid, status: 1, signal: null }); } { const fn = common.mustCall(); - for await (const line of spawn(process.execPath, ['-e', 'console.error(42)']).readLines({ useStdErr: true })) { + for await (const line of spawn(process.execPath, ['-e', 'console.error(42)']).readLines({ listenTo: 'stderr' })) { assert.strictEqual(line, '42'); fn(); } } { - let stderr; - try { - execSync(`${process.execPath} -e 'throw new Error'`, { stdio: 'pipe', encoding: 'utf-8' }); - } catch (err) { - if (!Array.isArray(err?.output)) throw err; - stderr = err.output[2].split(/\r?\n/); - } + const stderr = (await spawn(process.execPath, ['-e', 'throw new Error']).stderr.toArray()).join('').split(/\r?\n/); const cp = spawn(process.execPath, ['-e', 'throw new Error']); - for await (const line of cp.readLines({ useStdErr: true, ignoreErrors: true })) { + assert.strictEqual(cp.exitCode, null); + for await (const line of cp.readLines({ listenTo: 'stderr' })) { assert.strictEqual(line, stderr.shift()); } + assert.strictEqual(cp.exitCode, 1); assert.deepStrictEqual(stderr, ['']); }