Make the allocation-only collection APIs infallible and drop the OOM wrappers around them

[robobun]

Problem

• Most of the Result<_, AllocError> plumbing in the tree is a leftover of the Zig port: the callee grows a Vec or a hashbrown table, which aborts on allocation failure, and then returns Ok unconditionally. ArrayHashMap::put, get_or_put, ensure_unused_capacity, StringHashMap::put, HashMap::put, ByteVecExt::write, the bit sets and so on all worked this way (array_hash_map.rs even said "kept as Result for API stability").
• Every caller then paid for it: bun_core::handle_oom(..), .unwrap_or_oom(), .expect("oom"), ?, .map_err(|_| OutOfMemory)?, let _ = ..; // OOM/capacity: fire-and-forget, // bun.handleOom comments, and branches that look like graceful OOM handling but are unreachable (Worker env clone returning "Out of memory", Bun.spawn throwing OOM for the env block, ArrayBufferSink / HTTP stream writes returning ENOMEM, SQL buffer writes mapping to OutOfMemory, markdown slug dedupe and package.json scripts silently giving up).
• let _ = self.graph.ast.append(..) on a MultiArrayList (the one container whose Result was real) would have silently desynced graph.ast from graph.input_files had the allocation ever failed.
• Reporting was inconsistent: handle_oom produced an out-of-memory crash report, .expect("oom") a panic report saying it is a bug in Bun, and a plain Vec growth failure went through std's handle_alloc_error, printing memory allocation of N bytes failed and aborting, which is reported as an abort() crash (or, on ASAN builds, not at all). Nothing hooked std's alloc error path.

Fix

• bun_collections: the allocation-only methods return their value directly. MultiArrayList and the dynamic bit sets, which allocate by hand, call std::alloc::handle_alloc_error on failure exactly like Vec does. Types whose inherent clone() returned Result implement Clone instead (ArrayHashMap, StringArrayHashMap, StringSet, the bit sets, bun_dotenv::Map, DeclaredSymbolList), which also removes the hand-written deep-clone code that existed to work around that (clone_macro_remap, clone_macro_map, Map::clone_with_allocator, the MacroRemap rebuilds in transpiler.rs, RuntimeTranspilerStore.rs and jsc_hooks.rs, and the manual Clone impls / clone() methods on ExternalModules, DependencyMap, Framework and ESMConditions, which are derives now).
• bun_crash_handler::install_hooks registers std::alloc::set_alloc_error_hook next to the panic hook, routing into the existing out_of_memory() path. With that, dropping the wrappers is purely a cleanup: a failed allocation anywhere in Rust code now produces the same "Bun has run out of memory" report handle_oom produced, and this is also what makes the handle_alloc_error calls in the containers correct. The hook does not allocate (one diverging call); a second failure inside the report hits the existing re-entry guard.
• Callers: the wrappers listed above are removed everywhere the compiler forced it, on all 10 CI targets. Thin wrappers whose only error source was one of these calls return directly (bun_dotenv::Map and Loader::load_process / load_node_js_config, DeclaredSymbolList, semver::StringPool, PathToSourceIndexMap, SavedSourceMap::put_value / put_mappings, LineOffsetTable::generate, mime_type::create_hash_table, ensure_stale_bit_capacity, register_barrel_with_deferrals, ...). bun_dotenv::Error::Alloc and the bun_alloc dependency of bun_dotenv / bun_http_types are gone because nothing produces an AllocError there anymore.
• The one real use of the old Result is kept: source map parsing reserves rows from the segment count of an untrusted map and turns a failed reservation into a parse error for the user. It now goes through MultiArrayList::try_ensure_total_capacity (the Vec::try_reserve analog); everything else that reached that Result crashed on Err anyway.
• Deliberately left alone: LinearFifo (a full StaticBuffer is reported through the same error and Channel blocks on it; it needs its own error type), yaml.rs (uses AllocError as its merge-key budget error), and the next layer of functions that still return Result<_, AllocError> (about 470, roughly half of which can no longer fail after this change), which cascade further and are for follow-up PRs, ending with deleting handle_oom / UnwrapOrOom and giving the few user-facing buffers that should survive OOM (the sinks and SQL buffers above) real try_ paths.
• Ratchets so this does not come back: clippy::let_unit_value is no longer allowed (the 174 remaining let _ = unit_call(); bindings are removed; cppbind.ts stops emitting let __r = for void C++ functions so the generated wrappers pass too), and the port-era source lint now rejects bun.handleOom comments (all 29 removed).
• Verified:
  • test/cli/run/run-crash-handler.test.ts: new crash_handler.allocError() hook calls handle_alloc_error; added to the terminal-signal and automatic-reporter matrices. Fails without the hook (memory allocation of 4096 bytes failed, no report on the ASAN debug build), passes with it.
  • test/internal/source-lints/port-era-markers.test.ts with the new pattern.
  • cargo check --workspace --tests, cargo clippy --workspace (clean, with let_unit_value enforced), bun run rust:check-all (10 of 10 targets), cargo test and cargo miri test for bun_collections.
  • bun bd test on sourcemap, env, bundler (barrel, splitting, html manifest), transpiler, isolated install / patch / audit, sql, glob, fs.watch, http response streams, http2, framework router suites.

Background

• AllocError is the unit error type every crate's Error enum wraps as Alloc(..); in Zig every allocating call returned error.OutOfMemory and bun.handleOom(x) was the idiom for "crash with an OOM report if this failed". The port translated the signatures literally, but Rust's Vec, Box and hashbrown never return an error: on failure they call handle_alloc_error, so a Result returned from code built on them can only ever be Ok.
• handle_alloc_error consults a process-global hook (std::alloc::set_alloc_error_hook, a nightly API; the workspace is already pinned to nightly) and aborts if the hook returns. Bun's out_of_memory() never returns: it prints the crash report with CrashReason::OutOfMemory and exits, which is what distinguishes an OOM from a bug in the crash telemetry.
• MultiArrayList is a struct-of-arrays container that allocates its columns with a raw Allocator, and the dynamic bit sets realloc a word buffer by hand, so those two were the only collections whose Result could actually be Err; they now have the same split as Vec (ensure_* aborts, try_ensure_* reports).
• clippy::let_unit_value flags let _ = f(); where f returns (); it had been allowed in Cargo.toml because the port produced these bindings wherever a formerly fallible call became infallible.

This also covers the hook proposed in #31690, which this PR supersedes.

Counts

Occurrences in src/**/*.rs, main vs this branch:

pattern	before	after
handle_oom(	168	75
.unwrap_or_oom()	59	18
.expect("oom") / .expect("OOM")	167	88
// OOM/capacity comments	62	35
// OOM-only comments	25	11
bun.handleOom comments	29	0
let _ = <unit> (clippy)	174	0

The remaining ones wrap functions outside bun_collections that still return Result<_, AllocError> (e.g. semver::Builder::allocate, AstBuilder, ini's OOM<T>, enqueue_* in bundle_v2); those are the next layer.

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/run/run-crash-handler.test.ts

[robobun]

^{Updated 11:58 PM PT - Aug 14th, 2026}

@robobun, your commit 55a42d9 is building: #97543

[robobun]

Status: diff complete and verified locally (cargo check/clippy on all targets, collections unit tests under miri, the crash handler test matrix, and the bundler/install/sourcemap/env/sql suites listed in the description). Waiting on CI. Follow-up PRs will peel the next layer of functions that still return Result<_, AllocError> and then remove handle_oom itself.

[alii]

@robobun fix conflicts + rebase

[robobun]

Rebased on main (conflicts were in computeChunks.rs, npm.rs, patch_install.rs, PackageManagerTask.rs, PackageInstall.rs and bun.lock.rs, all resolved in favor of main's newer code; main's new collect_bundled_deps returns bool now since StringSet::insert no longer returns a Result).

Also addressed the review on the remaining manual clones: the MacroMap rebuilds in RuntimeTranspilerStore.rs, jsc_hooks.rs and JSTranspiler.rs (clone_macro_map) are .clone() now, and ExternalModules, DependencyMap, Framework and ESMConditions derive Clone instead of hand-writing it. Swept the comments next to the changed call sites that still described the removed Result plumbing (JSON5Object.rs and a few others). Workspace check is clean on all targets, clippy is clean, and the affected suites pass locally.

[robobun]

Pushed 55a42d9 for the two remaining review threads: the ESMConditions rebuild in BundleOptions::deep_clone is self.conditions.clone(), the stale "result intentionally discarded" / "OOM-only Result" comments are gone, and every StringSet::insert caller is a bare call now. Clippy and the md, bundler, test-changed, websocket subprotocol and transpiler suites pass locally; the branch is still mergeable against main without another rebase.

[claude]

I reviewed this PR across three passes; the earlier findings (redundant clone rebuilds, stale OOM comments) were addressed in aec6dc0 and 55a42d9, and this pass found no new issues. Given the scope — 247 files, a new nightly alloc_error_hook install in the crash handler, and API-surface changes to every allocating collection — a human sign-off on the overall design is still worthwhile.

What was reviewed: the bun_collections signature changes and their call-site rewrites (verified the compiler forces every dropped ?/handle_oom); the MultiArrayList try_ split and its one intentional caller in sourcemap parsing; rust_alloc_error_hook wiring and the new allocError() test hook; and that LinearFifo/yaml.rs were left alone as stated.

Extended reasoning...

Overview

This PR makes the allocation-only methods on bun_collections (ArrayHashMap, StringArrayHashMap, StringHashMap, HashMap, MultiArrayList, the bit sets, ByteVecExt, StringSet, PriorityQueue) return their value directly instead of Result<_, AllocError>, and removes the ~400 caller-side handle_oom/.unwrap_or_oom()/.expect("oom")/let _ = wrappers the compiler then flags. It installs std::alloc::set_alloc_error_hook in bun_crash_handler::install_hooks so any handle_alloc_error (from Vec, hashbrown, or the containers' own new abort paths) routes through the existing out_of_memory() crash report. Types whose inherent clone() was fallible now impl Clone (or #[derive(Clone)]), which let several manual deep-clone helpers and impl Clone blocks collapse. Two ratchets are added: clippy::let_unit_value is un-allowed, and the port-era source lint rejects bun.handleOom comments. A new crash_handler.allocError() test hook is added to the crash-handler test matrix.

Security risks

None identified. The change is error-plumbing removal around allocation paths that already could not return Err; the one place that genuinely reserved from untrusted input (sourcemap segment count) keeps its fallible path via the new try_ensure_total_capacity. No auth/crypto/permission logic is touched beyond mechanical wrapper removal.

Level of scrutiny

High. The individual edits are mechanical and compiler-enforced (a missed ? or wrong return type would not build, and the PR states cargo check / clippy / rust:check-all on 10 targets pass), but the aggregate is a design decision about OOM handling for the whole runtime: a nightly #[feature(alloc_error_hook)] is added, a process-global hook is installed, bun_dotenv::Error::Alloc is removed, and the intent is to eventually delete handle_oom entirely. That direction — and whether the sinks/SQL buffers listed as future try_ candidates are the right cut line — is worth a maintainer's eye.

Other factors

I left two prior rounds of findings on this PR (redundant MacroRemap/ESMConditions rebuilds and stale "result discarded"/"OOM-only" comments); all are marked resolved and the fixes are visible in the current diff and commit log. The PR adds a real regression test for the new hook (fails without it per the description), extends the source lint, and documents what was deliberately left for follow-up. The sheer breadth (247 files across bundler, install, runtime, jsc, crash handler) is the reason to defer rather than any specific concern.

[robobun]

Nothing further from my side: the review rounds are addressed and resolved, and CI on 55a42d9 is at 176 jobs passed with the last 3 still running, no failures so far (the lint workflows, including mordant, were already green on the previous push). Ready for a maintainer to look at the design, in particular the cut line described under Fix: bulk reservations sized from untrusted input keep a try_ path (source maps today), everything else aborts through the OOM report like Vec, and the sinks and SQL buffers get real try_ paths in a follow-up.

[robobun]

CI is green on 55a42d9 (build 97543, all 179 jobs passed; the only test failures were per-test retries that passed on retry). Ready for review.