From 891a18b9074983382d65c298563d04bc43bd1ddc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 22:27:43 +0000 Subject: [PATCH 1/5] install: fail the update request of an npm: alias whose download fails A download that fails in the resolve phase fails the update requests it was for, so that bun add and bun update exit 1 and write nothing. The four arms of run_tasks compared the request name with the name of the registry package. A request names its package.json key, so a request for an npm: alias, for an entry that an override renames, or for a catalog: entry that is an alias never matched. When the dependency is optional the failure is only a warning, and the install continued: it saved bun.lock, and bun update wrote an empty version into package.json. fail_update_requests replaces the four copies. It also matches the dependencies the download was for: the waiters of the task, and for an npm tarball every dependency that resolved to its package. --- src/install/PackageManager/runTasks.rs | 106 +++++++++------ .../cli/install/bun-update-transitive.test.ts | 128 +++++++++++++++++- 2 files changed, 189 insertions(+), 45 deletions(-) diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index 318e0b6ef2e4..cc622c3b518a 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -542,16 +542,7 @@ fn run_tasks_erased( ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, name) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + fail_update_requests(manager, task.task_id, name, None); } continue; @@ -599,16 +590,7 @@ fn run_tasks_erased( response.status_code, ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, name) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + fail_update_requests(manager, task.task_id, name, None); continue; } @@ -853,16 +835,12 @@ fn run_tasks_erased( .fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto,), ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, extract.name.slice()) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + fail_update_requests( + manager, + task.task_id, + extract.name.slice(), + Some(extract.dependency_id), + ); if let Some(removed) = manager.task_queue.remove(&task.task_id) { drop(removed); @@ -936,16 +914,12 @@ fn run_tasks_erased( response.status_code, ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, extract.name.slice()) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + fail_update_requests( + manager, + task.task_id, + extract.name.slice(), + Some(extract.dependency_id), + ); if let Some(removed) = manager.task_queue.remove(&task.task_id) { drop(removed); @@ -1946,6 +1920,58 @@ pub(crate) fn network_task_has_failed(this: &PackageManager, task_id: Task::Id) .is_some_and(|e| e.failed) } +/// `bun add` / `bun update ` of a package that cannot be fetched exits 1 and saves nothing. +/// A request names its dependency, which an `npm:` alias or an override spells differently from +/// `package_name`, so it is also matched against the dependencies the download was for: the +/// task's waiters, and for an npm tarball (it has no waiters) every dependency resolved to the +/// package of `tarball_dependency_id`. +fn fail_update_requests( + this: &mut PackageManager, + task_id: Task::Id, + package_name: &[u8], + tarball_dependency_id: Option, +) { + if this.subcommand == Subcommand::Remove { + return; + } + let buffers = &this.lockfile.buffers; + let string_buf = buffers.string_bytes.as_slice(); + let dependencies = buffers.dependencies.as_slice(); + let resolutions = buffers.resolutions.as_slice(); + let waiters = this.task_queue.get(&task_id).map_or(&[][..], Vec::as_slice); + let package_id = tarball_dependency_id + .and_then(|id| resolutions.get(id as usize).copied()) + .filter(|&package_id| package_id != INVALID_PACKAGE_ID); + + let mut any_failed = false; + for request in this.update_requests.iter_mut() { + let waits = waiters.iter().any(|waiter| match waiter { + bun_install::TaskCallbackContext::Dependency(id) + | bun_install::TaskCallbackContext::RootDependency(id) => dependencies + .get(*id as usize) + .is_some_and(|dependency| request.matches(dependency, string_buf)), + _ => false, + }); + let resolved = package_id.is_some_and(|package_id| { + resolutions + .iter() + .zip(dependencies) + .any(|(&resolution, dependency)| { + resolution == package_id && request.matches(dependency, string_buf) + }) + }); + if waits || resolved || strings::eql(request.name, package_name) { + request.failed = true; + any_failed = true; + } + } + if any_failed { + this.options + .do_ + .remove(Do::SAVE_LOCKFILE | Do::SAVE_YARN_LOCK | Do::INSTALL_PACKAGES); + } +} + /// The first failed download in a `run_tasks` pass halves the number of /// concurrent requests (down to the configured minimum). fn throttle_after_network_error(manager: &PackageManager, has_network_error: &mut bool) { diff --git a/test/cli/install/bun-update-transitive.test.ts b/test/cli/install/bun-update-transitive.test.ts index 43d7ae599a80..eb42a24553f0 100644 --- a/test/cli/install/bun-update-transitive.test.ts +++ b/test/cli/install/bun-update-transitive.test.ts @@ -1235,7 +1235,12 @@ type Manifests = Record>; // Serves one manifest per name from memory; verdaccio has no parent whose newer version keeps a range on the same child, and its dist-tags cannot move mid-test. `tags` is read per request, so a test can move a tag after installing. -type RegistryKnobs = { times?: Record>; status?: Record }; +// `status` is keyed by package name or by tarball file name ("leaf-1.1.0.tgz"); `tarballOrigin` replaces this server's origin in every `dist.tarball`. +type RegistryKnobs = { + times?: Record>; + status?: Record; + tarballOrigin?: string; +}; async function serveRegistry(manifests: Manifests, tags: Tags = {}, knobs: RegistryKnobs = {}) { const tarballs = new Map(); @@ -1252,16 +1257,17 @@ async function serveRegistry(manifests: Manifests, tags: Tags = {}, knobs: Regis port: 0, fetch(request) { const { origin, pathname } = new URL(request.url); - const tarball = tarballs.get(pathname); - if (tarball) return new Response(tarball); const name = pathname.slice(1); - const entry = manifests[name]; const status = knobs.status?.[name]; if (status) return new Response("registry says no", { status }); + const tarball = tarballs.get(pathname); + if (tarball) return new Response(tarball); + const entry = manifests[name]; if (!entry) return new Response("not found", { status: 404 }); + const tarballOrigin = knobs.tarballOrigin ?? origin; const versions: Json = {}; for (const [version, extra] of Object.entries(entry)) { - versions[version] = { name, version, dist: { tarball: `${origin}/${name}-${version}.tgz` }, ...extra }; + versions[version] = { name, version, dist: { tarball: `${tarballOrigin}/${name}-${version}.tgz` }, ...extra }; } const latest = Object.keys(entry).sort(Bun.semver.order).at(-1); const time = knobs.times?.[name]; @@ -1860,6 +1866,118 @@ test.concurrent( }, ); +// A named request whose download fails exits 1 and writes nothing, also when the failure is only a warning (an optional dependency). The request names the package.json key; the failed download reports the registry name, which an `npm:` alias spells differently. +const LEAF_ONLY: Manifests = { leaf: { "1.0.0": {}, "1.1.0": {} } }; +const warningLines = (stderr: string) => stderr.split("\n").filter(line => line.startsWith("warn:")); + +// A host that closes every connection before it answers. It keeps its port for the whole test; the port of a stopped server could go to another test's registry. +const hangUp = () => + Bun.listen({ hostname: "127.0.0.1", port: 0, socket: { open: socket => void socket.end(), data() {} } }); + +// Breaks the download of leaf's manifest or of leaf@1.1.0's tarball; `down` is the origin of a `hangUp()` host. Returns the one warning to expect and the flags the update needs. +type Outage = (server: Bun.Server, knobs: RegistryKnobs, down: string) => { warning: unknown; flags?: string[] }; +const OUTAGES: Record = { + "a 404 for the manifest": (server, knobs) => { + knobs.status!.leaf = 404; + return { warning: `warn: ${manifestFailure(server, 404)}` }; + }, + "a registry that hangs up": (_server, _knobs, down) => ({ + warning: expect.stringMatching(/^warn: \w+ downloading package manifest leaf$/), + flags: ["--registry", `${down}/`], + }), + "a 404 for the tarball": (server, knobs) => { + knobs.status!["leaf-1.1.0.tgz"] = 404; + return { warning: `warn: GET ${server.url.origin}/leaf-1.1.0.tgz - 404` }; + }, + "a tarball host that hangs up": (_server, knobs, down) => { + knobs.tarballOrigin = down; + return { warning: expect.stringMatching(/^warn: \w+ downloading tarball leaf@1\.1\.0$/) }; + }, +}; + +// [label, the package.json key that reaches leaf, the fields that declare it with `range` on leaf, the update's arguments] +const aliasedLeaf = (range: string) => ({ optionalDependencies: { aliased: `npm:leaf@${range}` } }); +const OPTIONAL_LEAF_ENTRIES: [string, string, (range: string) => Json, string[]][] = [ + ["a plain entry", "leaf", range => ({ optionalDependencies: { leaf: range } }), ["leaf"]], + ["an npm: alias named by its key", "aliased", aliasedLeaf, ["aliased"]], + ["an npm: alias named by its key with --latest", "aliased", aliasedLeaf, ["aliased", "--latest"]], + ["an npm: alias named by its target", "aliased", aliasedLeaf, ["leaf"]], + [ + "an entry that an override renames", + "renamed", + range => ({ optionalDependencies: { renamed: "*" }, overrides: { renamed: `npm:leaf@${range}` } }), + ["renamed"], + ], + [ + "a catalog: entry that is an npm: alias", + "cataloged", + range => ({ + workspaces: { catalog: { cataloged: `npm:leaf@${range}` } }, + optionalDependencies: { cataloged: "catalog:" }, + }), + ["cataloged"], + ], +]; + +test.concurrent.each( + Object.keys(OUTAGES).flatMap(outage => OPTIONAL_LEAF_ENTRIES.map(entry => [outage, ...entry] as const)), +)("%s fails `bun update ` for %s and writes nothing", async (outage, _, key, fields, args) => { + const knobs: RegistryKnobs = { status: {} }; + using server = await serveRegistry(LEAF_ONLY, {}, knobs); + using down = hangUp(); + const optional = (range: string) => ({ name: "foo", ...fields(range) }); + const dir = await setupServed(server, "update-failed-download-", optional("1.0.0"), optional("^1.0.0")); + expect(await installedVersion(dir, key)).toBe("1.0.0"); + const before = { packageJson: await packageJsonText(dir), lock: await lockText(dir) }; + + const { warning, flags = [] } = OUTAGES[outage](server, knobs, `http://127.0.0.1:${down.port}`); + const { stderr, exitCode } = await run(dir, "update", ...args, ...flags); + expect(warningLines(stderr)).toStrictEqual([warning]); + expect(errorLines(stderr)).toStrictEqual([]); + expect(stderr).not.toContain("Saved lockfile"); + expect(await packageJsonText(dir)).toBe(before.packageJson); + expect(await lockText(dir)).toBe(before.lock); + expect(await installedVersion(dir, key)).toBe("1.0.0"); + expect(exitCode).toBe(1); +}); + +// An npm tarball is downloaded once for every entry that resolves to it, on behalf of the first one. +test.concurrent("a 404 for the tarball fails the request for a second alias of the same package", async () => { + const knobs: RegistryKnobs = { status: { "leaf-1.1.0.tgz": 404 } }; + using server = await serveRegistry(LEAF_ONLY, {}, knobs); + const packageJson = stringify({ + name: "foo", + optionalDependencies: { first: "npm:leaf@^1.0.0", second: "npm:leaf@^1.0.0" }, + }); + const dir = String(tempDir("update-failed-shared-tarball-", { "package.json": packageJson })); + await servedBunfig(server, dir); + + const { stderr, exitCode } = await run(dir, "update", "second"); + expect(warningLines(stderr)).toStrictEqual([`warn: GET ${server.url.origin}/leaf-1.1.0.tgz - 404`]); + expect(errorLines(stderr)).toStrictEqual([]); + expect(await packageJsonText(dir)).toBe(packageJson); + expect(await exists(join(dir, "bun.lock"))).toBe(false); + expect(exitCode).toBe(1); +}); + +test.concurrent.each(["leaf", "aliased@npm:leaf@^1.0.0"])( + "a 404 for the manifest fails `bun add --optional %s` and writes nothing", + async spec => { + const knobs: RegistryKnobs = { status: { leaf: 404 } }; + using server = await serveRegistry(LEAF_ONLY, {}, knobs); + const packageJson = stringify({ name: "foo" }); + const dir = String(tempDir("add-failed-download-", { "package.json": packageJson })); + await servedBunfig(server, dir); + + const { stderr, exitCode } = await run(dir, "add", "--optional", spec); + expect(warningLines(stderr)).toStrictEqual([`warn: ${manifestFailure(server, 404)}`]); + expect(errorLines(stderr)).toStrictEqual([]); + expect(await packageJsonText(dir)).toBe(packageJson); + expect(await exists(join(dir, "bun.lock"))).toBe(false); + expect(exitCode).toBe(1); + }, +); + test.concurrent("`bun update ` from a member leaves a sibling's own entry alone but lets it follow", async () => { const { dir, pkg2Text } = await staleMembers("~1.0.0", "^1.0.0"); const { stderr, exitCode } = await runIn(dir, "packages/pkg1", "update", "no-deps"); From c144dbe3af77be2ac692b142376128193db515c2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 23:33:30 +0000 Subject: [PATCH 2/5] install: match a failed download against the entries the request names fail_update_requests now looks only at the dependency lists that Lockfile::bind_update_requests binds a request to: the workspaces that received it under --filter or -r, otherwise the cwd workspace. A dependency with the same key in another workspace or deeper in the tree no longer fails the request. workspaces_of_update_request holds that rule for both callers. bunx @npm: for a package that the registry does not have now prints the 404 alone, as bunx does. The request is failed, so the install stops before it reports the same dependency as "failed to resolve". test/regression/issue/15276.test.ts expected that second line. --- src/install/PackageManager/runTasks.rs | 54 +++++++----- src/install/lockfile.rs | 26 +++--- .../cli/install/bun-update-transitive.test.ts | 83 +++++++++++++++---- test/regression/issue/15276.test.ts | 4 +- 4 files changed, 116 insertions(+), 51 deletions(-) diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index cc622c3b518a..9ec5aa4f2e5d 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -1922,9 +1922,9 @@ pub(crate) fn network_task_has_failed(this: &PackageManager, task_id: Task::Id) /// `bun add` / `bun update ` of a package that cannot be fetched exits 1 and saves nothing. /// A request names its dependency, which an `npm:` alias or an override spells differently from -/// `package_name`, so it is also matched against the dependencies the download was for: the -/// task's waiters, and for an npm tarball (it has no waiters) every dependency resolved to the -/// package of `tarball_dependency_id`. +/// `package_name`. So it also fails when the download was for a dependency that +/// `Lockfile::bind_update_requests` would bind it to: a waiter of the task, or, for an npm tarball +/// (it has no waiters), a dependency resolved to the package of `tarball_dependency_id`. fn fail_update_requests( this: &mut PackageManager, task_id: Task::Id, @@ -1934,33 +1934,43 @@ fn fail_update_requests( if this.subcommand == Subcommand::Remove { return; } - let buffers = &this.lockfile.buffers; - let string_buf = buffers.string_bytes.as_slice(); - let dependencies = buffers.dependencies.as_slice(); - let resolutions = buffers.resolutions.as_slice(); + let lockfile = &*this.lockfile; + let string_buf = lockfile.buffers.string_bytes.as_slice(); + let dependencies = lockfile.buffers.dependencies.as_slice(); + let resolutions = lockfile.buffers.resolutions.as_slice(); let waiters = this.task_queue.get(&task_id).map_or(&[][..], Vec::as_slice); let package_id = tarball_dependency_id .and_then(|id| resolutions.get(id as usize).copied()) .filter(|&package_id| package_id != INVALID_PACKAGE_ID); + let was_for = |id: DependencyID| { + package_id.is_some_and(|package_id| resolutions.get(id as usize) == Some(&package_id)) + || waiters.iter().any(|waiter| { + matches!( + waiter, + bun_install::TaskCallbackContext::Dependency(waiting) + | bun_install::TaskCallbackContext::RootDependency(waiting) if *waiting == id + ) + }) + }; + let pending = this.pending_filtered_write.as_deref(); let mut any_failed = false; for request in this.update_requests.iter_mut() { - let waits = waiters.iter().any(|waiter| match waiter { - bun_install::TaskCallbackContext::Dependency(id) - | bun_install::TaskCallbackContext::RootDependency(id) => dependencies - .get(*id as usize) - .is_some_and(|dependency| request.matches(dependency, string_buf)), - _ => false, - }); - let resolved = package_id.is_some_and(|package_id| { - resolutions - .iter() - .zip(dependencies) - .any(|(&resolution, dependency)| { - resolution == package_id && request.matches(dependency, string_buf) + let names_its_dependency = lockfile + .workspaces_of_update_request(pending, this.workspace_name_hash, request) + .into_iter() + .any(|workspace_id| { + let lists = lockfile.packages.items_dependencies(); + lists.get(workspace_id as usize).is_some_and(|list| { + (list.off..list.off + list.len).any(|id| { + dependencies + .get(id as usize) + .is_some_and(|dependency| request.matches(dependency, string_buf)) + && was_for(id) + }) }) - }); - if waits || resolved || strings::eql(request.name, package_name) { + }); + if names_its_dependency || strings::eql(request.name, package_name) { request.failed = true; any_failed = true; } diff --git a/src/install/lockfile.rs b/src/install/lockfile.rs index e753842367a5..4c46bc82500e 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -950,6 +950,19 @@ impl Lockfile { } } + /// The workspaces whose dependency lists `request` names: the ones that received it under `--filter` / `-r`, else the cwd's. + pub(crate) fn workspaces_of_update_request( + &self, + pending: Option<&crate::package_manager_real::add_remove_with_filter::PendingWrite>, + workspace_name_hash: Option, + request: &UpdateRequest, + ) -> Vec { + match pending { + Some(pending) => pending.workspace_ids_receiving(self, request.name_hash), + None => vec![self.get_workspace_package_id(workspace_name_hash)], + } + } + /// Re-runnable: package_json_write_back binds again after re-deriving the declared columns. #[cold] #[inline(never)] @@ -963,19 +976,12 @@ impl Lockfile { let string_buf = self.buffers.string_bytes.as_slice(); let string_buf_ptr = bun_ptr::RawSlice::new(string_buf); let slice = self.packages.slice(); - let cwd_workspace = [self.get_workspace_package_id(workspace_name_hash)]; 'request_updated: for update in updates.iter_mut() { update.e_string = None; - let filtered: Vec; - let workspace_ids: &[PackageID] = match pending { - Some(pending) => { - filtered = pending.workspace_ids_receiving(self, update.name_hash); - &filtered - } - None => &cwd_workspace, - }; - for &workspace_package_id in workspace_ids { + let workspace_ids = + self.workspaces_of_update_request(pending, workspace_name_hash, update); + for &workspace_package_id in &workspace_ids { let dep_list = slice.items_dependencies()[workspace_package_id as usize]; let res_list = slice.items_resolutions()[workspace_package_id as usize]; let workspace_deps: &[Dependency] = diff --git a/test/cli/install/bun-update-transitive.test.ts b/test/cli/install/bun-update-transitive.test.ts index eb42a24553f0..7607ca55ef05 100644 --- a/test/cli/install/bun-update-transitive.test.ts +++ b/test/cli/install/bun-update-transitive.test.ts @@ -1,6 +1,6 @@ import { file, write } from "bun"; import { afterAll, beforeAll, expect, test } from "bun:test"; -import { exists } from "fs/promises"; +import { exists, rm } from "fs/promises"; import { VerdaccioRegistry, bunEnv, bunExe, tempDir } from "harness"; import { join } from "path"; @@ -1960,23 +1960,70 @@ test.concurrent("a 404 for the tarball fails the request for a second alias of t expect(exitCode).toBe(1); }); -test.concurrent.each(["leaf", "aliased@npm:leaf@^1.0.0"])( - "a 404 for the manifest fails `bun add --optional %s` and writes nothing", - async spec => { - const knobs: RegistryKnobs = { status: { leaf: 404 } }; - using server = await serveRegistry(LEAF_ONLY, {}, knobs); - const packageJson = stringify({ name: "foo" }); - const dir = String(tempDir("add-failed-download-", { "package.json": packageJson })); - await servedBunfig(server, dir); - - const { stderr, exitCode } = await run(dir, "add", "--optional", spec); - expect(warningLines(stderr)).toStrictEqual([`warn: ${manifestFailure(server, 404)}`]); - expect(errorLines(stderr)).toStrictEqual([]); - expect(await packageJsonText(dir)).toBe(packageJson); - expect(await exists(join(dir, "bun.lock"))).toBe(false); - expect(exitCode).toBe(1); - }, -); +// The failed download is the only message: a warning for an optional dependency, an error otherwise. A request that is not failed goes on to a second error, " failed to resolve". +test.concurrent.each([ + ["warn", ["--optional", "leaf"]], + ["warn", ["--optional", "aliased@npm:leaf@^1.0.0"]], + ["error", ["leaf"]], + ["error", ["aliased@npm:leaf@^1.0.0"]], +])("a 404 for the manifest is the one %s of `bun add %p`, which writes nothing", async (level, args) => { + const knobs: RegistryKnobs = { status: { leaf: 404 } }; + using server = await serveRegistry(LEAF_ONLY, {}, knobs); + const packageJson = stringify({ name: "foo" }); + const dir = String(tempDir("add-failed-download-", { "package.json": packageJson })); + await servedBunfig(server, dir); + + const { stderr, exitCode } = await run(dir, "add", ...args); + expect([...warningLines(stderr), ...errorLines(stderr)]).toStrictEqual([`${level}: ${manifestFailure(server, 404)}`]); + expect(await packageJsonText(dir)).toBe(packageJson); + expect(await exists(join(dir, "bun.lock"))).toBe(false); + expect(exitCode).toBe(1); +}); + +// A version that bun.lock already holds gets no download while it resolves. The install phase downloads it, after the package.json entry was rewritten in memory. +test.concurrent.each([ + ["a plain entry", "leaf", ""], + ["an npm: alias", "aliased", "npm:leaf@"], +])("a 404 for a tarball that the install phase downloads fails `bun update ` for %s", async (_, key, target) => { + const knobs: RegistryKnobs = { status: {} }; + using server = await serveRegistry(LEAF_ONLY, {}, knobs); + const packageJson = { name: "foo", optionalDependencies: { [key]: `${target}^1.0.0` } }; + const dir = await installServed(server, "update-failed-install-download-", packageJson); + expect(await installedVersion(dir, key)).toBe("1.1.0"); + await Promise.all(["node_modules", ".bun-cache"].map(name => rm(join(dir, name), { recursive: true, force: true }))); + const before = { packageJson: await packageJsonText(dir), lock: await lockText(dir) }; + + knobs.status!["leaf-1.1.0.tgz"] = 404; + const { stderr, exitCode } = await run(dir, "update", key); + expect(warningLines(stderr)).toStrictEqual([`warn: GET ${server.url.origin}/leaf-1.1.0.tgz - 404`]); + expect(errorLines(stderr)).toStrictEqual([]); + expect(await packageJsonText(dir)).toBe(before.packageJson); + expect(await lockText(dir)).toBe(before.lock); + expect(exitCode).toBe(1); +}); + +// pkg2's optional `leaf` is an alias of a package the registry does not have. It has the key of the request, but the request is for pkg1's entry. The alias range must not admit pkg1's range, or bun resolves pkg1's `leaf` through the alias too. +test.concurrent("a failed download for the same key in another workspace does not fail the request", async () => { + using server = await serveRegistry(LEAF_ONLY); + const pkg2 = { name: "pkg2", version: "1.0.0", optionalDependencies: { leaf: "npm:missing@^9.0.0" } }; + const dir = String( + tempDir("update-failed-download-elsewhere-", { + "package.json": stringify(ROOT), + "packages/pkg1/package.json": stringify(member("pkg1", { leaf: "^1.0.0" })), + "packages/pkg2/package.json": stringify(pkg2), + }), + ); + await servedBunfig(server, dir); + + const { stderr, exitCode } = await runIn(dir, "packages/pkg1", "update", "leaf"); + expect(warningLines(stderr)).toStrictEqual([`warn: GET ${server.url.origin}/missing - 404`]); + expect(errorLines(stderr)).toStrictEqual([]); + expect(stderr).toContain("Saved lockfile"); + expect(await packageJsonOf(dir, "packages/pkg1")).toStrictEqual(member("pkg1", { leaf: "^1.1.0" })); + expect(await packageJsonOf(dir, "packages/pkg2")).toStrictEqual(pkg2); + expect(await installedVersion(dir, "leaf")).toBe("1.1.0"); + expect(exitCode).toBe(0); +}); test.concurrent("`bun update ` from a member leaves a sibling's own entry alone but lets it follow", async () => { const { dir, pkg2Text } = await staleMembers("~1.0.0", "^1.0.0"); diff --git a/test/regression/issue/15276.test.ts b/test/regression/issue/15276.test.ts index fcae8b61d250..b0d4c9145c66 100644 --- a/test/regression/issue/15276.test.ts +++ b/test/regression/issue/15276.test.ts @@ -12,6 +12,8 @@ test("parsing npm aliases without package manager does not crash", () => { }); expect(exitCode).toBe(1); - expect(stderr.toString()).toContain("error: bunbunbunbunbun@npm:another-bun@1.0.0 failed to resolve"); + // The 404 fails the request, so it is the only error, as for `bunx another-bun@1.0.0`. + expect(stderr.toString()).toContain("error: GET https://registry.npmjs.org/another-bun - 404"); + expect(stderr.toString()).not.toContain("failed to resolve"); expect(stdout.toString()).toBe(""); }); From 6f98d1b1d7a3416d88f1484152dcae02cad8e6d7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 23:44:27 +0000 Subject: [PATCH 3/5] install: shorten the comment on fail_update_requests --- src/install/PackageManager/runTasks.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index 9ec5aa4f2e5d..3feca04e87af 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -1920,11 +1920,7 @@ pub(crate) fn network_task_has_failed(this: &PackageManager, task_id: Task::Id) .is_some_and(|e| e.failed) } -/// `bun add` / `bun update ` of a package that cannot be fetched exits 1 and saves nothing. -/// A request names its dependency, which an `npm:` alias or an override spells differently from -/// `package_name`. So it also fails when the download was for a dependency that -/// `Lockfile::bind_update_requests` would bind it to: a waiter of the task, or, for an npm tarball -/// (it has no waiters), a dependency resolved to the package of `tarball_dependency_id`. +/// `bun add` / `bun update ` exits 1 and saves nothing when a download for the request fails. fn fail_update_requests( this: &mut PackageManager, task_id: Task::Id, @@ -1939,6 +1935,7 @@ fn fail_update_requests( let dependencies = lockfile.buffers.dependencies.as_slice(); let resolutions = lockfile.buffers.resolutions.as_slice(); let waiters = this.task_queue.get(&task_id).map_or(&[][..], Vec::as_slice); + // An npm tarball task has no waiters: the dependencies it was for already resolved to its package. let package_id = tarball_dependency_id .and_then(|id| resolutions.get(id as usize).copied()) .filter(|&package_id| package_id != INVALID_PACKAGE_ID); @@ -1956,6 +1953,7 @@ fn fail_update_requests( let mut any_failed = false; for request in this.update_requests.iter_mut() { + // A request names a package.json key, which an `npm:` alias or an override spells differently from `package_name`. let names_its_dependency = lockfile .workspaces_of_update_request(pending, this.workspace_name_hash, request) .into_iter() From caef1ce8f6c7bb9c5090aae4f7e19d9b69d14927 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 23:53:44 +0000 Subject: [PATCH 4/5] test: assert the exit code of 15276 after its output --- test/regression/issue/15276.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/regression/issue/15276.test.ts b/test/regression/issue/15276.test.ts index b0d4c9145c66..12f74c2cbab5 100644 --- a/test/regression/issue/15276.test.ts +++ b/test/regression/issue/15276.test.ts @@ -11,9 +11,9 @@ test("parsing npm aliases without package manager does not crash", () => { env: bunEnv, }); - expect(exitCode).toBe(1); // The 404 fails the request, so it is the only error, as for `bunx another-bun@1.0.0`. expect(stderr.toString()).toContain("error: GET https://registry.npmjs.org/another-bun - 404"); expect(stderr.toString()).not.toContain("failed to resolve"); expect(stdout.toString()).toBe(""); + expect(exitCode).toBe(1); }); From 2a8782a225b3b425206e9c5f697d4a0380291c12 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 18 Sep 2026 00:00:23 +0000 Subject: [PATCH 5/5] test: serve the 404 of 15276 from a local registry --- test/regression/issue/15276.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/test/regression/issue/15276.test.ts b/test/regression/issue/15276.test.ts index 12f74c2cbab5..0fff469fc878 100644 --- a/test/regression/issue/15276.test.ts +++ b/test/regression/issue/15276.test.ts @@ -1,19 +1,21 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe } from "harness"; -test("parsing npm aliases without package manager does not crash", () => { +test("parsing npm aliases without package manager does not crash", async () => { // Easiest way to repro this regression with `bunx bunbunbunbunbun@npm:another-bun@1.0.0`. The package // doesn't need to exist, we just need `bunx` to parse the package version. - const { stdout, stderr, exitCode } = Bun.spawnSync({ + using registry = Bun.serve({ port: 0, fetch: () => new Response("{}", { status: 404 }) }); + await using proc = Bun.spawn({ cmd: [bunExe(), "x", "bunbunbunbunbun@npm:another-bun@1.0.0"], stdout: "pipe", stderr: "pipe", - env: bunEnv, + env: { ...bunEnv, BUN_CONFIG_REGISTRY: registry.url.href }, }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); // The 404 fails the request, so it is the only error, as for `bunx another-bun@1.0.0`. - expect(stderr.toString()).toContain("error: GET https://registry.npmjs.org/another-bun - 404"); - expect(stderr.toString()).not.toContain("failed to resolve"); - expect(stdout.toString()).toBe(""); + expect(stderr).toContain(`error: GET ${registry.url.href}another-bun - 404`); + expect(stderr).not.toContain("failed to resolve"); + expect(stdout).toBe(""); expect(exitCode).toBe(1); });