shell: exit 1 on builtin failure instead of raw errno; fix cd ENOENT message
Problem
- A failing builtin reports the raw errno as its exit code.
echo hi > /dev/fullandwhich sh > /dev/fullexit 65508 (ShellError: Failed with exit code 65508).mv nosuchsrc dstexits 2. echo.rs:113andwhich.rs:182cast the negatedSystemError.errnoto theu16exit code, so ENOSPC (-28) wraps.mv.rs:332,cat.rs,rm.rs,CondExpr.rsand theCmd.rsoutput relay use the positive errno.cd /nonexistentprintscd: not a directory: /nonexistent(cd.rs:76).
Fix
- Each of these sites exits 1, as
pwd,export,basenameand the other builtins already do. The errno text still goes to stderr.rm -valso exits 1 when its verbose output is lost (it exited 0). - bash, dash, zsh and coreutils
mv,catandrmall exit 1 here. Bun Shell is bash-like and its builtins stand in for those programs, so$?means the same thing. An errno is not a portable exit code: it differs by platform, and values above 125 collide with the shell's 126, 127 and 128+n. Cmd.rs: a lost relay of captured subprocess output records exit 1 (it recorded the errno), and #41436'son_exitkeeps it whichever event lands first. A sole command substitution's status no longer hides the named command's:$(echo "sh -c false")exits 1,$(echo env; exit 5)exits 0, as in bash.cdprintscd: {path}: No such file or directoryorcd: {path}: Not a directory, like bash (folds in #36336).- Verified:
test/js/bun/shell/bunshell.test.ts(new tests for every site above),commands/{echo,which,mv}.test.ts,shell-pipe-read-fault.test.ts(now expects 1). Also the rest oftest/js/bun/shell.
Background
- A builtin writes to a real fd through
IOWriter, the shell's write queue. The result reaches itson_io_writer_chunkas anOption<bun_sys::SystemError>. SystemError.errnois negated to match Node'serr.errno.ExitCodeis au16, so the negated field wraps: -28 becomes 65508.- When output is captured,
Cmd.rsrelays the child's stdout and stderr into the shell's buffers. The Cmd finishes once it has an exit status and both relays are closed.
Notes
Exit codes before and after, Linux:
| command | before | after | bash / coreutils |
|---|---|---|---|
echo hi > /dev/full | 65508 | 1 | 1 |
which sh > /dev/full | 65508 | 1 | 1 |
export FOO=bar; export > /dev/full | 1 | 1 | 1 |
cd /nonexistent 2> /dev/full | 1 | 1 | 1 |
rm 2> /dev/full (usage message) | 28 | 1 | 1 |
echo hi | cat, process stdout on /dev/full | 28 | 1 | 1 |
[[ -f <glob, no match> ]], process stderr on /dev/full | 28 | 1 | 1 |
rm -v a b > /dev/full | 0 | 1 | 1 |
mv nosuchsrc dst | 2 | 1 | 1 |
mv d d/inside | 22 | 1 | 1 |
mv d plain | 20 | 1 | 1 |
$(echo "sh -c false") | 0 | 1 | 1 |
$(echo env; exit 5) | 5 | 0 | 0 |
cd /nonexistent (stderr) | cd: not a directory: /nonexistent | cd: /nonexistent: No such file or directory | same as after |
Not changed here: ls, mkdir -v and cp -v still exit 0 when their verbose output write fails, because OutputTask::on_io_writer_chunk drops the error. #40033 fixes that site and adds the bash-style name: write error: ... report. rm <missing file> already exits 1 (the task path reports its own error).
The pre-Rust Zig builtins (src/shell/builtin/*.zig at 23427dbc12^) did not agree with each other: echo, which, export, cd and cat returned e.getErrno(), basename, dirname, seq, yes and mkdir returned 1, pwd and ls ignored the write error. So there is no single old behavior to restore, and exit 1 is the one the Rust port already uses for half of the builtins.
#40702 proposed the positive errno for echo, which, export and cd and is closed in favor of this PR. Its test cases for export > /dev/full and cd /nonexistent 2> /dev/full are carried over here as exit 1 assertions (they already exit 1 on main, so they guard the invariant).
#40033 (open) adds the stderr report for every builtin write failure, also with exit 1. It overlaps this PR on echo.rs, which.rs, cat.rs, rm.rs, Cmd.rs and CondExpr.rs. Whichever lands second needs a rebase. The two do not disagree on the exit code.
Relay ordering in Cmd.rs: rebased onto #41436, which routes exit, stdin close, stdout close and stderr close through one finish_if_done and makes on_exit keep a status a stdio error already recorded. This PR now only changes what that recorded status is (1, not the errno), so a relay failure yields 1 in either event order, including over a child that exits nonzero after its output was lost. The earlier output_relay_failed flag and is_done() guard from review are gone because #41436 settles the ordering itself. A relay failure only happens when a pipe read or the shell's own stdout write fails (ENOMEM, ENOSPC, EPIPE).
Rebase onto #41436 resolved three conflicts: Cmd.rs (took main's on_exit/finish_if_done, kept the exit-1 assignment in the two relay-close handlers and the substitution-stash clear), subproc.rs (took main's on_process_exit unchanged), bunshell.test.ts (two adjacent added tests, kept both). #41436's two new SHELL_FAIL_EPOLL_IN tests asserted exitCode: ENOMEM; they now expect 1.
Tests on main that pinned the old errno-valued exit codes and were updated to expect 1: the shell-pipe-read-fault.test.ts cases (previously ENOMEM, 12, including the two added by #41436) and the mv cross-device "unreadable directory" case (previously EACCES, 13).
Tests are Linux only where they use /dev/full, which does not exist on macOS and Windows.
Suites run on the debug build: bunshell, commands/*, shell-pipe-read-fault, epipe, shell-write-fault, exec, throw, file-io, lazy, pipeline_stack, shelloutput, yield, shell-seq-condexpr, shell-hang. The ls permission-denied tests fail as root on stock bun too. The leak, shell-load and shell-leak-args tests time out or hit the pid cap of the build container and do not touch an error exit path.
no test proof · iteration 17 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/shell-pipe-read-fault.test.ts, test/js/bun/shell/commands/which.test.ts, test/js/bun/shell/commands/echo.test.ts, test/js/bun/shell/bunshell.test.ts