diff --git a/lib/arguments/fd-options.js b/lib/arguments/fd-options.js index 3876c5bd4e..941ae6a4f1 100644 --- a/lib/arguments/fd-options.js +++ b/lib/arguments/fd-options.js @@ -1,6 +1,6 @@ import {parseFd} from './specific.js'; -export const getWritable = (destination, to = 'stdin') => { +export const getToStream = (destination, to = 'stdin') => { const isWritable = true; const {options, fileDescriptors} = SUBPROCESS_OPTIONS.get(destination); const fdNumber = getFdNumber(fileDescriptors, to, isWritable); @@ -13,7 +13,7 @@ export const getWritable = (destination, to = 'stdin') => { return destinationStream; }; -export const getReadable = (source, from = 'stdout') => { +export const getFromStream = (source, from = 'stdout') => { const isWritable = false; const {options, fileDescriptors} = SUBPROCESS_OPTIONS.get(source); const fdNumber = getFdNumber(fileDescriptors, from, isWritable); diff --git a/lib/arguments/options.js b/lib/arguments/options.js index e044c37e6e..6cb4075af4 100644 --- a/lib/arguments/options.js +++ b/lib/arguments/options.js @@ -10,7 +10,7 @@ import {normalizeCwd} from './cwd.js'; import {normalizeFileUrl} from './file-url.js'; import {normalizeFdSpecificOptions} from './specific.js'; -export const handleOptions = (filePath, rawArguments, rawOptions) => { +export const normalizeOptions = (filePath, rawArguments, rawOptions) => { rawOptions.cwd = normalizeCwd(rawOptions.cwd); const [processedFile, processedArguments, processedOptions] = handleNodeOption(filePath, rawArguments, rawOptions); diff --git a/lib/convert/iterable.js b/lib/convert/iterable.js index 2754c39903..0e652ba015 100644 --- a/lib/convert/iterable.js +++ b/lib/convert/iterable.js @@ -1,5 +1,5 @@ import {BINARY_ENCODINGS} from '../arguments/encoding-option.js'; -import {getReadable} from '../arguments/fd-options.js'; +import {getFromStream} from '../arguments/fd-options.js'; import {iterateOnSubprocessStream} from '../io/iterate.js'; export const createIterable = (subprocess, encoding, { @@ -8,7 +8,7 @@ export const createIterable = (subprocess, encoding, { preserveNewlines = false, } = {}) => { const binary = binaryOption || BINARY_ENCODINGS.has(encoding); - const subprocessStdout = getReadable(subprocess, from); + const subprocessStdout = getFromStream(subprocess, from); const onStdoutData = iterateOnSubprocessStream({ subprocessStdout, subprocess, diff --git a/lib/convert/readable.js b/lib/convert/readable.js index 1a77528d7a..a50a08fdb1 100644 --- a/lib/convert/readable.js +++ b/lib/convert/readable.js @@ -1,7 +1,7 @@ import {Readable} from 'node:stream'; import {callbackify} from 'node:util'; import {BINARY_ENCODINGS} from '../arguments/encoding-option.js'; -import {getReadable} from '../arguments/fd-options.js'; +import {getFromStream} from '../arguments/fd-options.js'; import {iterateOnSubprocessStream, DEFAULT_OBJECT_HIGH_WATER_MARK} from '../io/iterate.js'; import {addConcurrentStream, waitForConcurrentStreams} from './concurrent.js'; import { @@ -42,7 +42,7 @@ export const createReadable = ({subprocess, concurrentStreams, encoding}, {from, // Retrieve `stdout` (or other stream depending on `from`) export const getSubprocessStdout = (subprocess, from, concurrentStreams) => { - const subprocessStdout = getReadable(subprocess, from); + const subprocessStdout = getFromStream(subprocess, from); const waitReadableDestroy = addConcurrentStream(concurrentStreams, subprocessStdout, 'readableDestroy'); return {subprocessStdout, waitReadableDestroy}; }; diff --git a/lib/convert/writable.js b/lib/convert/writable.js index 6757fe370b..fd727e3ee3 100644 --- a/lib/convert/writable.js +++ b/lib/convert/writable.js @@ -1,6 +1,6 @@ import {Writable} from 'node:stream'; import {callbackify} from 'node:util'; -import {getWritable} from '../arguments/fd-options.js'; +import {getToStream} from '../arguments/fd-options.js'; import {addConcurrentStream, waitForConcurrentStreams} from './concurrent.js'; import { safeWaitForSubprocessStdout, @@ -29,7 +29,7 @@ export const createWritable = ({subprocess, concurrentStreams}, {to} = {}) => { // Retrieve `stdin` (or other stream depending on `to`) export const getSubprocessStdin = (subprocess, to, concurrentStreams) => { - const subprocessStdin = getWritable(subprocess, to); + const subprocessStdin = getToStream(subprocess, to); const waitWritableFinal = addConcurrentStream(concurrentStreams, subprocessStdin, 'writableFinal'); const waitWritableDestroy = addConcurrentStream(concurrentStreams, subprocessStdin, 'writableDestroy'); return {subprocessStdin, waitWritableFinal, waitWritableDestroy}; diff --git a/lib/io/max-buffer.js b/lib/io/max-buffer.js index 1c98f85c85..3d51ba7746 100644 --- a/lib/io/max-buffer.js +++ b/lib/io/max-buffer.js @@ -51,5 +51,15 @@ export const isMaxBufferSync = (resultError, output, maxBuffer) => resultError?. && output !== null && output.some(result => result !== null && result.length > getMaxBufferSync(maxBuffer)); +// When `maxBuffer` is hit, ensure the result is truncated +export const truncateMaxBufferSync = (result, isMaxBuffer, maxBuffer) => { + if (!isMaxBuffer) { + return result; + } + + const maxBufferValue = getMaxBufferSync(maxBuffer); + return result.length > maxBufferValue ? result.slice(0, maxBufferValue) : result; +}; + // `spawnSync()` does not allow differentiating `maxBuffer` per file descriptor, so we always use `stdout` -export const getMaxBufferSync = maxBuffer => maxBuffer[1]; +export const getMaxBufferSync = ([, stdoutMaxBuffer]) => stdoutMaxBuffer; diff --git a/lib/io/output-sync.js b/lib/io/output-sync.js index 9c15c1229f..e92dcbc612 100644 --- a/lib/io/output-sync.js +++ b/lib/io/output-sync.js @@ -4,7 +4,7 @@ import {runGeneratorsSync} from '../transform/generator.js'; import {splitLinesSync} from '../transform/split.js'; import {joinToString, joinToUint8Array, bufferToUint8Array} from '../utils/uint-array.js'; import {FILE_TYPES} from '../stdio/type.js'; -import {getMaxBufferSync} from './max-buffer.js'; +import {truncateMaxBufferSync} from './max-buffer.js'; // Apply `stdout`/`stderr` options, after spawning, in sync mode export const transformOutputSync = ({fileDescriptors, syncResult: {output}, options, isMaxBuffer, verboseInfo}) => { @@ -30,7 +30,7 @@ const transformOutputResultSync = ({result, fileDescriptors, fdNumber, state, is return; } - const truncatedResult = truncateResult(result, isMaxBuffer, getMaxBufferSync(maxBuffer)); + const truncatedResult = truncateMaxBufferSync(result, isMaxBuffer, maxBuffer); const uint8ArrayResult = bufferToUint8Array(truncatedResult); const {stdioItems, objectMode} = fileDescriptors[fdNumber]; const chunks = runOutputGeneratorsSync([uint8ArrayResult], stdioItems, encoding, state); @@ -67,10 +67,6 @@ const transformOutputResultSync = ({result, fileDescriptors, fdNumber, state, is } }; -const truncateResult = (result, isMaxBuffer, maxBuffer) => isMaxBuffer && result.length > maxBuffer - ? result.slice(0, maxBuffer) - : result; - const runOutputGeneratorsSync = (chunks, stdioItems, encoding, state) => { try { return runGeneratorsSync(chunks, stdioItems, encoding, false); diff --git a/lib/methods/bind.js b/lib/methods/bind.js new file mode 100644 index 0000000000..d5fae18c20 --- /dev/null +++ b/lib/methods/bind.js @@ -0,0 +1,23 @@ +import isPlainObject from 'is-plain-obj'; +import {FD_SPECIFIC_OPTIONS} from '../arguments/specific.js'; + +// Deep merge specific options like `env`. Shallow merge the other ones. +export const mergeOptions = (boundOptions, options) => { + const newOptions = Object.fromEntries( + Object.entries(options).map(([optionName, optionValue]) => [ + optionName, + mergeOption(optionName, boundOptions[optionName], optionValue), + ]), + ); + return {...boundOptions, ...newOptions}; +}; + +const mergeOption = (optionName, boundOptionValue, optionValue) => { + if (DEEP_OPTIONS.has(optionName) && isPlainObject(boundOptionValue) && isPlainObject(optionValue)) { + return {...boundOptionValue, ...optionValue}; + } + + return optionValue; +}; + +const DEEP_OPTIONS = new Set(['env', ...FD_SPECIFIC_OPTIONS]); diff --git a/lib/methods/create.js b/lib/methods/create.js index f3f3446725..179e55e617 100644 --- a/lib/methods/create.js +++ b/lib/methods/create.js @@ -1,9 +1,9 @@ import isPlainObject from 'is-plain-obj'; -import {FD_SPECIFIC_OPTIONS} from '../arguments/specific.js'; import {normalizeParameters} from './parameters.js'; import {isTemplateString, parseTemplates} from './template.js'; import {execaCoreSync} from './main-sync.js'; import {execaCoreAsync} from './main-async.js'; +import {mergeOptions} from './bind.js'; export const createExeca = (mapArguments, boundOptions, deepOptions, setBoundExeca) => { const createNested = (mapArguments, boundOptions, setBoundExeca) => createExeca(mapArguments, boundOptions, deepOptions, setBoundExeca); @@ -58,24 +58,3 @@ const parseArguments = ({mapArguments, firstArgument, nextArguments, deepOptions isSync, }; }; - -// Deep merge specific options like `env`. Shallow merge the other ones. -const mergeOptions = (boundOptions, options) => { - const newOptions = Object.fromEntries( - Object.entries(options).map(([optionName, optionValue]) => [ - optionName, - mergeOption(optionName, boundOptions[optionName], optionValue), - ]), - ); - return {...boundOptions, ...newOptions}; -}; - -const mergeOption = (optionName, boundOptionValue, optionValue) => { - if (DEEP_OPTIONS.has(optionName) && isPlainObject(boundOptionValue) && isPlainObject(optionValue)) { - return {...boundOptionValue, ...optionValue}; - } - - return optionValue; -}; - -const DEEP_OPTIONS = new Set(['env', ...FD_SPECIFIC_OPTIONS]); diff --git a/lib/methods/main-async.js b/lib/methods/main-async.js index 6a916b2c13..c888180f2e 100644 --- a/lib/methods/main-async.js +++ b/lib/methods/main-async.js @@ -2,7 +2,7 @@ import {setMaxListeners} from 'node:events'; import {spawn} from 'node:child_process'; import {MaxBufferError} from 'get-stream'; import {handleCommand} from '../arguments/command.js'; -import {handleOptions} from '../arguments/options.js'; +import {normalizeOptions} from '../arguments/options.js'; import {SUBPROCESS_OPTIONS} from '../arguments/fd-options.js'; import {makeError, makeSuccessResult} from '../return/result.js'; import {handleResult} from '../return/reject.js'; @@ -46,7 +46,7 @@ const handleAsyncArguments = (rawFile, rawArguments, rawOptions) => { const {command, escapedCommand, startTime, verboseInfo} = handleCommand(rawFile, rawArguments, rawOptions); try { - const {file, commandArguments, options: normalizedOptions} = handleOptions(rawFile, rawArguments, rawOptions); + const {file, commandArguments, options: normalizedOptions} = normalizeOptions(rawFile, rawArguments, rawOptions); const options = handleAsyncOptions(normalizedOptions); const fileDescriptors = handleStdioAsync(options, verboseInfo); return { diff --git a/lib/methods/main-sync.js b/lib/methods/main-sync.js index ab6c30ac84..38d3d6533d 100644 --- a/lib/methods/main-sync.js +++ b/lib/methods/main-sync.js @@ -1,6 +1,6 @@ import {spawnSync} from 'node:child_process'; import {handleCommand} from '../arguments/command.js'; -import {handleOptions} from '../arguments/options.js'; +import {normalizeOptions} from '../arguments/options.js'; import {makeError, makeEarlyError, makeSuccessResult} from '../return/result.js'; import {handleResult} from '../return/reject.js'; import {handleStdioSync} from '../stdio/handle-sync.js'; @@ -32,7 +32,7 @@ const handleSyncArguments = (rawFile, rawArguments, rawOptions) => { try { const syncOptions = normalizeSyncOptions(rawOptions); - const {file, commandArguments, options} = handleOptions(rawFile, rawArguments, syncOptions); + const {file, commandArguments, options} = normalizeOptions(rawFile, rawArguments, syncOptions); validateSyncOptions(options); const fileDescriptors = handleStdioSync(options, verboseInfo); return { diff --git a/lib/methods/template.js b/lib/methods/template.js index a90d53ce64..0b76e412a3 100644 --- a/lib/methods/template.js +++ b/lib/methods/template.js @@ -1,4 +1,5 @@ import {ChildProcess} from 'node:child_process'; +import isPlainObject from 'is-plain-obj'; import {isUint8Array, uint8ArrayToString} from '../utils/uint-array.js'; export const isTemplateString = templates => Array.isArray(templates) && Array.isArray(templates.raw); @@ -116,26 +117,30 @@ const parseExpression = expression => { return String(expression); } - if ( - typeOfExpression === 'object' - && expression !== null - && !isSubprocess(expression) - && 'stdout' in expression - ) { - const typeOfStdout = typeof expression.stdout; - - if (typeOfStdout === 'string') { - return expression.stdout; - } - - if (isUint8Array(expression.stdout)) { - return uint8ArrayToString(expression.stdout); - } + if (isPlainObject(expression) && 'stdout' in expression) { + return getSubprocessResult(expression); + } - throw new TypeError(`Unexpected "${typeOfStdout}" stdout in template expression`); + if (expression instanceof ChildProcess || Object.prototype.toString.call(expression) === '[object Promise]') { + // eslint-disable-next-line no-template-curly-in-string + throw new TypeError('Unexpected subprocess in template expression. Please use ${await subprocess} instead of ${subprocess}.'); } throw new TypeError(`Unexpected "${typeOfExpression}" in template expression`); }; -const isSubprocess = value => value instanceof ChildProcess; +const getSubprocessResult = ({stdout}) => { + if (typeof stdout === 'string') { + return stdout; + } + + if (isUint8Array(stdout)) { + return uint8ArrayToString(stdout); + } + + if (stdout === undefined) { + throw new TypeError('Missing result.stdout in template expression. This is probably due to the previous subprocess\' "stdout" option.'); + } + + throw new TypeError(`Unexpected "${typeof stdout}" stdout in template expression`); +}; diff --git a/lib/pipe/pipe-arguments.js b/lib/pipe/pipe-arguments.js index eaec04410b..1f732244fb 100644 --- a/lib/pipe/pipe-arguments.js +++ b/lib/pipe/pipe-arguments.js @@ -1,6 +1,6 @@ import {normalizeParameters} from '../methods/parameters.js'; import {getStartTime} from '../return/duration.js'; -import {SUBPROCESS_OPTIONS, getWritable, getReadable} from '../arguments/fd-options.js'; +import {SUBPROCESS_OPTIONS, getToStream, getFromStream} from '../arguments/fd-options.js'; export const normalizePipeArguments = ({source, sourcePromise, boundOptions, createNested}, ...pipeArguments) => { const startTime = getStartTime(); @@ -33,7 +33,7 @@ const getDestinationStream = (boundOptions, createNested, pipeArguments) => { destination, pipeOptions: {from, to, unpipeSignal} = {}, } = getDestination(boundOptions, createNested, ...pipeArguments); - const destinationStream = getWritable(destination, to); + const destinationStream = getToStream(destination, to); return { destination, destinationStream, @@ -76,7 +76,7 @@ const mapDestinationArguments = ({options}) => ({options: {...options, stdin: 'p const getSourceStream = (source, from) => { try { - const sourceStream = getReadable(source, from); + const sourceStream = getFromStream(source, from); return {sourceStream}; } catch (error) { return {sourceError: error}; diff --git a/lib/terminate/kill.js b/lib/terminate/kill.js index cf4b7d245f..2855b5a6fd 100644 --- a/lib/terminate/kill.js +++ b/lib/terminate/kill.js @@ -21,7 +21,11 @@ export const normalizeForceKillAfterDelay = forceKillAfterDelay => { const DEFAULT_FORCE_KILL_TIMEOUT = 1000 * 5; // Monkey-patches `subprocess.kill()` to add `forceKillAfterDelay` behavior and `.kill(error)` -export const subprocessKill = ({kill, subprocess, options: {forceKillAfterDelay, killSignal}, controller}, signalOrError, errorArgument) => { +export const subprocessKill = ( + {kill, subprocess, options: {forceKillAfterDelay, killSignal}, controller}, + signalOrError, + errorArgument, +) => { const {signal, error} = parseKillArguments(signalOrError, errorArgument, killSignal); emitKillError(subprocess, error); const killResult = kill(signal); diff --git a/test/methods/create-bind.js b/test/methods/bind.js similarity index 100% rename from test/methods/create-bind.js rename to test/methods/bind.js diff --git a/test/methods/create-main.js b/test/methods/create.js similarity index 100% rename from test/methods/create-main.js rename to test/methods/create.js diff --git a/test/methods/template.js b/test/methods/template.js index 23c46d10a8..c1efde9ca8 100644 --- a/test/methods/template.js +++ b/test/methods/template.js @@ -297,33 +297,48 @@ test('$`\\t`', testEmptyScript, () => $` `); test('$`\\n`', testEmptyScript, () => $` `); -const testInvalidExpression = (t, invalidExpression, execaMethod) => { - const expression = typeof invalidExpression === 'function' ? invalidExpression() : invalidExpression; +const testInvalidExpression = (t, invalidExpression) => { t.throws( - () => execaMethod`echo.js ${expression}`, + () => $`echo.js ${invalidExpression}`, {message: /in template expression/}, ); }; -test('$ throws on invalid expression - undefined', testInvalidExpression, undefined, $); -test('$ throws on invalid expression - null', testInvalidExpression, null, $); -test('$ throws on invalid expression - true', testInvalidExpression, true, $); -test('$ throws on invalid expression - {}', testInvalidExpression, {}, $); -test('$ throws on invalid expression - {foo: "bar"}', testInvalidExpression, {foo: 'bar'}, $); -test('$ throws on invalid expression - {stdout: undefined}', testInvalidExpression, {stdout: undefined}, $); -test('$ throws on invalid expression - {stdout: 1}', testInvalidExpression, {stdout: 1}, $); -test('$ throws on invalid expression - Promise.resolve()', testInvalidExpression, Promise.resolve(), $); -test('$ throws on invalid expression - Promise.resolve({stdout: "foo"})', testInvalidExpression, Promise.resolve({foo: 'bar'}), $); -test('$ throws on invalid expression - $', testInvalidExpression, () => $`noop.js`, $); -test('$ throws on invalid expression - $(options).sync', testInvalidExpression, () => $({stdio: 'ignore'}).sync`noop.js`, $); -test('$ throws on invalid expression - [undefined]', testInvalidExpression, [undefined], $); -test('$ throws on invalid expression - [null]', testInvalidExpression, [null], $); -test('$ throws on invalid expression - [true]', testInvalidExpression, [true], $); -test('$ throws on invalid expression - [{}]', testInvalidExpression, [{}], $); -test('$ throws on invalid expression - [{foo: "bar"}]', testInvalidExpression, [{foo: 'bar'}], $); -test('$ throws on invalid expression - [{stdout: undefined}]', testInvalidExpression, [{stdout: undefined}], $); -test('$ throws on invalid expression - [{stdout: 1}]', testInvalidExpression, [{stdout: 1}], $); -test('$ throws on invalid expression - [Promise.resolve()]', testInvalidExpression, [Promise.resolve()], $); -test('$ throws on invalid expression - [Promise.resolve({stdout: "foo"})]', testInvalidExpression, [Promise.resolve({stdout: 'foo'})], $); -test('$ throws on invalid expression - [$]', testInvalidExpression, () => [$`noop.js`], $); -test('$ throws on invalid expression - [$(options).sync]', testInvalidExpression, () => [$({stdio: 'ignore'}).sync`noop.js`], $); +test('$ throws on invalid expression - undefined', testInvalidExpression, undefined); +test('$ throws on invalid expression - null', testInvalidExpression, null); +test('$ throws on invalid expression - true', testInvalidExpression, true); +test('$ throws on invalid expression - {}', testInvalidExpression, {}); +test('$ throws on invalid expression - {foo: "bar"}', testInvalidExpression, {foo: 'bar'}); +test('$ throws on invalid expression - {stdout: 1}', testInvalidExpression, {stdout: 1}); +test('$ throws on invalid expression - [undefined]', testInvalidExpression, [undefined]); +test('$ throws on invalid expression - [null]', testInvalidExpression, [null]); +test('$ throws on invalid expression - [true]', testInvalidExpression, [true]); +test('$ throws on invalid expression - [{}]', testInvalidExpression, [{}]); +test('$ throws on invalid expression - [{foo: "bar"}]', testInvalidExpression, [{foo: 'bar'}]); +test('$ throws on invalid expression - [{stdout: 1}]', testInvalidExpression, [{stdout: 1}]); + +const testMissingOutput = (t, invalidExpression) => { + t.throws( + () => $`echo.js ${invalidExpression()}`, + {message: /Missing result.stdout/}, + ); +}; + +test('$ throws on invalid expression - {stdout: undefined}', testMissingOutput, () => ({stdout: undefined})); +test('$ throws on invalid expression - [{stdout: undefined}]', testMissingOutput, () => [{stdout: undefined}]); +test('$ throws on invalid expression - $(options).sync', testMissingOutput, () => $({stdio: 'ignore'}).sync`noop.js`); +test('$ throws on invalid expression - [$(options).sync]', testMissingOutput, () => [$({stdio: 'ignore'}).sync`noop.js`]); + +const testInvalidPromise = (t, invalidExpression) => { + t.throws( + () => $`echo.js ${invalidExpression()}`, + {message: /Please use \${await subprocess}/}, + ); +}; + +test('$ throws on invalid expression - Promise.resolve()', testInvalidPromise, async () => ({})); +test('$ throws on invalid expression - Promise.resolve({stdout: "foo"})', testInvalidPromise, async () => ({foo: 'bar'})); +test('$ throws on invalid expression - [Promise.resolve()]', testInvalidPromise, () => [Promise.resolve()]); +test('$ throws on invalid expression - [Promise.resolve({stdout: "foo"})]', testInvalidPromise, () => [Promise.resolve({stdout: 'foo'})]); +test('$ throws on invalid expression - $', testInvalidPromise, () => $`noop.js`); +test('$ throws on invalid expression - [$]', testInvalidPromise, () => [$`noop.js`]);