Skip to content

Report a failed command's status through both code and exitCode - #110

Merged
konard merged 10 commits into
mainfrom
issue-38-8ff5f784
Sep 15, 2026
Merged

konard merged 10 commits into
mainfrom
issue-38-8ff5f784

Conversation

@konard

@konard konard commented Sep 9, 2025

Copy link
Copy Markdown
Member

Fixes #38 with Option 1 (backward-compatible alias): a failing command now
reports its status under both code (Node.js child_process naming) and
exitCode (Execa, zx, nano-spawn, Bun shell naming), in both languages, as
requested in review.

How to reproduce

import { $, shell } from 'command-stream';

shell.errexit(true);
try {
  await $`node -e "process.exit(23)"`;
} catch (error) {
  console.log(error.code); // 23 (already worked)
  console.log(error.exitCode); // undefined before this PR
}

What changed

JavaScript

  • js/src/$.result.mjs: new createCommandError(message, { code, stdout, stderr, result })
    factory — the single place that sets code and exitCode on a failure,
    mirroring how createResult handles the result-level alias from Add exitCode alias for code #36.
  • Every errexit/pipefail throw site now goes through it:
    $.process-runner-execution.mjs (async and sync), $.process-runner-pipeline.mjs
    (4 sites), $.process-runner-virtual.mjs, commands/$.exit.mjs.
  • handleVirtualError builds its result through createResult, so the attached
    error.result.exitCode is defined too; it also normalizes a POSIX errno
    code (ENOENT, EACCES, …) into a numeric status instead of leaking the
    string into result.code.
  • attachExitCodeAlias(error, code) covers the failures that never reached a
    running process. Node.js keeps the POSIX errno in code there (ENOENT,
    EACCES, …) and the library relies on that string to recognise a launch
    failure, so code is left alone and exitCode reports the shell-compatible
    status the result already carries (127 for a missing executable, 126 for a
    permission error).

Rust

  • Error::code() reports the status of a failure (CommandFailed, 127 for a
    missing command, 126 for a permission error, 130 for a cancelled command,
    None for a parse error) and Error::exit_code() is its alias.
  • CommandResult::error_for_status() turns a non-zero result into that error —
    the Rust counterpart of errexit.

Competitor verification

experiments/issue-38-error-exitcode-competitors.mjs probes the installed
packages with the same failing command and prints what each one exposes:

failing command: node -e "process.exit(23)"

  command-stream code=23     exitCode=23
  Node.js exec   code=23     exitCode=-
  Bun shell      code=-      exitCode=23
  zx             code=-      exitCode=23
  Execa          code=-      exitCode=23
  nano-spawn     code=-      exitCode=23

Both spellings are in real use, which is why command-stream answers to both.
The ported competitor case [nonzero-exit] is extended accordingly in both
languages (js/tests/competitor-compatibility.test.mjs,
rust/tests/competitor_compatibility/behavior.rs), so the corpus now covers the
failure path, not just the result path.

Tests

  • js/tests/error-exitcode-alias.test.mjs (new, 9 cases): errexit error,
    attached result, statuses 1/2/127/255, failing external command, failing
    pipeline with pipefail, failing .pipe() result, missing executable, a
    binary that cannot be launched at all, and the unchanged non-errexit path.
  • rust/tests/utils.rs: 5 cases for Error::code()/exit_code() and
    error_for_status(), next to the existing Add exitCode alias for code #36 alias test.
  • js/examples/error-exitcode-alias.mjs: runnable demonstration of both styles.

The macOS-only CI failure this PR had to chase

carries both aliases for a failing pipeline failed on bun on macos-latest
only, reporting code: "ENOENT" instead of 19. Root cause, reproduced in
./experiments:

  • js/tests/test-helper.mjs registers its beforeEach/afterEach while the
    module is evaluated. ES modules are evaluated once, so the hooks belong to the
    first test file that imports the helper and no other file is reset
    (experiments/issue-38-hook-scope/). Test files that call
    disableVirtualCommands() in their own hooks therefore leak that flag into
    whatever Bun runs next, and the file order differs per platform.
  • With virtual commands disabled, a parsed pipeline is spawned command by
    command, so the shell builtin exit is looked up in $PATH and Bun reports
    ENOENT/127 instead of the pipeline status
    (experiments/issue-38-virtual-disabled-pipeline.mjs prints both outcomes).

The test now enables virtual commands in its own beforeEach and restores the
shell settings it changes, so it neither depends on nor adds to the leak.

Both underlying defects are pre-existing and out of scope here, but worth their
own issues:

  1. js/tests/test-helper.mjs only cleans up one test file, so the whole suite
    runs with per-file state leaking in a platform-dependent order.
  2. With virtual commands disabled, the pipeline path spawns shell builtins as
    real executables (exit, and the same would apply to cd, export, …)
    instead of handling them as builtins.

Local runs: bun test js/tests/ → 1370 pass, 11 skip, 28 fail — all 28 are
jq: not found in this sandbox and unrelated to this change; cargo test
all suites pass, including the new doctest; eslint, prettier and cargo fmt/
clippy -D warnings are clean.

Release

  • js/.changeset/error-exitcode-alias.md (patch)
  • rust/changelog.d/20260915_000000_error_exit_code_alias.md (patch)

Adding CLAUDE.md with task information for AI processing.
This file will be removed when the task is complete.

Issue: #38
@konard konard self-assigned this Sep 9, 2025
konard and others added 2 commits September 9, 2025 20:58
This change addresses issue #38 by adding error.exitCode as an alias for
error.code to maintain compatibility with Node.js standard error handling
patterns while preserving backward compatibility.

Changes:
- Add error.exitCode property alongside error.code in all error creation locations
- Fix $.exit.mjs virtual command to throw proper Error objects instead of plain objects
- Add comprehensive tests for exitCode compatibility
- Add example script demonstrating both old and new error handling patterns

Both error.code and error.exitCode now contain the same exit code value,
allowing developers to use either the traditional command-stream pattern
or the standard Node.js pattern.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@konard konard changed the title [WIP] The library uses error.code instead of error.exitCode Add exitCode property as alias for code in error objects Sep 9, 2025
@konard
konard marked this pull request as ready for review September 9, 2025 18:05
@konard

konard commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

I think we should support both error.code and error.exitCode, and in all programming languages, double check our competitors and take there tests also to us.

@konard
konard marked this pull request as draft September 15, 2026 11:57
@konard

konard commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

🤖 AI Work Session Started

Starting automated work session at 2026-09-15T11:57:32.551Z

The PR has been converted to draft mode while work is in progress.

This comment marks the beginning of an AI work session. Please wait for the session to finish, and provide your feedback.

Runtime: solve v2.29.0 · tool claude · model opus · task image konard/hive-mind-dind:2.29.0@sha256:11a236d03854cbaea86fdd7ca06265c912cf0b67d1fcc9b3eb60bd84c43ff749

Resolve conflicts caused by the js/ + rust/ repository restructure:
- drop the stale src/$.mjs copy (source now lives in js/src/)
- keep the issue #38 files under js/ where the tree was renamed
Failing commands now throw errors that carry the exit status under both
`code` (Node.js child_process naming) and `exitCode` (execa, zx,
nano-spawn and Bun Shell naming). A shared createCommandError factory is
the single place where both names are set, and virtual command errors
build their attached result through createResult so error.result.exitCode
is defined as well.

Rust gains the matching accessors Error::code()/Error::exit_code() and
CommandResult::error_for_status().
Extend the ported [nonzero-exit] competitor case in both languages to
assert the failure path as well: Execa, zx, nano-spawn and the Bun shell
name the status exitCode while Node.js names it code, and command-stream
now answers to both. Adds Rust unit tests for Error::code()/exit_code()
and CommandResult::error_for_status(), a competitor probe experiment and
a runnable example.
Both READMEs, the JS best practices guide and the competitor audits now
state that a failed command reports its status under code and exitCode
(exit_code in Rust). Adds the changeset and the Rust changelog fragment
that release the change.
@konard konard changed the title Add exitCode property as alias for code in error objects Report a failed command's status through both code and exitCode Sep 15, 2026
Errors escaping the runner in errexit mode now always carry the
shell-compatible status in `exitCode`, including spawn failures whose
`code` stays the POSIX errno string (issue #38).

Also adds temporary diagnostics to the pipeline alias test to identify a
macOS-only CI failure where the virtual `exit` command is spawned as a
real executable.
The macOS run of `bun test js/tests/` failed `carries both aliases for a
failing pipeline` with `code: "ENOENT"` instead of 19. The shared
`tests/test-helper.mjs` registers its cleanup hooks while the module is
evaluated, so they belong to the first test file that imports it and no other
file is reset; files that call `disableVirtualCommands()` in their own hooks
leak that flag into whatever runs next, and the order differs per platform.

With virtual commands disabled the parsed pipeline is spawned command by
command, so the shell builtin `exit` is looked up in $PATH and Bun reports
ENOENT. The test now enables virtual commands itself and restores the shell
settings it changes, and both effects are reproduced in ./experiments.
@konard
konard marked this pull request as ready for review September 15, 2026 12:39
@konard

konard commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Working session summary

Done — PR #110 is green and out of draft: #110

CI on 40af9c9 (all five workflows, no non-passing runs to investigate):

Workflow Result
JavaScript checks and release success
Rust checks and release success
Security success
Language parity check success
Repository quality checks success

Check rollup on the PR: 28 SUCCESS, 6 SKIPPED (release jobs), 0 failures.

Local verification

  • bun test js/tests/error-exitcode-alias.test.mjs → 9 pass, 0 fail.
Rest of the working session summary (1 KB)
- `bun test js/tests/` → 1372 pass, 11 skip, 28 fail; all 28 failures are `jq: not found` in this sandbox (jq is unavailable here — `apt-get` can't locate the package), unrelated to this change.
- `cargo test` → every suite ok, including the `error_for_status` doctest.

**PR description** now also documents the macOS-only failure this branch had to chase: `js/tests/test-helper.mjs` registers its `beforeEach`/`afterEach` at module scope, so ES-module caching binds them to the first importing test file and no other file gets reset; files that call `disableVirtualCommands()` leak that flag in a platform-dependent order, and with virtual commands off the pipeline path spawns the `exit` builtin as a real executable (`ENOENT`/127). Both effects are reproduced in `experiments/issue-38-hook-scope/` and `experiments/issue-38-virtual-disabled-pipeline.mjs`. The test was made self-sufficient rather than patching the helper — both underlying defects are pre-existing and called out in the PR body as worth separate issues.

This summary was automatically extracted from the AI working session output.

@konard

konard commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost: $20.744002

📊 Context and tokens usage:

Claude Opus 5: (5 sub-sessions)

  1. 116.1K / 1M (12%) input tokens, 33.0K / 128K (26%) output tokens
  2. 116.6K / 1M (12%) input tokens, 26.7K / 128K (21%) output tokens
  3. 115.5K / 1M (12%) input tokens, 33.4K / 128K (26%) output tokens
  4. 116.9K / 1M (12%) input tokens, 40.6K / 128K (32%) output tokens
  5. 47.3K / 1M (5%) input tokens, 4.1K / 128K (3%) output tokens

Total: (11.3K new + 452.1K cache writes + 23.6M cache reads) input tokens, 175.4K output tokens, $20.744003 cost

🤖 Models used:

  • Tool: Anthropic Claude Code
  • Requested: opus (claude-opus-5)
  • Thinking level: high (~23999 tokens)
  • Model: Claude Opus 5 (claude-opus-5)

📎 Log file uploaded as Gist (7784KB)


Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard
konard merged commit 5fda28e into main Sep 15, 2026
34 checks passed
@konard

konard commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

🎉 Auto-merged

This pull request has been automatically merged by hive-mind.

  • All CI checks have passed

Auto-merged by hive-mind with --auto-merge flag

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The library uses error.code instead of error.exitCode

1 participant