diff --git a/index.js b/index.js index f080bf9..107ff04 100644 --- a/index.js +++ b/index.js @@ -16,25 +16,51 @@ export default function nanoSpawn(file, commandArguments = [], options = {}) { const command = getCommand(file, commandArguments); const spawnOptions = getOptions(options); [file, commandArguments] = handleNode(file, commandArguments); - const forcedShell = getForcedShell(file, spawnOptions); - spawnOptions.shell ||= forcedShell; const input = getInput(spawnOptions); - const subprocess = spawn(...escapeArguments(file, commandArguments, forcedShell), spawnOptions); - - useInput(subprocess, input); - const resultPromise = getResult(subprocess, start, command); + const instancePromise = getInstance({ + file, + commandArguments, + spawnOptions, + start, + command, + }); + const resultPromise = getResult(instancePromise, input, start, command); + const stdoutLines = lineIterator(instancePromise, 'stdout', resultPromise); + const stderrLines = lineIterator(instancePromise, 'stderr', resultPromise); - const stdoutLines = lineIterator(subprocess.stdout, resultPromise); - const stderrLines = lineIterator(subprocess.stderr, resultPromise); return Object.assign(resultPromise, { - subprocess, + nodeChildProcess: instancePromise, [Symbol.asyncIterator]: () => combineAsyncIterators(stdoutLines, stderrLines), stdout: stdoutLines, stderr: stderrLines, }); } +const getInstance = async ({file, commandArguments, spawnOptions, start, command}) => { + try { + const forcedShell = await getForcedShell(file, spawnOptions); + spawnOptions.shell ||= forcedShell; + const instance = spawn(...escapeArguments(file, commandArguments, forcedShell), spawnOptions); + + // The `error` event is caught by `once(instance, 'spawn')` and `once(instance, 'close')`. + // But it creates an uncaught exception if it happens exactly one tick after 'spawn'. + // This prevents that. + instance.once('error', () => {}); + + await once(instance, 'spawn'); + return instance; + } catch (error) { + throw getResultError({ + error, + result: initResult(), + instance: {}, + start, + command, + }); + } +}; + const getCommand = (file, commandArguments) => [file, ...commandArguments] .map(part => getCommandPart(part)) .join(' '); @@ -95,48 +121,43 @@ const getInput = ({stdio}) => { return input; }; -const useInput = (subprocess, input) => { +const useInput = (instance, input) => { if (input !== undefined) { - subprocess.stdin.end(input); + instance.stdin.end(input); } }; -const getResult = async (subprocess, start, command) => { - const result = {}; - const onExit = waitForExit(subprocess); - const onStdoutDone = bufferOutput(subprocess.stdout, result, 'stdout'); - const onStderrDone = bufferOutput(subprocess.stderr, result, 'stderr'); +const getResult = async (instancePromise, input, start, command) => { + const instance = await instancePromise; + useInput(instance, input); + const result = initResult(); + const onExit = once(instance, 'close'); + const onStdoutDone = bufferOutput(instance.stdout, result, 'stdout'); + const onStderrDone = bufferOutput(instance.stderr, result, 'stderr'); try { await Promise.all([onExit, onStdoutDone, onStderrDone]); - checkFailure(command, getErrorOutput(subprocess)); + checkFailure(command, getErrorOutput(instance)); return getOutput(result, command, start); } catch (error) { await Promise.allSettled([onExit, onStdoutDone, onStderrDone]); - throw Object.assign( - getResultError(error, command), - getErrorOutput(subprocess), - getOutput(result, command, start), - ); + throw getResultError({ + error, + result, + instance, + start, + command, + }); } }; -// The `error` event on subprocess is emitted either: -// - Before `spawn`, e.g. for a non-existing executable file. -// Then, `subprocess.pid` is `undefined` and `close` is never emitted. -// - After `spawn`, e.g. for the `signal` option. -// Then, `subprocess.pid` is set and `close` is always emitted. -const waitForExit = async subprocess => { - try { - await once(subprocess, 'close'); - } catch (error) { - if (subprocess.pid !== undefined) { - await Promise.allSettled([once(subprocess, 'close')]); - } +const initResult = () => ({stdout: '', stderr: ''}); - throw error; - } -}; +const getResultError = ({error, result, instance, start, command}) => Object.assign( + getErrorInstance(error, command), + getErrorOutput(instance), + getOutput(result, command, start), +); const bufferOutput = async (stream, result, streamName) => { if (!stream) { @@ -144,7 +165,6 @@ const bufferOutput = async (stream, result, streamName) => { } stream.setEncoding('utf8'); - result[streamName] = ''; stream.on('data', chunk => { result[streamName] += chunk; }); @@ -163,7 +183,7 @@ const stripNewline = input => input?.at(-1) === '\n' : input; const getErrorOutput = ({exitCode, signalCode}) => ({ - // `exitCode` can be a negative number (`errno`) when the `error` event is emitted on the subprocess + // `exitCode` can be a negative number (`errno`) when the `error` event is emitted on the `instance` ...(exitCode === null || exitCode < 1 ? {} : {exitCode}), ...(signalCode === null ? {} : {signalName: signalCode}), }); @@ -178,6 +198,6 @@ const checkFailure = (command, {exitCode, signalName}) => { } }; -const getResultError = (error, command) => error?.message.startsWith('Command ') +const getErrorInstance = (error, command) => error?.message.startsWith('Command ') ? error : new Error(`Command failed: ${command}`, {cause: error}); diff --git a/iterable.js b/iterable.js index 9d6e94b..853d2eb 100644 --- a/iterable.js +++ b/iterable.js @@ -30,7 +30,9 @@ const getNext = async iterator => { } }; -export async function * lineIterator(stream, resultPromise) { +export async function * lineIterator(instancePromise, streamName, resultPromise) { + const instance = await instancePromise; + const stream = instance[streamName]; if (!stream) { return; } diff --git a/test.js b/test.js index 96fbd88..1338ad5 100644 --- a/test.js +++ b/test.js @@ -8,6 +8,7 @@ import pathKey from 'path-key'; import {red} from 'yoctocolors'; import nanoSpawn from './index.js'; +const isLinux = process.platform === 'linux'; const isWindows = process.platform === 'win32'; const FIXTURES_URL = new URL('fixtures', import.meta.url); const fixturesPath = fileURLToPath(FIXTURES_URL); @@ -36,49 +37,56 @@ test('Can pass options.argv0, shell', async t => { test('Can pass options.stdin', async t => { const promise = nanoSpawn('node', ['--version'], {stdin: 'ignore'}); - t.is(promise.subprocess.stdin, null); + const {stdin} = await promise.nodeChildProcess; + t.is(stdin, null); await promise; }); test('Can pass options.stdout', async t => { const promise = nanoSpawn('node', ['--version'], {stdout: 'ignore'}); - t.is(promise.subprocess.stdout, null); + const {stdout} = await promise.nodeChildProcess; + t.is(stdout, null); await promise; }); test('Can pass options.stderr', async t => { const promise = nanoSpawn('node', ['--version'], {stderr: 'ignore'}); - t.is(promise.subprocess.stderr, null); + const {stderr} = await promise.nodeChildProcess; + t.is(stderr, null); await promise; }); test('Can pass options.stdio array', async t => { const promise = nanoSpawn('node', ['--version'], {stdio: ['ignore', 'pipe', 'pipe', 'pipe']}); - t.is(promise.subprocess.stdin, null); - t.not(promise.subprocess.stdout, null); - t.not(promise.subprocess.stderr, null); - t.not(promise.subprocess.stdio[3], null); + const {stdin, stdout, stderr, stdio} = await promise.nodeChildProcess; + t.is(stdin, null); + t.not(stdout, null); + t.not(stderr, null); + t.not(stdio[3], null); await promise; }); test('Can pass options.stdio string', async t => { const promise = nanoSpawn('node', ['--version'], {stdio: 'ignore'}); - t.is(promise.subprocess.stdin, null); - t.is(promise.subprocess.stdout, null); - t.is(promise.subprocess.stderr, null); - t.is(promise.subprocess.stdio.length, 3); + const {stdin, stdout, stderr, stdio} = await promise.nodeChildProcess; + t.is(stdin, null); + t.is(stdout, null); + t.is(stderr, null); + t.is(stdio.length, 3); await promise; }); test('options.stdio array has priority over options.stdout', async t => { const promise = nanoSpawn('node', ['--version'], {stdio: ['pipe', 'pipe', 'pipe'], stdout: 'ignore'}); - t.not(promise.subprocess.stdout, null); + const {stdout} = await promise.nodeChildProcess; + t.not(stdout, null); await promise; }); test('options.stdio string has priority over options.stdout', async t => { const promise = nanoSpawn('node', ['--version'], {stdio: 'pipe', stdout: 'ignore'}); - t.not(promise.subprocess.stdout, null); + const {stdout} = await promise.nodeChildProcess; + t.not(stdout, null); await promise; }); @@ -129,13 +137,13 @@ test('Error on signal termination', async t => { t.is(cause, undefined); }); -test('Error on invalid child_process options', t => { - const {exitCode, signalName, message, cause} = t.throws(() => nanoSpawn('node', ['--version'], {detached: 'true'})); +test('Error on invalid child_process options', async t => { + const {exitCode, signalName, message, cause} = await t.throwsAsync(nanoSpawn('node', ['--version'], {detached: 'true'})); t.is(exitCode, undefined); t.is(signalName, undefined); - t.true(message.includes('options.detached')); - t.false(message.includes('Command')); - t.is(cause, undefined); + t.is(message, 'Command failed: node --version'); + t.true(cause.message.includes('options.detached')); + t.false(cause.message.includes('Command')); }); test('Error on "error" event before spawn', async t => { @@ -148,16 +156,43 @@ test('Error on "error" event before spawn', async t => { } }); -test('Error on "error" event after spawn', async t => { +test('Error on "error" event during spawn', async t => { const error = new Error(testString); const {exitCode, signalName, message, cause} = await t.throwsAsync(nanoSpawn('node', {signal: AbortSignal.abort(error)})); t.is(exitCode, undefined); t.is(signalName, 'SIGTERM'); - t.is(message, 'Command failed: node'); - t.is(cause.message, 'The operation was aborted'); - t.is(cause.cause, error); + t.is(message, 'Command was terminated with SIGTERM: node'); + t.is(cause, undefined); +}); + +test('Error on "error" event during spawn, with iteration', async t => { + const error = new Error(testString); + const promise = nanoSpawn('node', {signal: AbortSignal.abort(error)}); + const {exitCode, signalName, message, cause} = await t.throwsAsync(arrayFromAsync(promise.stdout)); + t.is(exitCode, undefined); + t.is(signalName, 'SIGTERM'); + t.is(message, 'Command was terminated with SIGTERM: node'); + t.is(cause, undefined); }); +// The `signal` option sends `SIGTERM`. +// Whether the subprocess is terminated before or after an `error` event is emitted depends on the speed of the OS syscall. +if (isLinux) { + test('Error on "error" event after spawn', async t => { + const error = new Error(testString); + const controller = new AbortController(); + const promise = nanoSpawn('node', {signal: controller.signal}); + await promise.nodeChildProcess; + controller.abort(error); + const {exitCode, signalName, message, cause} = await t.throwsAsync(promise); + t.is(exitCode, undefined); + t.is(signalName, undefined); + t.is(message, 'Command failed: node'); + t.is(cause.message, 'The operation was aborted'); + t.is(cause.cause, error); + }); +} + test('promise.stdout can be iterated', async t => { const promise = nanoSpawn('node', ['-e', 'console.log(".")']); const lines = await arrayFromAsync(promise.stdout); @@ -263,16 +298,16 @@ test('promise[Symbol.asyncIterator] has no iterations if only options.stdout + o t.deepEqual(lines, []); }); -test('result.stdout is undefined if options.stdout "ignore"', async t => { +test('result.stdout is an empty string if options.stdout "ignore"', async t => { const {stdout, stderr} = await nanoSpawn('node', ['-e', 'console.log("."); console.error(".");'], {stdout: 'ignore'}); - t.is(stdout, undefined); + t.is(stdout, ''); t.is(stderr, '.'); }); -test('result.stderr is undefined if options.stderr "ignore"', async t => { +test('result.stderr is an empty string if options.stderr "ignore"', async t => { const {stdout, stderr} = await nanoSpawn('node', ['-e', 'console.log("."); console.error(".");'], {stderr: 'ignore'}); t.is(stdout, '.'); - t.is(stderr, undefined); + t.is(stderr, ''); }); test('promise[Symbol.asyncIterator] is line-wise', async t => { @@ -358,9 +393,10 @@ const multibyteFirstHalf = multibyteUint8Array.slice(0, 3); const multibyteSecondHalf = multibyteUint8Array.slice(3); const writeMultibyte = async promise => { - promise.subprocess.stdin.write(multibyteFirstHalf); + const {stdin} = await promise.nodeChildProcess; + stdin.write(multibyteFirstHalf); await setTimeout(1e2); - promise.subprocess.stdin.end(multibyteSecondHalf); + stdin.end(multibyteSecondHalf); }; test.serial('promise.stdout works with multibyte sequences', async t => { @@ -380,7 +416,8 @@ test.serial('result.stdout works with multibyte sequences', async t => { test('Handles promise.stdout error', async t => { const promise = nanoSpawn('node', ['--version']); const error = new Error(testString); - promise.subprocess.stdout.emit('error', error); + const {stdout} = await promise.nodeChildProcess; + stdout.emit('error', error); const {cause} = await t.throwsAsync(arrayFromAsync(promise.stdout)); t.is(cause, error); }); @@ -388,7 +425,8 @@ test('Handles promise.stdout error', async t => { test('Handles promise.stderr error', async t => { const promise = nanoSpawn('node', ['--version']); const error = new Error(testString); - promise.subprocess.stderr.emit('error', error); + const {stderr} = await promise.nodeChildProcess; + stderr.emit('error', error); const {cause} = await t.throwsAsync(arrayFromAsync(promise.stderr)); t.is(cause, error); }); @@ -396,7 +434,8 @@ test('Handles promise.stderr error', async t => { test('Handles promise.stdout error in promise[Symbol.asyncIterator]', async t => { const promise = nanoSpawn('node', ['--version']); const error = new Error(testString); - promise.subprocess.stdout.emit('error', error); + const {stdout} = await promise.nodeChildProcess; + stdout.emit('error', error); const {cause} = await t.throwsAsync(arrayFromAsync(promise)); t.is(cause, error); }); @@ -404,7 +443,8 @@ test('Handles promise.stdout error in promise[Symbol.asyncIterator]', async t => test('Handles promise.stderr error in promise[Symbol.asyncIterator]', async t => { const promise = nanoSpawn('node', ['--version']); const error = new Error(testString); - promise.subprocess.stderr.emit('error', error); + const {stderr} = await promise.nodeChildProcess; + stderr.emit('error', error); const {cause} = await t.throwsAsync(arrayFromAsync(promise)); t.is(cause, error); }); @@ -412,7 +452,8 @@ test('Handles promise.stderr error in promise[Symbol.asyncIterator]', async t => test('Handles result.stdout error', async t => { const promise = nanoSpawn('node', ['--version']); const error = new Error(testString); - promise.subprocess.stdout.emit('error', error); + const {stdout} = await promise.nodeChildProcess; + stdout.emit('error', error); const {cause} = await t.throwsAsync(promise); t.is(cause, error); }); @@ -420,7 +461,8 @@ test('Handles result.stdout error', async t => { test('Handles result.stderr error', async t => { const promise = nanoSpawn('node', ['--version']); const error = new Error(testString); - promise.subprocess.stderr.emit('error', error); + const {stderr} = await promise.nodeChildProcess; + stderr.emit('error', error); const {cause} = await t.throwsAsync(promise); t.is(cause, error); }); @@ -432,10 +474,11 @@ test.serial('promise.stdout iteration break waits for the subprocess success', a // eslint-disable-next-line no-unreachable-loop for await (const line of promise.stdout) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); break; @@ -453,10 +496,11 @@ test.serial('promise[Symbol.asyncIterator] iteration break waits for the subproc // eslint-disable-next-line no-unreachable-loop for await (const line of promise) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); break; @@ -476,10 +520,11 @@ test.serial('promise.stdout iteration exception waits for the subprocess success // eslint-disable-next-line no-unreachable-loop for await (const line of promise.stdout) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); throw cause; @@ -502,10 +547,11 @@ test.serial('promise[Symbol.asyncIterator] iteration exception waits for the sub // eslint-disable-next-line no-unreachable-loop for await (const line of promise) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); throw cause; @@ -528,10 +574,11 @@ test.serial('promise.stdout iteration break waits for the subprocess failure', a // eslint-disable-next-line no-unreachable-loop for await (const line of promise.stdout) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); break; @@ -555,10 +602,11 @@ test.serial('promise[Symbol.asyncIterator] iteration break waits for the subproc // eslint-disable-next-line no-unreachable-loop for await (const line of promise) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); break; @@ -582,10 +630,11 @@ test.serial('promise.stdout iteration exception waits for the subprocess failure // eslint-disable-next-line no-unreachable-loop for await (const line of promise.stdout) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); throw cause; @@ -609,10 +658,11 @@ test.serial('promise[Symbol.asyncIterator] iteration exception waits for the sub // eslint-disable-next-line no-unreachable-loop for await (const line of promise) { t.is(line, 'a'); - globalThis.setTimeout(() => { - t.true(promise.subprocess.stdout.readable); - t.true(promise.subprocess.stdin.writable); - promise.subprocess.stdin.end('b'); + globalThis.setTimeout(async () => { + const {stdin, stdout} = await promise.nodeChildProcess; + t.true(stdout.readable); + t.true(stdin.writable); + stdin.end('b'); done = true; }, 1e2); throw cause; @@ -635,9 +685,10 @@ test('Returns a promise', async t => { await promise; }); -test('promise.subprocess is set', async t => { +test('promise.nodeChildProcess is set', async t => { const promise = nanoSpawn('node'); - promise.subprocess.kill(); + const nodeChildProcess = await promise.nodeChildProcess; + nodeChildProcess.kill(); const {signalName} = await t.throwsAsync(promise); t.is(signalName, 'SIGTERM'); @@ -759,6 +810,12 @@ if (isWindows) { test('Can run .cmd file, no shell', testCmd, false); test('Can run .cmd file, shell', testCmd, true); + test('Memoize .cmd file logic', async t => { + await nanoSpawn('spawnecho.cmd', [testString], {cwd: FIXTURES_URL}); + const {stdout} = await nanoSpawn('spawnecho.cmd', [testString], {cwd: FIXTURES_URL}); + t.is(stdout, testString); + }); + test('Uses PATHEXT by default', async t => { const {stdout} = await nanoSpawn('spawnecho', [testString], {cwd: FIXTURES_URL}); t.is(stdout, testString); @@ -1045,7 +1102,7 @@ if (isWindows) { const TEST_NODE_VERSION = '18.0.0'; -test('Keeps Node version', async t => { +test.serial('Keeps Node version', async t => { const {path: nodePath} = await getNode(TEST_NODE_VERSION); t.not(nodePath, process.execPath); const {stdout} = await nanoSpawn(nodePath, ['node-version.js'], {cwd: FIXTURES_URL}); diff --git a/windows.js b/windows.js index f0b789e..d56c49e 100644 --- a/windows.js +++ b/windows.js @@ -1,4 +1,4 @@ -import {statSync} from 'node:fs'; +import {stat} from 'node:fs/promises'; import path from 'node:path'; import process from 'node:process'; @@ -7,21 +7,26 @@ import process from 'node:process'; // We detect this situation and automatically: // - Set the `shell: true` option // - Escape shell-specific characters -export const getForcedShell = (file, {shell, cwd, env = process.env}) => process.platform === 'win32' +export const getForcedShell = async (file, {shell, cwd, env = process.env}) => process.platform === 'win32' && !shell - && !isExe(file, cwd, env); + && !(await isExe(file, cwd, env)); // Detect whether the executable file is a *.exe or *.com file. // Windows allows omitting file extensions (present in the `PATHEXT` environment variable). // Therefore we must use the `PATH` environment variable and make `stat` calls to check this. // Environment variables are case-insensitive on Windows, so we check both `PATH` and `Path`. -const isExe = (file, cwd, {Path = '', PATH = Path}) => { +// eslint-disable-next-line no-return-assign +const isExe = async (file, cwd, {Path = '', PATH = Path}) => // If the *.exe or *.com file extension was not omitted. // Windows common file systems are case-insensitive. - if (exeExtensions.some(extension => file.toLowerCase().endsWith(extension))) { - return true; - } + exeExtensions.some(extension => file.toLowerCase().endsWith(extension)) + // Use returned assignment to keep code small + || (EXE_MEMO[`${file}\0${cwd}\0${PATH}`] ??= await mIsExe(file, cwd, PATH)); +// Memoize the following function, for performance +const EXE_MEMO = {}; + +const mIsExe = async (file, cwd, PATH) => { const parts = PATH // `PATH` is ;-separated on Windows .split(path.delimiter) @@ -29,16 +34,28 @@ const isExe = (file, cwd, {Path = '', PATH = Path}) => { .filter(Boolean) // `PATH` parts can be double quoted on Windows .map(part => part.replace(/^"(.*)"$/, '$1')); - const possibleFiles = exeExtensions.flatMap(extension => - [cwd, ...parts].map(part => `${path.resolve(part, file)}${extension}`)); - return possibleFiles.some(possibleFile => { - try { - // This must unfortunately be synchronous because we return the spawned `subprocess` synchronously - return statSync(possibleFile).isFile(); - } catch { - return false; - } - }); + + // For performance, parallelize and stop iteration as soon as an *.exe of *.com file is found + try { + await Promise.all(exeExtensions + .flatMap(extension => + [cwd, ...parts].map(part => `${path.resolve(part, file)}${extension}`)) + .map(async possibleFile => { + let fileStat; + try { + fileStat = await stat(possibleFile); + } catch {} + + if (fileStat?.isFile()) { + // eslint-disable-next-line no-throw-literal + throw 0; + } + })); + } catch { + return true; + } + + return false; }; // Other file extensions require using a shell