Retrieve the subprocess asynchronously

[ehmicky]

We return the subprocess synchronously, as promise.subprocess.

nano-spawn/index.js

Line 23 in 5f449e1

const subprocess = spawn(...escapeArguments(file, commandArguments, forcedShell), spawnOptions);

nano-spawn/index.js

Line 26 in 5f449e1

const resultPromise = getResult(subprocess, start, command);

nano-spawn/index.js

Lines 30 to 31 in 5f449e1

	return Object.assign(resultPromise, {
	subprocess,

That's because node:child_process spawn() is synchronous. However, in our case, this requires some of our own logic to be synchronous so we can return promise.subprocess. This creates the following problems.

Problems

fs.statSync() calls

On Windows, we must make stat syscalls to know whether the binary requires calling cmd.exe or not. We do this twice (since we check two file extensions: .exe and .com) on every directory of the PATH. Due to the above, we must make synchronous calls. That's 20-50 sync stat calls done serially.

Being able to make them asynchronously would be much faster, since we could parallelize those calls.

nano-spawn/windows.js

Lines 34 to 41 in 5f449e1

	return possibleFiles.some(possibleFile => {
	try {
	// This must unfortunately be synchronous because we return the spawned `subprocess` synchronously
	return statSync(possibleFile).isFile();
	} catch {
	return false;
	}
	});

Synchronous exceptions

Even though nanoSpawn() is async, it can throw sometimes synchronously, which is inconsistent. That's because node:child_process spawn() throws synchronously when passing some invalid options, e.g. {uid: -1} or {detached: 'should_be_a_boolean'}.

Early errors

When running a binary that cannot be found (e.g. due to being misspelled, or not being installed), no subprocess is actually spawned at the OS level. However, Node.js returns an "empty" subprocess instance, which is not very useful. It's also tricky, e.g. its subprocess.pid is undefined, leading to potential bugs. Node.js then emits an error event on the subprocess instance, which leads to nanoSpawn() to throw asynchronously.

This situation happens in a couple of other situations, e.g. when spawning too many processes at once, or when hitting OS permissions errors.

Instead of returning a subprocess instance which might be "empty", we should wait for the spawn event first. That event is actually emitted only one tick later.

Output iteration + buffering

Due to the above, we need to spawn the subprocess before knowing whether the user wants to iterate its output line-wise. This leads to the problem described in #33.

Solutions

The subprocess instance is useful for a few purposes such as calling subprocess.kill() or piping subprocess.std*. However, it is meant mostly as an escape hatch, so it's probably ok if it's slightly harder to access.

const promise = nanoSpawn(...);
const subprocess = promise.subprocess;
await promise;

We should make retrieving the subprocess instance asynchronous. One possible implementation is:

const promise = nanoSpawn(...);
const subprocess = await promise.subprocess();
await promise;

Using either callbacks (e.g. an onStart(subprocess) option) or events (e.g. a "start" event with a subprocess payload) is possible too, but harder to use in a promise-based codebase.

If you have any other idea on the specific API, please let me know.

---

What do you think?

[sindresorhus]

const promise = nanoSpawn(...);
const subprocess = await promise.subprocess();
await promise;

That looks fine to me. I cannot think of a better solution.

I would make it a property instead of a function though.

[ehmicky]

Sounds good. 👍 It's not ideal API-wise, but it solves quite a few problems.