output: report a failed write to stdout instead of claiming success

RO
24d ago
+637-465

Adopts #38807 by @alii. His 16 commits are rebased onto main, he remains their author. The last five commits are mine, from review findings: install: the remaining log prints to stderr are best-effort too, bun build: write a single bundle to stdout unbuffered so a failed write is reported, the two restore the formatter indent on every exit of write_format commits (BuildArtifact, Blob), and install: an allocation failure while printing the summary tree is still an OOM. Supersedes #38807.

Problem

  • A write to stdout through Output::writer() cannot fail. fd_write_all_quiet (src/sys/lib.rs:9255) drops the error and adapter_write_all (src/sys/lib.rs:9301) returns Ok(()) to the caller.
  • So bun pm hash, bun pm hash-string and a single file bun build to stdout with a closed or read only stdout print nothing and exit 0. Bun 1.3 exits 1 with WriteFailed in this case.
  • The quiet path for bun bun.lockb | head (issue #5828) matches install::Error::WriteFailed (src/install/lockfile.rs:1655). The #[from] conversion wrapped the core error as Core(WriteFailed), so the arm did not match. Nobody could see this while writes never failed.
  • The console formatter calls .expect("unreachable") on writes of Map, Set, Request and long string values (src/jsc/ConsoleObject.rs, src/runtime/webcore/Request.rs, src/runtime/api/JSBundler.rs). Once a write can fail, console.log of a large Map into a closed pipe aborts inside a C callback.

Fix

  • fd_write_all_quiet returns the write error. A zero length write is EIO. adapter_write_all and adapter_flush map it to bun_core::Error::WriteFailed. The buffer position is reset before the error returns, so a later write does not drain stale bytes.
  • From<bun_core::Error> for install::Error maps WriteFailed to install::Error::WriteFailed, so the quiet arm in lockfile.rs matches again. A broken pipe stays quiet and exits 0. EIO, ENOSPC and a bad fd exit 1 with WriteFailed, as in 1.3.
  • The console formatter records a failed write in Formatter::failed and stops printing that value. This is what the other formatter paths already do. Request and BuildArtifact return the error to their caller. BuildArtifact::write_format and Blob::write_format now hold their indent in an IndentScope, as Request does, so an early return leaves the formatter at its original depth.
  • Prints in the middle of real work are best effort now: the bun --filter child output and exit line (multi_run.rs), the coverage table (test_command.rs), the bun build summary (build_command.rs), the install log and summary, and five more log prints (print_fetch_failures, root_package_json_source, pm view, update -i, the bun bun.lockb printer). The work after each print (dependents, lcov and the exit code, output files, lockfile, Global::crash()) always happens.
  • Where the output is the result, the error is kept: pm hash, pm hash-string, and a single file bun build to stdout. The build path now flushes and writes the bundle through the unbuffered Output::writer(). Before, a bundle under 4 KB sat in the stdout buffer and Output::flush() dropped the error, so only bundles of 4 KB or more reported the failure.
  • Verified: the six changed test files and test/regression/issue/05828.test.ts pass on a debug build of the current head (after each rebase). The two bun pm hash tests fail on main (stderr empty, exit 0). The other new tests pass on main, where no write fails. They guard the sites that a failing write reaches after this change. Details in the notes.

Background

  • Output::writer() is the buffered stdout writer the CLI uses. It is a bun_core::io::Writer vtable. src/sys/lib.rs supplies the implementation (SysQuietWriterAdapter): small writes go into a buffer, a large write or a flush calls write(2) on the fd.
  • bun_core::Error::WriteFailed is the one error a Writer can return. Each crate has its own Error enum and converts the core error with From. A ? on a write inside bun_install produces an install::Error.
  • The console formatter (Formatter in src/jsc/ConsoleObject.rs) prints one value at a time. It has a failed flag. Iteration over a Map or a Set runs inside a C++ callback, so an error cannot be returned from there. The flag is how the formatter stops.
  • console.log writes into a 4 KB buffer. A value that formats to more than 4 KB makes the formatter write to the fd while it is still printing. This is why the console test prints 200 entries per value.
  • The tests make stdout fail by passing a read only descriptor of /dev/null as the child's stdout. write(2) on it fails with EBADF at once. This does not work on Windows, so the tests are skipped there.
Notes

Rebases:

  • First rebase: one conflict, in the imports of test/bundler/cli.test.ts (2a707c0dda added normalizeBunSnapshot). The new callers on main (bun pm diff uses Output::print_bytes, which ignores the error) need no change.
  • Second rebase: c6a05c1014 removed install::Error::BrokenPipe. The quiet arm in lockfile.rs matches WriteFailed only, which is the variant the new From impl produces. 05828.test.ts still passes.
  • Third rebase: #39770 (3d3016e329) turned the const generics of print_code_coverage into runtime bools and a pretty_lit! macro. This conflicted with the coverage commit. The resolution keeps main's spelling and this PR's shape: every table write is let _ =, none of them returns early any more. The resolved commit is still authored by @alii.
  • Fourth rebase: #39962 (b4684c80cb) added normalizeBunSnapshot to the imports of test/cli/install/bun-pm.test.ts, next to the imports this PR adds. Union of both, nothing else conflicted. The dead code removals in #39582 and #39732 do not touch anything this PR uses (disable_buffering_scope, written_slice, InvalidCharacter are all still there).
  • Fifth rebase: no conflicts. It picks up #40039, which fixes test/cli/install/bun-audit.test.ts on main. That test was red on every lane of the previous CI run (build 103036) because the bump to 1.4.1 made normalizeBunSnapshot rewrite the literal <1.4.1 inside the audit snapshot. This PR does not touch it.
  • Sixth rebase: #39934 added a test at the end of test/cli/test/coverage.test.ts, where this PR adds one too. Both are kept. Four pm ls tests in test/cli/install/bun-pm.test.ts fail on main itself since #38952 changed the header to (N installed) without updating the inline snapshots that #39962 added (node_modules (5)). This PR does not touch pm ls.
  • Seventh rebase: no conflicts. It picks up #40062, which fixes those pm ls snapshots and the prune summary expectation. bun-pm (24), bun-prune (110, 1 skipped) and bun-audit (182) pass on the debug build of this head, together with the files listed above.
  • Eighth rebase: #40201 added the BuiltinBytecode and BytecodeStringTable arms to the two match f.output_kind expressions of the build summary, which this PR moved into print_output_file_line. The arms are in the moved function. bundler cli (35, 2 skipped), bun-build-api (56, 1 skipped), bun-pm, console-log, coverage, filter-workspace, bun-update-transitive and 05828 pass on the debug build of this head.
  • Ninth rebase: same spot. #40509 added the ModuleInfoStringTable arm, which is in print_output_file_line now. The same files pass on the debug build of this head. Two new bytecode tests in bun-build-api.test.ts from main hit the 5 s limit on this machine (a debug build, 14 s and 31 s when run alone with a longer timeout) and are unrelated to this PR.

Two review comments outside the diff: print_summary_tree discarded every error of Tree::print, including Alloc. The last commit sends Alloc to bun_core::out_of_memory(), as the lockfile printer does, and keeps only WriteFailed best-effort (bun-add 70 and bun-update-transitive 175 pass). The Bun.sleep(100) in the --parallel merges line coverage across workers test belongs to #39934 on main, not to this PR.

The install commit: four of the five prints sit right before Global::crash() or Global::exit(1), where a failed write replaced the intended exit with a WriteFailed error. In print_fetch_failures a failed write made bun outdated exit 1 for an optional dependency's warning and skipped the log reset. print_fetch_failures returns the bool and root_package_json_source returns the Source, since neither has an error path left. The new test in bun-update-transitive.test.ts exits 1 with that site reverted and passes with it.

The bun build commit: Output::writer() is the unbuffered adapter, Output::writer_buffered() the 4 KB one, and Output::flush() drops the flush error. The single bundle path wrote through the buffered one, so a small bundle to a read only stdout exited 0 with nothing written while a large one exited 1. Probed by hand on the debug build: a 25 KB and a 20 byte bundle to a read only stdout both exit 1 with WriteFailed now, a normal stdout still prints the bundle, and bun build a.js | head -c 1 exits 1 as it did for large bundles before this commit. The new test in test/bundler/cli.test.ts fails on main and on the head before this commit (empty stderr, exit 0). The same commit removes a leftover inner block in print_exit_status.

Test runs on the debug build after the fifth rebase and the bun build commit: bun-pm (24, this now includes the pm ls tests from #39962), filter-workspace (80, 2 skipped on linux), console-log (5), bundler cli (35, 2 skipped on linux), coverage (13), bun-update-transitive (175), 05828 (1), bun-audit (182). After the third rebase also the coverage consumers 29925, 30205, 31503. Before the rebases also bun-info, update_interactive_install and the pnpm-lock-v9 manifest block. One run of bundler cli.test.ts had the three concurrent --compile tests at about 4.5 s each on this machine and one of them hit the 5 s limit once. It passed alone and in the next full run. This PR does not touch that path.

Fail-before on the release build of main (8326d1bd3): bun pm hash and bun pm hash-string with a read only stdout print nothing to stderr and exit 0. The tests expect WriteFailed and exit 1.

no test proof · iteration 8 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/web/console/console-log.test.ts, test/cli/test/coverage.test.ts, test/cli/run/filter-workspace.test.ts, test/cli/install/bun-update-transitive.test.ts, test/cli/install/bun-pm.test.ts, test/bundler/cli.test.ts