Fix missing stdout/stderr data and unhandled promise rejections

[ehmicky]

Background

Currently, childProcess.stdout and childProcess.stderr are only consumed if await childProcess is called. This was implemented by #95 to solve #84. The main rationale is to avoid buffering result.stdout and result.stderr if the user does not use those, since this requires CPU, I/O and memory (potentially reaching the maxBuffer limit).

Eventually, the buffer: false option was also added for this use case.

The way this is implemented is that the process exit/error handling and stream consumption logic only starts after await childProcess has been called. Unfortunately, this implementation creates the following problems.

Unhandled promise rejections

The above pattern encourages those users to not call await childProcess. However, this results in unhandled promise rejections.

Even when await childProcess is not called, Execa still creates a child process promise. When the child process fails, that promise is rejected. If await childProcess is not called, this results in an unhandledRejection event on the parent process, which crashes it. This also happens when the child process times out (timeout option) or when childProcess.stdout or childProcess.stderr abort or error.

Missing stdout/stderr data if await childProcess is late

When users need to call the childProcess.* methods or access its properties, they need to delay calling await childProcess.

const childProcess = execa(...)
childProcess.stdout.on('data', ...)
const result = await childProcess

This pattern becomes problematic when await childProcess is not done in the same microtask. For example:

const childProcess = execa(...)
childProcess.stdout.on('data', ...)
await doSomethingAsync()
const result = await childProcess

When this happens, childProcess.stdout|stderr only starts being read when await childProcess is called. This means result.stdout will miss any data output while doSomethingAsync() was ongoing. Since that data is consumed by childProcess.stdout.on('data', ...), it won't be buffered by Node.js.

Missing stdout/stderr data if the child process is fast

For similar reasons, result.stdout and result.stderr will miss data if the child process exits before await childProcess is called. This is more likely to happen if the child process has a short duration.

We have the following tests which show this problem.

execa/test/stream.js

Lines 211 to 225 in 6cf1c5e

	// This test is not the desired behavior, but is the current one.
	// I.e. this is mostly meant for documentation and regression testing.
	test.serial('Processes might successfully exit before their stdout is read', async t => {
	const childProcess = execa('noop.js', ['foobar']);
	await setTimeout(1e3);
	const {stdout} = await childProcess;
	t.is(stdout, '');
	});
	test.serial('Processes might fail before their stdout is read', async t => {
	const childProcess = execa('noop-fail.js', ['foobar'], {reject: false});
	await setTimeout(1e3);
	const {stdout} = await childProcess;
	t.is(stdout, '');
	});

Solution

Overall, the pattern of avoiding to consume childProcess.stdout and childProcess.stderr by not calling await childProcess is not something we should recommend, for the above reasons. Instead, the buffer: false option should be used.

Most users do use await childProcess (right away or eventually). For the ones who do not, they will only notice a difference in terms of resources (CPU, I/O, memory), and only if they do not use buffer: false.

To fix the above problems, we should start consuming childProcess.stdout and childProcess.stderr as soon as the child process is spawned, before await childProcess is called.

PR at #658.

[sindresorhus]

Thanks for explaining so thoroughly. Makes total sense to do this.