diff --git a/experiments/issue-38-error-exitcode-competitors.mjs b/experiments/issue-38-error-exitcode-competitors.mjs new file mode 100644 index 00000000..381b385e --- /dev/null +++ b/experiments/issue-38-error-exitcode-competitors.mjs @@ -0,0 +1,137 @@ +// Which property carries the exit status of a failing command? +// Node.js `child_process` names it `code`, while Execa, zx, nano-spawn and the +// Bun shell name it `exitCode`. Issue #38 asks command-stream to answer to both +// names, so this probe prints what every implementation actually exposes. +// Optional packages are reported as unavailable instead of being required by +// this repository. +// +// References: +// https://github.com/link-foundation/command-stream/issues/38 +// https://nodejs.org/api/child_process.html#child_processexeccommand-options-callback +// https://github.com/sindresorhus/execa/blob/main/docs/errors.md +// https://google.github.io/zx/process-output +// https://bun.com/docs/runtime/shell +// +// Run: bun experiments/issue-38-error-exitcode-competitors.mjs + +import { exec as nodeExec } from 'node:child_process'; +import { $, shell } from '../js/src/$.mjs'; + +const EXIT_CODE = 23; +const FAILING_COMMAND = `node -e "process.exit(${EXIT_CODE})"`; + +async function optionalImport(name) { + try { + return await import(name); + } catch (error) { + if (error?.code === 'ERR_MODULE_NOT_FOUND') { + return null; + } + throw error; + } +} + +// Returns the value thrown (or resolved) by a failing command, or null when the +// implementation is not installed here. +async function commandStream() { + shell.errexit(true); + try { + return await $({ mirror: false })`node -e "process.exit(${EXIT_CODE})"`; + } catch (error) { + return error; + } finally { + shell.errexit(false); + } +} + +function nodeChildProcess() { + return new Promise((resolve) => { + nodeExec(FAILING_COMMAND, (error) => resolve(error)); + }); +} + +async function bunShell() { + if (typeof Bun === 'undefined') { + return null; + } + const { $: bun$ } = await import('bun'); + try { + return await bun$`node -e ${`process.exit(${EXIT_CODE})`}`.quiet(); + } catch (error) { + return error; + } +} + +async function zx() { + const module = await optionalImport('zx'); + if (!module) { + return null; + } + try { + return await module.$({ + quiet: true, + })`node -e ${`process.exit(${EXIT_CODE})`}`; + } catch (error) { + return error; + } +} + +async function execa() { + const module = await optionalImport('execa'); + if (!module) { + return null; + } + try { + return await module.execa('node', ['-e', `process.exit(${EXIT_CODE})`]); + } catch (error) { + return error; + } +} + +async function nanoSpawn() { + const module = await optionalImport('nano-spawn'); + if (!module) { + return null; + } + try { + return await module.default('node', ['-e', `process.exit(${EXIT_CODE})`]); + } catch (error) { + return error; + } +} + +const implementations = [ + ['command-stream', commandStream], + ['Node.js exec', nodeChildProcess], + ['Bun shell', bunShell], + ['zx', zx], + ['Execa', execa], + ['nano-spawn', nanoSpawn], +]; + +const describe = (value) => { + const has = (name) => + value?.[name] === undefined ? '-' : String(value[name]); + return `code=${has('code').padEnd(6)} exitCode=${has('exitCode')}`; +}; + +console.log(`failing command: ${FAILING_COMMAND}\n`); + +let failures = 0; +for (const [name, run] of implementations) { + const thrown = await run(); + if (thrown === null) { + console.log(` ${name.padEnd(14)} unavailable`); + continue; + } + console.log(` ${name.padEnd(14)} ${describe(thrown)}`); + + if (name === 'command-stream') { + // command-stream must satisfy both conventions at once (issue #38). + if (thrown.code !== EXIT_CODE || thrown.exitCode !== EXIT_CODE) { + failures += 1; + } + } +} + +process.exitCode = failures === 0 ? 0 : 1; diff --git a/experiments/issue-38-hook-scope/README.md b/experiments/issue-38-hook-scope/README.md new file mode 100644 index 00000000..349164b7 --- /dev/null +++ b/experiments/issue-38-hook-scope/README.md @@ -0,0 +1,35 @@ +# Issue #38: why `js/tests/test-helper.mjs` does not clean up every test file + +`js/tests/test-helper.mjs` calls `beforeEach`/`afterEach` at module scope and +every test file imports it for "automatic" cleanup. ES modules are evaluated +once, so those hooks are registered in the scope of whichever test file Bun +evaluates first; every other file runs with no cleanup hooks at all. + +Run it: + +``` +bun test experiments/issue-38-hook-scope/ +``` + +Output (the winning file depends on the order Bun picks): + +``` +experiments/issue-38-hook-scope/b.test.mjs: +[b] shared beforeEach active for this file: true + +experiments/issue-38-hook-scope/a.test.mjs: +[a] shared beforeEach active for this file: false +``` + +Consequence: global state (the `enableVirtualCommands`/`disableVirtualCommands` +flag, the virtual command registry, shell settings) leaks from one test file to +the next, and whether it leaks depends on an ordering that differs per platform. +That is what made +`error exitCode alias for error code > carries both aliases for a failing pipeline` +fail on macOS only: an earlier file left virtual commands disabled, and with them +disabled `exit 19 | cat` spawns the shell builtin `exit` as a real executable +(see `../issue-38-virtual-disabled-pipeline.mjs`), so the rejection carries +`code: "ENOENT"` instead of `19`. + +`js/tests/error-exitcode-alias.test.mjs` therefore re-enables virtual commands in +its own `beforeEach` instead of trusting the shared helper. diff --git a/experiments/issue-38-hook-scope/a.test.mjs b/experiments/issue-38-hook-scope/a.test.mjs new file mode 100644 index 00000000..904eb07b --- /dev/null +++ b/experiments/issue-38-hook-scope/a.test.mjs @@ -0,0 +1,19 @@ +import { test, expect } from 'bun:test'; +import './helper.mjs'; + +// Two tests are needed to tell whether the shared hook is active for this file: +// the counter can only grow between them if the hook runs for this file's tests. +let runsBeforeSecondTest = null; + +test('a: records how often the shared hook has run', () => { + runsBeforeSecondTest = globalThis.__hookRuns; + expect(typeof runsBeforeSecondTest).toBe('number'); +}); + +test('a: only the file that imported the helper first gets the hook', () => { + const active = globalThis.__hookRuns > runsBeforeSecondTest; + globalThis.__filesWithHook = + (globalThis.__filesWithHook ?? 0) + (active ? 1 : 0); + console.log('[a] shared beforeEach active for this file:', active); + expect(globalThis.__filesWithHook).toBeLessThanOrEqual(1); +}); diff --git a/experiments/issue-38-hook-scope/b.test.mjs b/experiments/issue-38-hook-scope/b.test.mjs new file mode 100644 index 00000000..6cb216f8 --- /dev/null +++ b/experiments/issue-38-hook-scope/b.test.mjs @@ -0,0 +1,19 @@ +import { test, expect } from 'bun:test'; +import './helper.mjs'; + +// Two tests are needed to tell whether the shared hook is active for this file: +// the counter can only grow between them if the hook runs for this file's tests. +let runsBeforeSecondTest = null; + +test('b: records how often the shared hook has run', () => { + runsBeforeSecondTest = globalThis.__hookRuns; + expect(typeof runsBeforeSecondTest).toBe('number'); +}); + +test('b: only the file that imported the helper first gets the hook', () => { + const active = globalThis.__hookRuns > runsBeforeSecondTest; + globalThis.__filesWithHook = + (globalThis.__filesWithHook ?? 0) + (active ? 1 : 0); + console.log('[b] shared beforeEach active for this file:', active); + expect(globalThis.__filesWithHook).toBeLessThanOrEqual(1); +}); diff --git a/experiments/issue-38-hook-scope/helper.mjs b/experiments/issue-38-hook-scope/helper.mjs new file mode 100644 index 00000000..bec6fdf6 --- /dev/null +++ b/experiments/issue-38-hook-scope/helper.mjs @@ -0,0 +1,9 @@ +import { beforeEach } from 'bun:test'; + +// Registering the hook while this module is evaluated binds it to the scope of +// the test file that imported it *first*. ES module caching means the body +// never runs again, so no other file gets the hook. +globalThis.__hookRuns = 0; +beforeEach(() => { + globalThis.__hookRuns += 1; +}); diff --git a/experiments/issue-38-virtual-disabled-pipeline.mjs b/experiments/issue-38-virtual-disabled-pipeline.mjs new file mode 100644 index 00000000..136c6172 --- /dev/null +++ b/experiments/issue-38-virtual-disabled-pipeline.mjs @@ -0,0 +1,37 @@ +#!/usr/bin/env bun +// Issue #38 investigation: why `exit 19 | cat` reports ENOENT instead of 19. +// +// With virtual commands disabled, a parsed pipeline is handed to Bun.spawn one +// command at a time, so the shell builtin `exit` is looked up in $PATH and the +// spawn fails with ENOENT. Test files leak that disabled flag (see +// experiments/issue-38-test-helper-hook-scope.mjs), which is how the CI failure +// on macOS was produced. +import { + $, + shell, + disableVirtualCommands, + enableVirtualCommands, +} from '../js/src/$.mjs'; + +shell.errexit(true); +shell.pipefail(true); + +for (const virtual of [true, false]) { + if (virtual) { + enableVirtualCommands(); + } else { + disableVirtualCommands(); + } + + const error = await $`exit 19 | cat`.catch((thrown) => thrown); + console.log( + `virtualCommands=${virtual ? 'enabled' : 'disabled'} ->`, + JSON.stringify({ + code: error?.code, + exitCode: error?.exitCode, + message: error?.message, + }) + ); +} + +enableVirtualCommands(); diff --git a/js/.changeset/error-exitcode-alias.md b/js/.changeset/error-exitcode-alias.md new file mode 100644 index 00000000..c84b7be5 --- /dev/null +++ b/js/.changeset/error-exitcode-alias.md @@ -0,0 +1,10 @@ +--- +'command-stream': patch +--- + +Expose the exit status of a failing command under both `error.code` and +`error.exitCode`, so handlers written for Node.js `child_process` and for +Execa, zx, nano-spawn or the Bun shell work unchanged. The attached +`error.result` carries both names as well, and a command that could not be +launched at all reports its shell-compatible status (127, 126) through +`error.exitCode` while `error.code` keeps the POSIX errno. diff --git a/js/BEST-PRACTICES.md b/js/BEST-PRACTICES.md index 0e633745..b1601970 100644 --- a/js/BEST-PRACTICES.md +++ b/js/BEST-PRACTICES.md @@ -286,8 +286,10 @@ shell.errexit(true); try { await $`critical-operation`; } catch (error) { - console.error('Critical operation failed:', error); - process.exit(1); + // The status is available under both names: `code` (Node.js + // `child_process`) and `exitCode` (Execa, zx, nano-spawn, Bun shell). + console.error('Critical operation failed with', error.exitCode); + process.exit(error.code); } ``` diff --git a/js/README.md b/js/README.md index 5a5f19e2..10cf5426 100644 --- a/js/README.md +++ b/js/README.md @@ -575,6 +575,10 @@ console.log(result.code); // exit code console.log(result.exitCode); // alias for result.code ``` +Errors thrown in `errexit` mode carry the same pair of names, so handlers +written for Node.js `child_process` (`error.code`) and for Execa, zx, +nano-spawn or the Bun shell (`error.exitCode`) both work unchanged. + ### Custom Options with $({ options }) Syntax (NEW!) ```javascript @@ -1617,6 +1621,8 @@ try { await $`ls nonexistent-file`; // Throws error } catch (error) { console.log('Command failed:', error.code); // → 2 + console.log('Same status:', error.exitCode); // → 2 (alias for error.code) + console.log('Full result:', error.result.exitCode); // → 2 } // ✅ Disable errexit: Back to non-throwing behavior diff --git a/js/docs/COMPETITOR_TEST_AUDIT.md b/js/docs/COMPETITOR_TEST_AUDIT.md index 0532f606..d4b21d51 100644 --- a/js/docs/COMPETITOR_TEST_AUDIT.md +++ b/js/docs/COMPETITOR_TEST_AUDIT.md @@ -105,7 +105,7 @@ summary-level assertion. | `newline-preservation` | Captured output preserves final and repeated newlines. | | `unicode-output` | UTF-8 output is decoded without loss. | | `large-output` | One MiB of output is captured without truncation or deadlock. | -| `nonzero-exit` | Non-zero status is returned through `code` and `exitCode`. | +| `nonzero-exit` | Non-zero status reads through `code` and `exitCode`, on results and on errors. | | `result-text` | `text()` returns captured stdout. | | `stdin-string` | String input is written completely and stdin is closed. | | `stdin-buffer` | Buffer input is written without textual coercion. | diff --git a/js/examples/error-exitcode-alias.mjs b/js/examples/error-exitcode-alias.mjs new file mode 100644 index 00000000..9bf3daf6 --- /dev/null +++ b/js/examples/error-exitcode-alias.mjs @@ -0,0 +1,44 @@ +#!/usr/bin/env node + +/** + * Handling a failed command through either property name. + * + * With `shell.errexit(true)` a non-zero exit throws, and the thrown error + * carries the status under both names: `code` (Node.js `child_process`) and + * `exitCode` (Execa, zx, nano-spawn, Bun shell). Code written for either + * convention works unchanged (issue #38). + * + * Run: node js/examples/error-exitcode-alias.mjs + */ + +import { $ as $raw, shell } from '../src/$.mjs'; + +// Keep the example output tidy: capture instead of mirroring child output. +const $ = $raw({ mirror: false }); + +shell.errexit(true); + +// Node.js style: read the status from `error.code`. +try { + await $`exit 3`; +} catch (error) { + console.log(`node style -> error.code = ${error.code}`); +} + +// Execa/zx style: read the very same status from `error.exitCode`. +try { + await $`node -e "process.exit(42)"`; +} catch (error) { + console.log(`execa style -> error.exitCode = ${error.exitCode}`); + console.log( + `result alias -> error.result.exitCode = ${error.result.exitCode}` + ); +} + +// Without errexit a failing command resolves, and the result carries both +// names as well. +shell.errexit(false); +const result = await $`exit 7`; +console.log( + `result -> code = ${result.code}, exitCode = ${result.exitCode}` +); diff --git a/js/src/$.process-runner-execution.mjs b/js/src/$.process-runner-execution.mjs index 1cda85b0..bc537887 100644 --- a/js/src/$.process-runner-execution.mjs +++ b/js/src/$.process-runner-execution.mjs @@ -11,7 +11,9 @@ import { import { StreamUtils, safeWrite, asBuffer } from './$.stream-utils.mjs'; import { pumpReadable } from './$.quote.mjs'; import { + attachExitCodeAlias, createCancelledResult, + createCommandError, createExecutionErrorResult, createResult, finishExecutionError, @@ -505,15 +507,15 @@ function throwErrexitIfNeeded(runner, globalShellSettings) { trace('ProcessRunner', () => `Errexit mode: throwing error`); - const error = new Error( - `Command failed with exit code ${runner.result.code}` + throw createCommandError( + `Command failed with exit code ${runner.result.code}`, + { + code: runner.result.code, + stdout: runner.result.stdout, + stderr: runner.result.stderr, + result: runner.result, + } ); - error.code = runner.result.code; - error.stdout = runner.result.stdout; - error.stderr = runner.result.stderr; - error.result = runner.result; - - throw error; } /** @@ -624,12 +626,12 @@ function processSyncResult(runner, result, globalShellSettings) { runner.finish(result); if (globalShellSettings.errexit && result.code !== 0) { - const error = new Error(`Command failed with exit code ${result.code}`); - error.code = result.code; - error.stdout = result.stdout; - error.stderr = result.stderr; - error.result = result; - throw error; + throw createCommandError(`Command failed with exit code ${result.code}`, { + code: result.code, + stdout: result.stdout, + stderr: result.stderr, + result, + }); } return result; @@ -1206,7 +1208,12 @@ export function attachExecutionMethods(ProcessRunner, deps) { })}` ); - finishExecutionError(this, error); + const errorResult = finishExecutionError(this, error); + + // Rejections escaping here include failures to launch a process, whose + // `code` stays the POSIX errno string. `exitCode` always reports the + // shell-compatible status the result carries (issue #38). + attachExitCodeAlias(error, errorResult?.code); // Match the library's default shell-like error contract for failures to // launch a direct executable. `errexit` continues to opt into rejection, diff --git a/js/src/$.process-runner-pipeline.mjs b/js/src/$.process-runner-pipeline.mjs index 102fcc19..d36ef818 100644 --- a/js/src/$.process-runner-pipeline.mjs +++ b/js/src/$.process-runner-pipeline.mjs @@ -5,7 +5,7 @@ import cp from 'child_process'; import { trace } from './$.trace.mjs'; import { findAvailableShell, withExportedProcessContext } from './$.shell.mjs'; import { StreamUtils, safeWrite } from './$.stream-utils.mjs'; -import { createResult } from './$.result.mjs'; +import { createCommandError, createResult } from './$.result.mjs'; import { applyVirtualProcessContext, effectiveCwd, @@ -188,11 +188,10 @@ function checkPipefail(exitCodes, shellSettings) { if (shellSettings.pipefail) { const failedIndex = exitCodes.findIndex((code) => code !== 0); if (failedIndex !== -1) { - const error = new Error( - `Pipeline command at index ${failedIndex} failed with exit code ${exitCodes[failedIndex]}` + throw createCommandError( + `Pipeline command at index ${failedIndex} failed with exit code ${exitCodes[failedIndex]}`, + { code: exitCodes[failedIndex] } ); - error.code = exitCodes[failedIndex]; - throw error; } } } @@ -204,12 +203,12 @@ function checkPipefail(exitCodes, shellSettings) { */ function throwErrexitError(result, shellSettings) { if (shellSettings.errexit && result.code !== 0) { - const error = new Error(`Pipeline failed with exit code ${result.code}`); - error.code = result.code; - error.stdout = result.stdout; - error.stderr = result.stderr; - error.result = result; - throw error; + throw createCommandError(`Pipeline failed with exit code ${result.code}`, { + code: result.code, + stdout: result.stdout, + stderr: result.stderr, + result, + }); } } @@ -655,12 +654,10 @@ async function handleVirtualPipelineCommand( } if (globalShellSettings.errexit && result.code !== 0) { - const error = new Error( - `Pipeline command failed with exit code ${result.code}` + throw createCommandError( + `Pipeline command failed with exit code ${result.code}`, + { code: result.code, result } ); - error.code = result.code; - error.result = result; - throw error; } return { input: result.stdout }; @@ -700,11 +697,10 @@ async function handleShellPipelineCommand( }; if (globalShellSettings.pipefail && result.code !== 0) { - const error = new Error( - `Pipeline command '${commandStr}' failed with exit code ${result.code}` + throw createCommandError( + `Pipeline command '${commandStr}' failed with exit code ${result.code}`, + { code: result.code } ); - error.code = result.code; - throw error; } if (isLastCommand) { diff --git a/js/src/$.process-runner-virtual.mjs b/js/src/$.process-runner-virtual.mjs index bd22ca22..74247063 100644 --- a/js/src/$.process-runner-virtual.mjs +++ b/js/src/$.process-runner-virtual.mjs @@ -8,6 +8,12 @@ import { effectiveCwd, effectiveEnv, } from './$.process-context.mjs'; +import { + attachExitCodeAlias, + createCommandError, + createResult, + executionErrorExitCode, +} from './$.result.mjs'; /** * Get stdin data from options @@ -83,17 +89,21 @@ function emitOutput(runner, type, data) { * @returns {object} Result object */ function handleVirtualError(runner, error, shellSettings, shouldFinish) { - let exitCode = error.code ?? 1; + // Handlers may throw system errors whose `code` is a POSIX errno string, so + // normalize the status to a number before reporting it. + let exitCode = executionErrorExitCode(error); if (runner._cancelled && runner._cancellationSignal) { exitCode = getCancellationExitCode(runner._cancellationSignal); } - const result = { + // Built through createResult so the nested `error.result` carries the + // `exitCode` alias as well (issues #36 and #38). + const result = createResult({ code: exitCode, stdout: error.stdout ?? '', stderr: error.stderr ?? error.message, stdin: '', - }; + }); emitOutput(runner, 'stderr', result.stderr); if (shouldFinish) { @@ -102,6 +112,8 @@ function handleVirtualError(runner, error, shellSettings, shouldFinish) { if (shellSettings.errexit) { error.result = result; + // `exitCode` is an alias for `code` for better compatibility (issue #38) + attachExitCodeAlias(error, exitCode); throw error; } @@ -295,12 +307,15 @@ export function attachVirtualCommandMethods(ProcessRunner, deps) { } if (globalShellSettings.errexit && result.code !== 0) { - const error = new Error(`Command failed with exit code ${result.code}`); - error.code = result.code; - error.stdout = result.stdout; - error.stderr = result.stderr; - error.result = result; - throw error; + throw createCommandError( + `Command failed with exit code ${result.code}`, + { + code: result.code, + stdout: result.stdout, + stderr: result.stderr, + result, + } + ); } return result; diff --git a/js/src/$.result.mjs b/js/src/$.result.mjs index fe696321..24f4d762 100644 --- a/js/src/$.result.mjs +++ b/js/src/$.result.mjs @@ -24,6 +24,57 @@ export function createResult({ code, stdout = '', stderr = '', stdin = '' }) { }; } +/** + * Create an Error describing a command that exited with a failing status. + * + * The status is exposed under both `code` (command-stream's original name) and + * `exitCode` (the name used by Node.js `child_process`, execa, zx, nano-spawn + * and Bun Shell), so either error-handling style works (issue #38). + * + * @param {string} message - Error message + * @param {object} params - Error parameters + * @param {number} params.code - Exit code of the failed command + * @param {string} [params.stdout] - Captured stdout + * @param {string} [params.stderr] - Captured stderr + * @param {object} [params.result] - Full result object of the failed command + * @returns {Error & {code: number, exitCode: number}} Command failure error + */ +export function createCommandError(message, { code, stdout, stderr, result }) { + const error = new Error(message); + error.code = code; + // `exitCode` is an alias for `code` for better compatibility (issue #38) + error.exitCode = code; + if (stdout !== undefined) { + error.stdout = stdout; + } + if (stderr !== undefined) { + error.stderr = stderr; + } + if (result !== undefined) { + error.result = result; + } + return error; +} + +/** + * Expose the numeric exit status of a rejected command as `exitCode`. + * + * Failures that never reached a running process keep the POSIX errno string in + * `code` (`ENOENT`, `EACCES`, ...) because that is what Node.js reports, so the + * shell-compatible status is taken from the result the runner already built + * (issue #38). + * + * @param {Error & {code?: string|number, exitCode?: number}} error - Thrown error + * @param {number} code - Numeric exit status to expose + * @returns {Error} The same error + */ +export function attachExitCodeAlias(error, code) { + if (error && typeof error === 'object' && error.exitCode === undefined) { + error.exitCode = code; + } + return error; +} + export function createCancelledResult(signal) { const signalCodes = { SIGINT: 130, SIGKILL: 137, SIGTERM: 143 }; return createResult({ diff --git a/js/src/commands/$.exit.mjs b/js/src/commands/$.exit.mjs index 7e6007c6..bfedc1e4 100644 --- a/js/src/commands/$.exit.mjs +++ b/js/src/commands/$.exit.mjs @@ -1,8 +1,12 @@ +import { createCommandError } from '../$.result.mjs'; + export default function createExitCommand(globalShellSettings) { return async function exit({ args }) { const code = parseInt(args[0] || 0); if (globalShellSettings.errexit || code !== 0) { - throw { code, message: `Command failed with exit code ${code}` }; + throw createCommandError(`Command failed with exit code ${code}`, { + code, + }); } return { stdout: '', code }; }; diff --git a/js/tests/competitor-compatibility.test.mjs b/js/tests/competitor-compatibility.test.mjs index b8ddae17..069543de 100644 --- a/js/tests/competitor-compatibility.test.mjs +++ b/js/tests/competitor-compatibility.test.mjs @@ -11,7 +11,7 @@ import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import './test-helper.mjs'; -import { $, exec, ProcessRunner } from '../src/$.mjs'; +import { $, exec, ProcessRunner, shell } from '../src/$.mjs'; import { competitors, excludedTestClasses, @@ -505,6 +505,24 @@ describe('ported public process behavior', () => { expect(result.code).toBe(42); expect(result.exitCode).toBe(42); + + // Execa, zx, nano-spawn and the Bun shell reject a failing command with + // an error that names the status `exitCode`, while Node.js names it + // `code`. In errexit mode command-stream answers to both (issue #38). + shell.errexit(true); + try { + const error = await runFixture('exit', ['42']).catch( + (thrown) => thrown + ); + + expect(error).toBeInstanceOf(Error); + expect(error.code).toBe(42); + expect(error.exitCode).toBe(42); + expect(error.result.code).toBe(42); + expect(error.result.exitCode).toBe(42); + } finally { + shell.errexit(false); + } } ); diff --git a/js/tests/error-exitcode-alias.test.mjs b/js/tests/error-exitcode-alias.test.mjs new file mode 100644 index 00000000..d397329c --- /dev/null +++ b/js/tests/error-exitcode-alias.test.mjs @@ -0,0 +1,126 @@ +import { test, expect, describe, afterEach, beforeEach } from 'bun:test'; +import './test-helper.mjs'; // Automatically sets up beforeEach/afterEach cleanup +import { $, exec, shell, enableVirtualCommands } from '../src/$.mjs'; + +// Errors thrown by failing commands expose the exit status under both `code` +// (Node.js `child_process` naming) and `exitCode` (execa, zx, nano-spawn and +// Bun Shell naming), so handlers written for either convention work (issue #38). +describe('error exitCode alias for error code', () => { + beforeEach(() => { + // Other test files disable the virtual commands and never restore them: + // `test-helper.mjs` registers its cleanup hooks while it is evaluated, so + // they belong to the first test file that imports it and no other file is + // reset (see experiments/issue-38-hook-scope/). Whether the leak reaches + // this file depends on the order Bun picks, which differs per platform, so + // the built-in `exit` used below is re-enabled explicitly. + enableVirtualCommands(); + shell.errexit(false); + shell.verbose(false); + shell.xtrace(false); + shell.pipefail(false); + shell.nounset(false); + }); + + // The same missing cleanup would let this file's `errexit`/`pipefail` escape + // into whichever file runs next, so they are restored here. + afterEach(() => { + shell.errexit(false); + shell.pipefail(false); + }); + + test('throws an Error carrying both aliases in errexit mode', async () => { + shell.errexit(true); + + const error = await $`exit 42`.catch((thrown) => thrown); + + expect(error).toBeInstanceOf(Error); + expect(error.message).toContain('42'); + expect(error.code).toBe(42); + expect(error.exitCode).toBe(42); + expect(error.exitCode).toBe(error.code); + }); + + test('keeps both aliases on the attached result', async () => { + shell.errexit(true); + + const error = await $`exit 7`.catch((thrown) => thrown); + + expect(error.result.code).toBe(7); + expect(error.result.exitCode).toBe(7); + }); + + test('carries both aliases for every exit status', async () => { + shell.errexit(true); + + for (const code of [1, 2, 127, 255]) { + const error = await $`exit ${code}`.catch((thrown) => thrown); + + expect(error.code).toBe(code); + expect(error.exitCode).toBe(code); + } + }); + + test('carries both aliases for a failing external command', async () => { + shell.errexit(true); + + const error = await $`node -e "process.exit(17)"`.catch((thrown) => thrown); + + expect(error).toBeInstanceOf(Error); + expect(error.code).toBe(17); + expect(error.exitCode).toBe(17); + }); + + test('carries both aliases for a failing pipeline', async () => { + shell.errexit(true); + shell.pipefail(true); + + const error = await $`exit 19 | cat`.catch((thrown) => thrown); + + expect(error).toBeInstanceOf(Error); + expect(error.code).toBe(19); + expect(error.exitCode).toBe(19); + }); + + test('carries both aliases on a failing .pipe() result', async () => { + shell.errexit(true); + + const result = await $`echo hello`.pipe($`node -e "process.exit(23)"`); + + expect(result.code).toBe(23); + expect(result.exitCode).toBe(23); + }); + + test('carries both aliases for a missing executable', async () => { + shell.errexit(true); + + const error = await $`command-stream-missing-binary-38`.catch( + (thrown) => thrown + ); + + expect(error).toBeInstanceOf(Error); + expect(typeof error.code).toBe('number'); + expect(error.exitCode).toBe(error.code); + }); + + test('reports a numeric exitCode when the executable cannot be launched', async () => { + shell.errexit(true); + + const error = await exec('command-stream-missing-binary-38', [], { + capture: true, + mirror: false, + }).catch((thrown) => thrown); + + expect(error).toBeInstanceOf(Error); + // A process that never started has no exit status of its own, so Node + // reports the POSIX errno in `code`. `exitCode` still answers with the + // shell-compatible status the result carries. + expect(error.exitCode).toBe(127); + }); + + test('leaves the non-errexit result path unchanged', async () => { + const result = await $`exit 3`; + + expect(result.code).toBe(3); + expect(result.exitCode).toBe(3); + }); +}); diff --git a/rust/README.md b/rust/README.md index 005461fe..bf93e912 100644 --- a/rust/README.md +++ b/rust/README.md @@ -33,6 +33,14 @@ async fn main() { // `exit_code()` is an alias for the `code` field, mirroring the // JavaScript `exitCode` alias. assert_eq!(result.exit_code(), result.code); + + // `error_for_status()` turns a failing result into an error, whose status + // reads through the same pair of names. + let error = CommandResult::error_with_code("boom", 2) + .error_for_status() + .unwrap_err(); + assert_eq!(error.code(), Some(2)); + assert_eq!(error.exit_code(), error.code()); } ``` diff --git a/rust/changelog.d/20260915_000000_error_exit_code_alias.md b/rust/changelog.d/20260915_000000_error_exit_code_alias.md new file mode 100644 index 00000000..7c6fad48 --- /dev/null +++ b/rust/changelog.d/20260915_000000_error_exit_code_alias.md @@ -0,0 +1,9 @@ +--- +bump: patch +--- + +### Added + +- `Error::code()` and its `Error::exit_code()` alias report the exit status of a + failed command, and `CommandResult::error_for_status()` turns a non-zero + result into that error. diff --git a/rust/docs/COMPETITOR_TEST_AUDIT.md b/rust/docs/COMPETITOR_TEST_AUDIT.md index 6df7753d..9487218c 100644 --- a/rust/docs/COMPETITOR_TEST_AUDIT.md +++ b/rust/docs/COMPETITOR_TEST_AUDIT.md @@ -106,7 +106,7 @@ inventory total, and ensure all fourteen selected projects are represented. | `newline-preservation` | Captured output preserves final and repeated newlines. | | `unicode-output` | UTF-8 output is captured without loss. | | `large-output` | One MiB is captured without truncation or deadlock. | -| `nonzero-exit` | A non-zero child status is returned without discarding output. | +| `nonzero-exit` | A non-zero child status is returned, and reads through `code` and `exit_code`. | | `stdin-string` | String input is written completely and stdin closes. | | `lazy-execution` | Constructing a runner does not spawn it. | | `concurrent-execution` | Concurrent children keep results isolated. | diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 760c7fbf..a0fe9d49 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -220,6 +220,47 @@ pub enum Error { Cancelled, } +impl Error { + /// Build a [`Error::CommandFailed`] for a command that exited with `code`. + pub fn command_failed(code: i32, message: impl Into) -> Self { + Error::CommandFailed { + code, + message: message.into(), + } + } + + /// Exit status carried by the error, when the failure has one. + /// + /// Mirrors the `error.code` property of the JavaScript implementation + /// (issue #38). Failures that never reached a child process, such as parse + /// errors, report `None`. + pub fn code(&self) -> Option { + match self { + Error::CommandFailed { code, .. } => Some(*code), + // `command not found` is 127 in POSIX shells, which is also what + // the JavaScript implementation reports for a missing executable. + Error::CommandNotFound(_) => Some(127), + Error::Io(error) => match error.kind() { + std::io::ErrorKind::NotFound => Some(127), + std::io::ErrorKind::PermissionDenied => Some(126), + _ => None, + }, + // A cancelled command is terminated with SIGINT (128 + 2). + Error::Cancelled => Some(130), + Error::ParseError(_) => None, + } + } + + /// Alias for [`code`](Self::code). + /// + /// Node.js `child_process` names this property `code`, while execa, zx, + /// nano-spawn, and Bun Shell name it `exitCode`. command-stream exposes + /// both spellings in every language (issue #38). + pub fn exit_code(&self) -> Option { + self.code() + } +} + /// Result type for command-stream operations pub type Result = std::result::Result; diff --git a/rust/src/utils.rs b/rust/src/utils.rs index 144463c3..dc4f4149 100644 --- a/rust/src/utils.rs +++ b/rust/src/utils.rs @@ -139,6 +139,33 @@ impl CommandResult { pub fn exit_code(&self) -> i32 { self.code } + + /// Turn a failing result into [`crate::Error::CommandFailed`]. + /// + /// This is the Rust counterpart of the JavaScript `errexit` mode: a + /// non-zero status becomes an error whose exit status is readable through + /// both [`crate::Error::code`] and [`crate::Error::exit_code`] (issue + /// #38). Successful results pass through unchanged. + /// + /// ``` + /// use command_stream::utils::CommandResult; + /// + /// let error = CommandResult::error_with_code("", 42) + /// .error_for_status() + /// .unwrap_err(); + /// assert_eq!(error.code(), Some(42)); + /// assert_eq!(error.exit_code(), error.code()); + /// ``` + pub fn error_for_status(self) -> crate::Result { + if self.is_success() { + return Ok(self); + } + + Err(crate::Error::command_failed( + self.code, + format!("Command failed with exit code {}", self.code), + )) + } } /// Utility functions for virtual commands diff --git a/rust/tests/competitor_compatibility/behavior.rs b/rust/tests/competitor_compatibility/behavior.rs index e3c94027..c2c78d80 100644 --- a/rust/tests/competitor_compatibility/behavior.rs +++ b/rust/tests/competitor_compatibility/behavior.rs @@ -175,7 +175,15 @@ async fn nonzero_exit_is_returned_as_a_result() { let result = run_fixture("exit", &["23"]).await; assert_eq!(result.code, 23); + assert_eq!(result.exit_code(), 23); assert!(!result.is_success()); + + // Execa, zx, nano-spawn and the Bun shell turn a failing command into an + // error that names the status `exitCode`, while Node.js names it `code`. + // Both spellings read the same status here (issue #38). + let error = result.error_for_status().unwrap_err(); + assert_eq!(error.code(), Some(23)); + assert_eq!(error.exit_code(), error.code()); } #[tokio::test] diff --git a/rust/tests/utils.rs b/rust/tests/utils.rs index 9ba4455c..d39791b0 100644 --- a/rust/tests/utils.rs +++ b/rust/tests/utils.rs @@ -3,6 +3,7 @@ //! These tests mirror the JavaScript utility tests use command_stream::utils::{quote, AnsiConfig, AnsiUtils, CommandResult, VirtualUtils}; +use command_stream::Error; use std::path::PathBuf; // ============================================================================ @@ -55,6 +56,64 @@ fn test_command_result_exit_code_alias() { assert_eq!(failure.exit_code(), failure.code); } +// ============================================================================ +// Error Exit Code Tests +// ============================================================================ + +#[test] +fn test_error_code_and_exit_code_alias() { + // Node.js names the property `code`, execa/zx/nano-spawn/Bun Shell name it + // `exitCode`; command-stream answers to both spellings (issue #38) + let error = Error::command_failed(42, "Command failed with exit code 42"); + assert_eq!(error.code(), Some(42)); + assert_eq!(error.exit_code(), Some(42)); + assert_eq!(error.exit_code(), error.code()); +} + +#[test] +fn test_error_code_for_missing_and_cancelled_commands() { + // `command not found` is 127 and an interrupted command is 128 + SIGINT, + // matching the statuses reported by POSIX shells and the JS implementation + assert_eq!(Error::CommandNotFound("nope".into()).code(), Some(127)); + assert_eq!(Error::Cancelled.code(), Some(130)); + + let missing = Error::Io(std::io::Error::new(std::io::ErrorKind::NotFound, "missing")); + assert_eq!(missing.code(), Some(127)); + assert_eq!(missing.exit_code(), missing.code()); + + let denied = Error::Io(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "denied", + )); + assert_eq!(denied.code(), Some(126)); +} + +#[test] +fn test_error_without_exit_status_reports_none() { + // Failures that never reached a child process have no exit status + let error = Error::ParseError("unbalanced quote".into()); + assert_eq!(error.code(), None); + assert_eq!(error.exit_code(), None); +} + +#[test] +fn test_error_for_status_keeps_successful_results() { + let result = CommandResult::success("hello").error_for_status().unwrap(); + assert_eq!(result.stdout, "hello"); + assert_eq!(result.code, 0); +} + +#[test] +fn test_error_for_status_turns_failures_into_errors() { + let error = CommandResult::error_with_code("boom", 23) + .error_for_status() + .unwrap_err(); + + assert_eq!(error.code(), Some(23)); + assert_eq!(error.exit_code(), Some(23)); + assert!(error.to_string().contains("23")); +} + // ============================================================================ // VirtualUtils Tests // ============================================================================