diff --git a/lib/convert/concurrent.js b/lib/convert/concurrent.js index 2e00602edf..4d921e4d26 100644 --- a/lib/convert/concurrent.js +++ b/lib/convert/concurrent.js @@ -1,4 +1,4 @@ -import {createDeferred} from './shared.js'; +import {createDeferred} from '../utils/deferred.js'; // When using multiple `.readable()`/`.writable()`/`.duplex()`, `final` and `destroy` should wait for other streams export const initializeConcurrentStreams = () => ({ diff --git a/lib/convert/readable.js b/lib/convert/readable.js index a50a08fdb1..a63b0c0098 100644 --- a/lib/convert/readable.js +++ b/lib/convert/readable.js @@ -3,9 +3,9 @@ import {callbackify} from 'node:util'; import {BINARY_ENCODINGS} from '../arguments/encoding-option.js'; import {getFromStream} from '../arguments/fd-options.js'; import {iterateOnSubprocessStream, DEFAULT_OBJECT_HIGH_WATER_MARK} from '../io/iterate.js'; +import {createDeferred} from '../utils/deferred.js'; import {addConcurrentStream, waitForConcurrentStreams} from './concurrent.js'; import { - createDeferred, safeWaitForSubprocessStdin, waitForSubprocessStdout, waitForSubprocess, diff --git a/lib/convert/shared.js b/lib/convert/shared.js index 96d2afcf3a..6e3d428348 100644 --- a/lib/convert/shared.js +++ b/lib/convert/shared.js @@ -44,11 +44,3 @@ export const destroyOtherStream = (stream, isOpen, error) => { stream.destroy(); } }; - -export const createDeferred = () => { - let resolve; - const promise = new Promise(resolve_ => { - resolve = resolve_; - }); - return Object.assign(promise, {resolve}); -}; diff --git a/lib/methods/main-async.js b/lib/methods/main-async.js index 916b9a6165..29f4cd225b 100644 --- a/lib/methods/main-async.js +++ b/lib/methods/main-async.js @@ -17,6 +17,7 @@ import {logEarlyResult} from '../verbose/complete.js'; import {makeAllStream} from '../resolve/all-async.js'; import {waitForSubprocessResult} from '../resolve/wait-subprocess.js'; import {addConvertedStreams} from '../convert/add.js'; +import {createDeferred} from '../utils/deferred.js'; import {mergePromise} from './promise.js'; // Main shared logic for all async methods: `execa()`, `execaCommand()`, `$`, `execaNode()` @@ -100,10 +101,11 @@ const spawnSubprocessAsync = ({file, commandArguments, options, startTime, verbo pipeOutputAsync(subprocess, fileDescriptors, controller); cleanupOnExit(subprocess, options, controller); + const onInternalError = createDeferred(); subprocess.kill = subprocessKill.bind(undefined, { kill: subprocess.kill.bind(subprocess), - subprocess, options, + onInternalError, controller, }); subprocess.all = makeAllStream(subprocess, options); @@ -118,13 +120,14 @@ const spawnSubprocessAsync = ({file, commandArguments, options, startTime, verbo originalStreams, command, escapedCommand, + onInternalError, controller, }); return {subprocess, promise}; }; // Asynchronous logic, as opposed to the previous logic which can be run synchronously, i.e. can be returned to user right away -const handlePromise = async ({subprocess, options, startTime, verboseInfo, fileDescriptors, originalStreams, command, escapedCommand, controller}) => { +const handlePromise = async ({subprocess, options, startTime, verboseInfo, fileDescriptors, originalStreams, command, escapedCommand, onInternalError, controller}) => { const context = {timedOut: false}; const [errorInfo, [exitCode, signal], stdioResults, allResult] = await waitForSubprocessResult({ @@ -134,9 +137,11 @@ const handlePromise = async ({subprocess, options, startTime, verboseInfo, fileD verboseInfo, fileDescriptors, originalStreams, + onInternalError, controller, }); controller.abort(); + onInternalError.resolve(); const stdio = stdioResults.map((stdioResult, fdNumber) => stripNewline(stdioResult, options, fdNumber)); const all = stripNewline(allResult, options, 'all'); diff --git a/lib/resolve/wait-subprocess.js b/lib/resolve/wait-subprocess.js index d877148c16..9eeb1d600d 100644 --- a/lib/resolve/wait-subprocess.js +++ b/lib/resolve/wait-subprocess.js @@ -1,6 +1,5 @@ import {once} from 'node:events'; import {isStream as isNodeStream} from 'is-stream'; -import {errorSignal} from '../terminate/kill.js'; import {throwOnTimeout} from '../terminate/timeout.js'; import {isStandardStream} from '../utils/standard-stream.js'; import {TRANSFORM_TYPES} from '../stdio/type.js'; @@ -18,6 +17,7 @@ export const waitForSubprocessResult = async ({ verboseInfo, fileDescriptors, originalStreams, + onInternalError, controller, }) => { const exitPromise = waitForExit(subprocess); @@ -62,8 +62,8 @@ export const waitForSubprocessResult = async ({ ...originalPromises, ...customStreamsEndPromises, ]), + onInternalError, throwOnSubprocessError(subprocess, controller), - throwOnInternalError(subprocess, controller), ...throwOnTimeout(subprocess, timeout, context, controller), ]); } catch (error) { @@ -100,10 +100,3 @@ const throwOnSubprocessError = async (subprocess, {signal}) => { const [error] = await once(subprocess, 'error', {signal}); throw error; }; - -// Fails right away when calling `subprocess.kill(error)`. -// Does not wait for actual signal termination. -const throwOnInternalError = async (subprocess, {signal}) => { - const [error] = await once(subprocess, errorSignal, {signal}); - throw error; -}; diff --git a/lib/terminate/kill.js b/lib/terminate/kill.js index a287c64b4e..0dccfa3f83 100644 --- a/lib/terminate/kill.js +++ b/lib/terminate/kill.js @@ -23,12 +23,12 @@ 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}, + {kill, options: {forceKillAfterDelay, killSignal}, onInternalError, controller}, signalOrError, errorArgument, ) => { const {signal, error} = parseKillArguments(signalOrError, errorArgument, killSignal); - emitKillError(subprocess, error); + emitKillError(error, onInternalError); const killResult = kill(signal); setKillTimeout({ kill, @@ -57,16 +57,15 @@ const parseKillArguments = (signalOrError, errorArgument, killSignal) => { return {signal, error}; }; -const emitKillError = (subprocess, error) => { +// Fails right away when calling `subprocess.kill(error)`. +// Does not wait for actual signal termination. +// Uses a deferred promise instead of the `error` event on the subprocess, as this is less intrusive. +const emitKillError = (error, onInternalError) => { if (error !== undefined) { - subprocess.emit(errorSignal, error); + onInternalError.reject(error); } }; -// Like `error` signal but internal to Execa. -// E.g. does not make subprocess crash when no `error` listener is set. -export const errorSignal = Symbol('error'); - const setKillTimeout = async ({kill, signal, forceKillAfterDelay, killSignal, killResult, controller}) => { if (!shouldForceKill(signal, forceKillAfterDelay, killSignal, killResult)) { return; diff --git a/lib/utils/deferred.js b/lib/utils/deferred.js new file mode 100644 index 0000000000..6c0a9d2728 --- /dev/null +++ b/lib/utils/deferred.js @@ -0,0 +1,7 @@ +export const createDeferred = () => { + const methods = {}; + const promise = new Promise((resolve, reject) => { + Object.assign(methods, {resolve, reject}); + }); + return Object.assign(promise, methods); +}; diff --git a/test/resolve/wait-subprocess.js b/test/resolve/wait-subprocess.js index 8b406d5bb4..7d4a8b3e17 100644 --- a/test/resolve/wait-subprocess.js +++ b/test/resolve/wait-subprocess.js @@ -19,7 +19,7 @@ test('stdio[*] is undefined if ignored - sync', testIgnore, 3, execaSync); const testSubprocessEventsCleanup = async (t, fixtureName) => { const subprocess = execa(fixtureName, {reject: false}); - t.deepEqual(subprocess.eventNames().map(String).sort(), ['Symbol(error)', 'error', 'exit', 'spawn']); + t.deepEqual(subprocess.eventNames().map(String).sort(), ['error', 'exit', 'spawn']); await subprocess; t.deepEqual(subprocess.eventNames(), []); };