From f12250b80f0ece99e45b216cc15163a2443b2cb4 Mon Sep 17 00:00:00 2001 From: Adrian Moldovan <3854374+adimoldovan@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:11:49 +0300 Subject: [PATCH 1/2] Build/Test Tools: Propagate the exit status of the local environment commands. `start.js` runs `docker compose up` through `spawnSync` and never inspects the result, so a failed pull does not fail the script. Execution continues into `composer update -W` with containers that may not exist, and the error surfaces later and in the wrong place. `docker.js` calls `process.exit( returns.status )`, and `status` is `null` when Docker cannot be spawned. `process.exit( null )` exits 0, so `npm run env:pull` reports success when the Docker CLI is missing. Reproduce with: LOCAL_PHP=this-tag-does-not-exist npm run env:start; echo $? See #65745. --- tools/local-env/scripts/docker.js | 8 +++++++- tools/local-env/scripts/start.js | 9 ++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tools/local-env/scripts/docker.js b/tools/local-env/scripts/docker.js index c7b11f0058424..5eaead5fd09d4 100644 --- a/tools/local-env/scripts/docker.js +++ b/tools/local-env/scripts/docker.js @@ -38,4 +38,10 @@ const returns = spawnSync( { stdio: 'inherit' } ); -process.exit( returns.status ); +if ( returns.error ) { + console.error( `Could not run Docker Compose. ${ returns.error.message }` ); +} + +// `status` is null when Docker could not be spawned at all, or was killed by a signal. Ctrl+C on +// a long-running command such as `env:logs` is not a failure worth an npm error block. +process.exit( returns.signal ? 0 : ( returns.status ?? 1 ) ); diff --git a/tools/local-env/scripts/start.js b/tools/local-env/scripts/start.js index 66559d4c10b85..110201c74d4bb 100644 --- a/tools/local-env/scripts/start.js +++ b/tools/local-env/scripts/start.js @@ -32,7 +32,7 @@ if ( process.env.LOCAL_PHP_MEMCACHED === 'true' ) { containers.push( 'memcached' ); } -spawnSync( +const up = spawnSync( 'docker', [ 'compose', @@ -45,6 +45,13 @@ spawnSync( { stdio: 'inherit' } ); +if ( up.status !== 0 ) { + console.error( `Could not start the Docker containers.${ up.error ? ` ${ up.error.message }` : '' }` ); + + // `status` is null when Docker could not be spawned at all, or was killed by a signal. + process.exit( up.status ?? 1 ); +} + // If Docker Toolbox is being used, we need to manually forward LOCAL_PORT to the Docker VM. if ( process.env.DOCKER_TOOLBOX_INSTALL_PATH ) { // VBoxManage is added to the PATH on every platform except Windows. From a88479c520c417cd6a9f5fbd710625ea46ef9133 Mon Sep 17 00:00:00 2001 From: Adrian Moldovan <3854374+adimoldovan@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:49:54 +0300 Subject: [PATCH 2/2] Build/Test Tools: Only treat SIGINT as a clean exit of a local environment command. `docker.js` exited 0 for any signal, so a command killed by SIGTERM or SIGKILL reported success. That reintroduced the false success this ticket set out to remove: the retry loop in `reusable-phpunit-tests-v3.yml` branches on the status of `npm run env:pull` and would treat a killed pull as a completed one. Restrict the exemption to SIGINT, which is how a long-running command such as `env:logs` is normally ended. Report every other signal and exit non-zero. `start.js` exempts no signal, because `env:start` runs `composer update -W` next and that must not run against containers that never came up. Say so in the comment, so the difference between the two files is deliberate. Also fold the unreachable `up.error` branch in `start.js` into the failure message. The `docker info` check above it already throws when the Docker CLI is missing or the daemon is down. --- tools/local-env/scripts/docker.js | 9 ++++++--- tools/local-env/scripts/start.js | 6 +++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/tools/local-env/scripts/docker.js b/tools/local-env/scripts/docker.js index 5eaead5fd09d4..ce71a4c075cd6 100644 --- a/tools/local-env/scripts/docker.js +++ b/tools/local-env/scripts/docker.js @@ -40,8 +40,11 @@ const returns = spawnSync( if ( returns.error ) { console.error( `Could not run Docker Compose. ${ returns.error.message }` ); +} else if ( returns.signal && returns.signal !== 'SIGINT' ) { + console.error( `Docker Compose was terminated by ${ returns.signal }.` ); } -// `status` is null when Docker could not be spawned at all, or was killed by a signal. Ctrl+C on -// a long-running command such as `env:logs` is not a failure worth an npm error block. -process.exit( returns.signal ? 0 : ( returns.status ?? 1 ) ); +// `status` is null when Docker could not be spawned at all, or was killed by a signal. SIGINT is +// how a long-running command such as `env:logs` is normally ended, so it is not a failure worth an +// npm error block. Every other signal means the command was killed before it finished. +process.exit( returns.signal === 'SIGINT' ? 0 : ( returns.status ?? 1 ) ); diff --git a/tools/local-env/scripts/start.js b/tools/local-env/scripts/start.js index 110201c74d4bb..a8e5dc868bf9e 100644 --- a/tools/local-env/scripts/start.js +++ b/tools/local-env/scripts/start.js @@ -45,8 +45,12 @@ const up = spawnSync( { stdio: 'inherit' } ); +// No signal is exempt here, unlike in `docker.js`: `env:start` runs `composer update -W` next, and +// that must not run against containers that never came up. if ( up.status !== 0 ) { - console.error( `Could not start the Docker containers.${ up.error ? ` ${ up.error.message }` : '' }` ); + const reason = up.signal ? `It was terminated by ${ up.signal }.` : up.error?.message ?? ''; + + console.error( `Could not start the Docker containers. ${ reason }`.trim() ); // `status` is null when Docker could not be spawned at all, or was killed by a signal. process.exit( up.status ?? 1 );