shell: exit with the positive errno when a builtin's output write fails

Closed
RO
2mo ago
+38-7

Problem

  • A builtin whose output write fails reports a garbage exit code. echo hi > /dev/full and which sh > /dev/full exit 65508 (ShellError: Failed with exit code 65508). export > /dev/full and cd /nonexistent 2> /dev/full exit 1 and drop the errno.
  • echo.rs:116 and which.rs:182 cast SystemError.errno to the u16 exit code. That field holds the negated errno (fill_system_error_common, src/sys/Error.rs:371), so ENOSPC (-28) wraps to 65508. export.rs:85 and cd.rs:127 hard-code 1.

Fix

  • The four builtins read the errno through SystemError::get_errno(), which undoes the negation. cat and rm already do this, and the Zig builtins did too (getErrno()). cd keeps exit 1 when the stderr message is written without error.
  • mv reads a failed rename's errno through bun_sys::Error::get_errno() instead of the raw field. That field is already positive, so the value does not change on POSIX.
  • Excluded on purpose: pwd, basename, dirname, seq and yes keep exit 1 on a failed write. They did that before the rewrite too. #40033 proposes one status for every builtin.
  • Verified: test/js/bun/shell/bunshell.test.ts (new test, stock bun reports 65508/65508/1/1). Also the rest of bunshell.test.ts, commands/{echo,which,mv}.test.ts, epipe.test.ts, shell-pipe-read-fault.test.ts, shell-write-fault.test.ts.

Background

  • A builtin that writes to a real fd (a file redirect, or the process stdout) goes through IOWriter, the shell's per-fd write queue. The result arrives later in the builtin's on_io_writer_chunk as an Option<bun_sys::SystemError>.
  • bun_sys::SystemError is the JS-facing error shape. Its errno is stored negated to match Node's err.errno. get_errno() returns the positive E value.
  • bun_sys::Error is the syscall-level error. Its errno is the positive u16.
Notes

Values from the repro with /dev/full as the redirect target (Linux, ENOSPC = 28):

command1.4.1now
echo hi > /dev/full6550828
which sh > /dev/full6550828
export FOO=bar; export > /dev/full128
cd /nonexistent 2> /dev/full128
cd /nonexistent (stderr ok)11
mv nosuch dst22
pwd > /dev/full11
ls / > /dev/full00

On Windows SystemError.errno is the libuv code (UV_ENOSPC = -4075), so the raw cast gave a different garbage value there. get_errno() canonicalizes it to the E discriminant, so the exit code is 28 on every platform.

The test is Linux only because /dev/full does not exist on macOS and Windows.

The excluded sites, with the pre-rewrite Zig behavior (commit 23427dbc12^, src/shell/builtin/*.zig):

  • pwd.zig, basename.zig, dirname.zig, seq.zig, yes.zig: onIOWriterChunk returned done(1) on a write error. The Rust port (pwd.rs:72, basename.rs:62, dirname.rs:64, seq.rs:209, yes.rs:166) does the same.
  • ls.zig: onIOWriterChunk dropped the error. ls.rs:182 does the same, so ls / > /dev/full exits 0.
  • echo.zig, which.zig, export.zig, cd.zig: onIOWriterChunk returned done(e.getErrno()). The Rust port changed these four, and this PR restores them.

#40033 and #32278 are open PRs that change the same sites to exit 1 (coreutils style) for every failed write. This PR keeps the errno as the exit code, which is what the other builtins report and what the Zig implementation did.

no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/bunshell.test.ts