install: don't re-download a tarball that already failed

Merged
RO
4mo ago
+229-29

What

bun install runs the entire download (including the full retry cycle) for a failing tarball twice: once during the resolve phase, then again during the install phase. A permanently-500 endpoint sees 12 GETs where 6 are intended, and the same error: GET ... - 404 line prints twice.

Repro

import * as fs from "node:fs";
const ROOT = fs.mkdtempSync("/tmp/tb-twice-");
let status = 404, reqs = [];
const server = Bun.serve({
  hostname: "127.0.0.1", port: 0,
  fetch(req) {
    const p = new URL(req.url).pathname;
    reqs.push(p);
    if (p === "/tb") return new Response("no", { status });
    if (p === "/pk") return Response.json({ name: "pk", "dist-tags": { latest: "1.0.0" },
      versions: { "1.0.0": { name: "pk", version: "1.0.0",
        dist: { tarball: `http://127.0.0.1:${server.port}/tb` } } } });
    return new Response("nope", { status: 404 });
  },
});
for (const s of [404, 500]) {
  status = s; reqs = [];
  const box = `${ROOT}/${s}`; fs.mkdirSync(box);
  fs.writeFileSync(`${box}/package.json`, '{"name":"s","dependencies":{"pk":"1.0.0"}}');
  fs.writeFileSync(`${box}/bunfig.toml`,
    `[install]\nregistry = "http://127.0.0.1:${server.port}/"\ncache = "${box}/.c"\n`);
  const proc = Bun.spawn([process.execPath, "install", "--no-progress", "--ignore-scripts"],
    { cwd: box, stderr: "pipe", env: { ...process.env, HOME: box, NO_COLOR: "1", CI: "1" } });
  const err = await new Response(proc.stderr).text(); await proc.exited;
  console.log(s, "tarball GETs =", reqs.filter(x => x === "/tb").length,
    "| error lines =", err.split("\n").filter(l => l.startsWith("error:")).length);
}
server.stop(true); fs.rmSync(ROOT, { recursive: true, force: true });

Before:

404 tarball GETs = 2 | error lines = 2
500 tarball GETs = 12 | error lines = 2

After:

404 tarball GETs = 1 | error lines = 1
500 tarball GETs = 6 | error lines = 1

Cause

On a fresh install (no lockfile) the resolve phase prefetches the tarball via generate_network_task_for_tarball. When that download terminally fails, runTasks.rs removed the entry from network_dedupe_map (and task_queue) so a later enqueue for the same task id would not wait forever on a callback that never arrives. But removing the entry meant the install phase's enqueue_package_for_download saw found_existing == false for both maps and scheduled a brand-new network task for the same URL, re-running the whole retry cycle and re-reporting the error.

Fix

Keep the dedupe entry and mark it failed instead of removing it. enqueue_package_for_download / enqueue_tarball_for_download check this flag up front and return a new ForTarballError::AlreadyFailed, which each caller handles by advancing its own bookkeeping without scheduling anything:

  • hoisted installer: summary.fail += 1 and increment_tree_install_count
  • isolated installer: mark the entry step Done and on_task_complete(Fail) (so the upfront per-entry pending_tasks slot is released and the loop does not hang)

Successful downloads are unchanged (still fetched exactly once). Both linkers covered by the new test in bun-install-retry.test.ts.


[review] gate passed · iteration 0 · 6 files touched

fails on main (without fix)
ASAN without fix: 6 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/cli/install/bun-install-retry.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (299bee1c6)

test/cli/install/bun-install-retry.test.ts:
(pass) retries a manifest whose redirect target 500s once [320.46ms]
(pass) retries an authorized manifest whose cross-origin redirect target 500s once [234.17ms]
(pass) retries a tarball whose redirect target 500s once [205.72ms]
(pass) retries on 500 [318.75ms]
343 |     });
344 |     const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]);
345 | 
346 |     const tarballGets = urls.filter(u => u === "/BaR-0.0.2.tgz");
347 |     const errorLines = err.split("\n").filter(l => l.startsWith("error:"));
348 |     expect({ tarballGets: tarballGets.length, errorLines }).toEqual({
                                                                
... (truncated)

release without fix: 1 FAILED
bun test v1.4.0-canary.1 (cfce8d9a2)

test/cli/install/bun-install-retry.test.ts:
(pass) retries a manifest whose redirect target 500s once [9.19ms]
(pass) retries an authorized manifest whose cross-origin redirect target 500s once [7.50ms]
(pass) retries a tarball whose redirect target 500s once [6.01ms]
(pass) retries on 500 [7.71ms]
(pass) linker=hoisted > does not re-download a tarball that already failed with 404 [4.77ms]
(pass) linker=hoisted > does not re-download a tarball that already failed with 500 [4.71ms]
414 |     expect(await file(join(package_dir, "node_modules", "baz", "package.json")).json()).toMatchObject({
415 |       name: "baz",
416 |       version: "0.0.3",
417 |     });
418 |     if (linker === "hoisted") {
419 |       expect(out).not.toContain("Failed to install");
                            ^
error: expect(received).not.toContain(expected)

Expected to not contain: "Failed to install"
Received: "bun install v1.4.0-canary.1 (cfce8d9a2)\n\n+ baz@0.0.3\n\n1 package installed [3.00ms]\nFailed to install 1 package\n"

      at <anonymous> (/workspace/bun/test/cli/install/bun-install-retry.test.ts:419:23)
(fail) linker=hoisted > does not re-down
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/cli/install/bun-install-retry.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (299bee1c6)

test/cli/install/bun-install-retry.test.ts:
(pass) retries a manifest whose redirect target 500s once [332.36ms]
(pass) retries an authorized manifest whose cross-origin redirect target 500s once [257.74ms]
(pass) retries a tarball whose redirect target 500s once [283.89ms]
(pass) retries on 500 [301.93ms]
(pass) linker=hoisted > does not re-download a tarball that already failed with 404 [192.95ms]
(pass) linker=hoisted > does not re-download a tarball that already failed with 500 [201.65ms]
(pass) linker=hoisted > does not re-download an optional dependency's tarball that already failed [291.58ms]
(pass) linker=isolated > does not re-download a tarball that already failed with 404 [240.88ms]
(pass) linker=isol
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
[configured] bun-profile → bun (stripped) in 722ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/21] gen generated_host_exports.rs
generated_host_exports.rs: 91 exports (host=3, lazy=10, generic=78, rust=0); 243 extern-C blocks audited
[2/21] gen JS modules (bundle-modules)
Preprocess modules (6866ms)
Bundle modules (31ms)
Postprocesss modules (130ms)
Bundle Functions (812ms)
Generate Code (118ms)

[7.98s] Bundled "src/js" for production
  1912 kb
  162 internal modules
  12 native modules
  90 internal functions across 19 files
[2/7] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: component rust-std is up to date

  night
... (truncated)
diff hotspot
src/install/NetworkTask.rs                         |  10 ++
 src/install/PackageInstaller.rs                    |  18 +++
 .../PackageManager/PackageManagerEnqueue.rs        |   6 +
 src/install/PackageManager/runTasks.rs             |  70 +++++++-----
 src/install/isolated_install.rs                    |  27 +++++
 test/cli/install/bun-install-retry.test.ts         | 127 ++++++++++++++++++++-
 6 files changed, 229 insertions(+), 29 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                                 reads  edits  tests
src/install/NetworkTask.rs                               2      2      0
src/install/PackageInstaller.rs                          8      8      0
src/install/PackageManager/PackageManagerEnqueue.rs      7      2      0
src/install/PackageManager/runTasks.rs                   5      5      0
src/install/isolated_install.rs                          2      3      0
test/cli/install/bun-install-retry.test.ts               3      3      0