install: fail bun outdated and bun update -i when a dependency's manifest cannot be fetched

src/install/PackageManager/PopulateManifestCache.rs

@@ -2,13 +2,11 @@ use crate::lockfile::package::PackageColumns as _;
 use bun_collections::HashMap;
 use bun_core::Output;
 
-use crate::Dependency;
 use crate::DependencyID;
 use crate::ManifestLoad;
 use crate::NetworkTask;
 use crate::PackageID;
 use crate::Resolution;
-use crate::dependency::Behavior;
 use crate::invalid_package_id;
 // Import the
 // *module* under the `Task` name so `Task::Id` resolves as a path (matches
@@ -44,21 +42,15 @@ impl From<StartManifestTaskError> for crate::Error {
     }
 }
 
+/// `is_required`: a failed fetch is logged as an error rather than a warning.
 fn start_manifest_task(
     manager: &mut PackageManager,
     pkg_name: &[u8],
-    dep: &Dependency,
+    is_required: bool,
     needs_extended_manifest: bool,
 ) -> Result<(), StartManifestTaskError> {
     let task_id = Task::Id::for_manifest(pkg_name);
-    // Read the *raw* OPTIONAL bit
-    // — not `Behavior.isOptional()` (which is `optional && !peer`). For
-    // optional-peer deps the raw bit is `true` but `is_optional()` is `false`,
-    // which would flip both the dedupe-map `is_required` bookkeeping and
-    // `for_manifest`'s error-suppression branch. Mirror runTasks.rs and read
-    // the raw flag.
-    let is_optional = dep.behavior.contains(Behavior::OPTIONAL);
-    if run_tasks::has_created_network_task(manager, task_id, is_optional) {
+    if run_tasks::has_created_network_task(manager, task_id, is_required) {
         return Ok(());
     }
     if manager.options.log_level.show_progress() {
@@ -91,7 +83,7 @@ fn start_manifest_task(
         pkg_name,
         scope.get(),
         None,
-        is_optional,
+        !is_required,
         needs_extended_manifest,
     )?;
 
@@ -101,9 +93,11 @@ fn start_manifest_task(
 
 #[derive(Clone, Copy)]
 pub enum Packages<'a> {
+    /// Every npm package in the lockfile; best-effort (the post-migration backfill), so failures are warnings.
     All,
+    /// The direct dependencies of these workspace packages; a required one failing is an error, see [`print_fetch_failures`].
     Ids(&'a [PackageID]),
-    /// The manifests of these packages themselves (by name), not of their dependencies.
+    /// The manifests of these packages themselves (by name), not of their dependencies; best-effort, so failures are warnings.
     Exact(&'a [PackageID]),
 }
 
@@ -157,7 +151,7 @@ pub fn populate_manifest_cache(
         Packages::All => {
             let mut seen_pkg_ids: HashMap<PackageID, ()> = HashMap::new();
 
-            for (_dep_id, dep) in dependencies.iter().enumerate() {
+            for _dep_id in 0..dependencies.len() {
                 let dep_id: DependencyID = DependencyID::try_from(_dep_id).expect("int cast");
 
                 let pkg_id = resolutions[dep_id as usize];
@@ -203,10 +197,10 @@ pub fn populate_manifest_cache(
                         // `start_manifest_task` only touches the network-task
                         // pool / progress bar / log, never `lockfile.buffers`
                         // or `lockfile.packages`, so the outstanding shared
-                        // slices (`pkg_name_slice`, `dep`) stay valid.
+                        // slice (`pkg_name_slice`) stays valid.
                         unsafe { &mut *manager_ptr },
                         pkg_name_slice,
-                        dep,
+                        false,
                         needs_extended_manifest,
                     )?;
                 }
@@ -258,10 +252,10 @@ pub fn populate_manifest_cache(
                             // root; `start_manifest_task` only touches the
                             // network-task pool / progress bar / log, never
                             // `lockfile.buffers` or `lockfile.packages`, so
-                            // `package_name` / `dep` stay valid.
+                            // `package_name` stays valid.
                             unsafe { &mut *manager_ptr },
                             package_name,
-                            dep,
+                            dep.behavior.is_required(),
                             needs_extended_manifest,
                         )?;
 
@@ -274,7 +268,6 @@ pub fn populate_manifest_cache(
             }
         }
         Packages::Exact(ids) => {
-            let placeholder = Dependency::default();
             for &pkg_id in ids {
                 if pkg_resolutions[pkg_id as usize].tag != ResolutionTag::Npm {
                     continue;
@@ -296,7 +289,7 @@ pub fn populate_manifest_cache(
                         // SAFETY: SRW root; `start_manifest_task` never mutates `lockfile`, so `package_name` stays valid.
                         unsafe { &mut *manager_ptr },
                         package_name,
-                        &placeholder,
+                        false,
                         needs_extended_manifest,
                     )?;
                     // SAFETY: SRW root; network-queue flush does not mutate `lockfile`.
@@ -371,3 +364,15 @@ pub fn populate_manifest_cache(
 
     Ok(())
 }
+
+/// Prints the fetch failures a [`Packages::Ids`] pass logged; true when one of them is a required dependency's.
+pub fn print_fetch_failures(manager: &PackageManager) -> crate::Result<bool> {
+    let log = manager.log_mut();
+    let failed_required = log.has_errors();
+    if !log.msgs.is_empty() {
+        Output::flush();
+        log.print(core::ptr::from_mut(Output::error_writer()))?;
+        log.reset();
+    }
+    Ok(failed_required)
+}

src/install/lockfile.rs

@@ -1365,6 +1365,8 @@ impl Lockfile {
         {
             return Ok(());
         }
+        // Otherwise the command loading this lockfile would dedupe its own fetch of a manifest this pass failed to get.
+        manager.network_dedupe_map.clear();
 
         let cache_ctx = manager.manifest_disk_cache_ctx();
         // `manifests` is a field of `manager`, and a `string_builder` is

src/runtime/cli/outdated_command.rs

@@ -164,31 +164,37 @@ impl OutdatedCommand {
         original_cwd: &[u8],
         manager: &mut PackageManager,
     ) -> crate::Result<()> {
-        if !manager.options.filter_patterns.is_empty() || manager.options.do_.recursive() {
-            let workspace_pkg_ids = WorkspaceFilter::select_workspaces(
-                &manager.lockfile,
-                manager.options.filter_patterns,
-                original_cwd,
-            );
-            populate_manifest_cache::populate_manifest_cache(
-                manager,
-                populate_manifest_cache::Packages::Ids(&workspace_pkg_ids),
-            )?;
-            Self::print_outdated_info_table::<ENABLE_ANSI_COLORS>(manager, &workspace_pkg_ids, true)
-        } else {
-            let root_pkg_id = manager
-                .root_package_id
-                .get(&manager.lockfile, manager.workspace_name_hash);
-            if root_pkg_id == bun_install::INVALID_PACKAGE_ID {
-                return Ok(());
-            }
-            let ids = [root_pkg_id];
-            populate_manifest_cache::populate_manifest_cache(
-                manager,
-                populate_manifest_cache::Packages::Ids(&ids),
-            )?;
-            Self::print_outdated_info_table::<ENABLE_ANSI_COLORS>(manager, &ids, false)
+        let (workspace_pkg_ids, was_filtered) =
+            if !manager.options.filter_patterns.is_empty() || manager.options.do_.recursive() {
+                let ids = WorkspaceFilter::select_workspaces(
+                    &manager.lockfile,
+                    manager.options.filter_patterns,
+                    original_cwd,
+                );
+                (ids, true)
+            } else {
+                let root_pkg_id = manager
+                    .root_package_id
+                    .get(&manager.lockfile, manager.workspace_name_hash);
+                if root_pkg_id == bun_install::INVALID_PACKAGE_ID {
+                    return Ok(());
+                }
+                (vec![root_pkg_id], false)
+            };
+        populate_manifest_cache::populate_manifest_cache(
+            manager,
+            populate_manifest_cache::Packages::Ids(&workspace_pkg_ids),
+        )?;
+        // The table covers what could be checked; what could not follows it.
+        Self::print_outdated_info_table::<ENABLE_ANSI_COLORS>(
+            manager,
+            &workspace_pkg_ids,
+            was_filtered,
+        )?;
+        if populate_manifest_cache::print_fetch_failures(manager)? {
+            Global::crash();
         }
+        Ok(())
     }
 
     fn group_catalog_dependencies(

src/runtime/cli/update_interactive_command.rs

@@ -575,6 +575,9 @@ impl UpdateInteractiveCommand {
             manager,
             populate_manifest_cache::Packages::Ids(&workspace_pkg_ids),
         )?;
+        if populate_manifest_cache::print_fetch_failures(manager)? {
+            Global::crash();
+        }
 
         // Get outdated packages
         let (mut outdated_packages, checked) =

test/cli/install/bun-update-transitive.test.ts

@@ -1779,6 +1779,87 @@ test.concurrent("`bun update --silent` swallows the unfetchable-manifest warning
   expect(exitCode).toBe(0);
 });
 
+// `bun outdated` and `bun update -i` exist to answer for the direct dependencies, so for them a manifest that does not arrive is reported and fails the command (a registry that is down must not read as "nothing to update"); only an optional dependency's is a warning.
+const errorLines = (stderr: string) => stderr.split("\n").filter(line => line.startsWith("error:"));
+const manifestFailure = (server: Bun.Server, status: number) => `GET ${server.url.origin}/leaf - ${status}`;
+
+// leaf@1.0.0 installed while 1.1.0 exists, with the manifest cache off so every later command asks the registry again.
+async function staleDirectLeaf(server: Bun.Server, groups: Groups = {}) {
+  const dir = await installServed(server, "direct-manifest-failure-", grouped({ leaf: "1.0.0" }, groups));
+  await servedBunfig(server, dir, { cache: false });
+  return dir;
+}
+
+// Nothing but the header: no table, and no "up to date" either.
+const expectHeaderOnly = (stdout: string, command: string) =>
+  expect(stdout).toMatch(new RegExp(`^bun ${command} v[^\\n]*\\n$`));
+
+test.concurrent.each<[number, string[]]>([
+  [502, []],
+  [404, []],
+  [502, ["-r"]],
+])("a %d for a direct dependency's manifest fails `bun outdated` (extra args: %p)", async (status, args) => {
+  const knobs: RegistryKnobs = { status: {} };
+  using server = await serveRegistry(TAGGED_FREE, {}, knobs);
+  const dir = await staleDirectLeaf(server);
+
+  const healthy = await run(dir, "outdated", ...args);
+  expect(healthy.stdout).toMatch(/\| leaf\s+\| 1\.0\.0\s+\| 1\.0\.0\s+\| 1\.1\.0\s+\|/);
+  expectCleanStderr(healthy.stderr);
+  expect(healthy.exitCode).toBe(0);
+
+  knobs.status!.leaf = status;
+  const { stdout, stderr, exitCode } = await run(dir, "outdated", ...args);
+  expectHeaderOnly(stdout, "outdated");
+  expect(errorLines(stderr)).toStrictEqual([`error: ${manifestFailure(server, status)}`]);
+  expect(exitCode).toBe(1);
+});
+
+test.concurrent("a registry that does not answer at all fails `bun outdated`", async () => {
+  using server = await serveRegistry(TAGGED_FREE);
+  const dir = await staleDirectLeaf(server);
+  server.stop(true);
+  const { stdout, stderr, exitCode } = await run(dir, "outdated");
+  expectHeaderOnly(stdout, "outdated");
+  expect(errorLines(stderr)).toStrictEqual([expect.stringMatching(/^error: \w+ downloading package manifest leaf$/)]);
+  expect(exitCode).toBe(1);
+});
+
+test.concurrent(
+  "an optional dependency whose manifest cannot be fetched is only warned about by `bun outdated`",
+  async () => {
+    const knobs: RegistryKnobs = { status: {} };
+    using server = await serveRegistry(TAGGED_FREE, {}, knobs);
+    const dir = await staleDirectLeaf(server, { leaf: "optionalDependencies" });
+    knobs.status!.leaf = 502;
+    const { stdout, stderr, exitCode } = await run(dir, "outdated");
+    expectHeaderOnly(stdout, "outdated");
+    expectWarnings(stderr, `warn: ${manifestFailure(server, 502)}\n`);
+    expect(exitCode).toBe(0);
+  },
+);
+
+test.concurrent(
+  "a 502 for a direct dependency's manifest fails `bun update -i` instead of offering nothing",
+  async () => {
+    const knobs: RegistryKnobs = { status: {} };
+    using server = await serveRegistry(TAGGED_FREE, {}, knobs);
+    const dir = await staleDirectLeaf(server);
+    const before = await lockText(dir);
+
+    const healthy = await runInteractive(dir, "", "--dry-run");
+    expect(healthy.stdout).toContain("leaf");
+    expect(healthy.exitCode).toBe(0);
+
+    knobs.status!.leaf = 502;
+    const { stdout, stderr, exitCode } = await runInteractive(dir, "", "--dry-run");
+    expectHeaderOnly(stdout, "update --interactive");
+    expect(errorLines(stderr)).toStrictEqual([`error: ${manifestFailure(server, 502)}`]);
+    expect(await lockText(dir)).toBe(before);
+    expect(exitCode).toBe(1);
+  },
+);
+
 test.concurrent("`bun update <name>` 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");

test/cli/install/migration/pnpm-lock-v9.test.ts

@@ -2366,6 +2366,81 @@ snapshots:
     });
   });
 
+  // The manifests fetched right after a migration only backfill bins and os/cpu, so one the registry does not serve is a warning whichever group the package is in; what fails the install is a required package's own tarball.
+  describe("manifests fetched after the migration", () => {
+    const lockfile = (group: string) => `lockfileVersion: '9.0'
+
+importers:
+
+  .:
+    ${group}:
+      no-deps:
+        specifier: 1.0.1
+        version: 1.0.1
+
+packages:
+
+  no-deps@1.0.1:
+    resolution: {integrity: ${NO_DEPS_1_0_1_INTEGRITY}}
+
+snapshots:
+
+  no-deps@1.0.1: {}
+`;
+
+    const goneRegistry = () => Bun.serve({ port: 0, fetch: () => new Response("gone", { status: 404 }) });
+    const project = (registry: Bun.Server, group: string) =>
+      tempDir("pnpm-v9-manifest-gone", {
+        "package.json": JSON.stringify({ name: "manifest-gone", [group]: { "no-deps": "1.0.1" } }),
+        "bunfig.toml": `[install]\nregistry = "${registry.url.href}"\n`,
+        "pnpm-lock.yaml": lockfile(group),
+      });
+    const fetchLines = (stderr: string) =>
+      stderr.split("\n").filter(line => line.startsWith("warn:") || line.startsWith("error:"));
+
+    test.concurrent.each([
+      ["optionalDependencies", "warn", 0],
+      ["dependencies", "error", 1],
+    ])(
+      "installing when the registry no longer serves the %s entry: the manifest line is a warning, the tarball line says %s",
+      async (group, tarballSeverity, expectedExitCode) => {
+        using registry = goneRegistry();
+        using dir = project(registry, group);
+
+        const { stdout, stderr, exitCode } = await run(String(dir), "install");
+
+        expect(stdout).toStartWith("bun install v");
+        expect(fetchLines(stderr)).toStrictEqual([
+          `warn: GET ${registry.url.origin}/no-deps - 404`,
+          `${tarballSeverity}: GET ${registry.url.origin}/no-deps/-/no-deps-1.0.1.tgz - 404`,
+        ]);
+        expect(existsSync(join(String(dir), "bun.lock"))).toBe(expectedExitCode === 0);
+        expect(exitCode).toBe(expectedExitCode);
+      },
+    );
+
+    // The command's own fetch must not be deduplicated against the migration's failed one, or the failure would stay a warning and the command would exit 0.
+    test.concurrent.each<[string, string[]]>([
+      ["outdated", ["outdated"]],
+      ["update --interactive", ["update", "-i", "--dry-run"]],
+    ])(
+      "`bun %s` run straight off the pnpm lockfile still fails on the manifest it cannot get",
+      async (header, args) => {
+        using registry = goneRegistry();
+        using dir = project(registry, "dependencies");
+
+        const { stdout, stderr, exitCode } = await run(String(dir), ...args);
+
+        expect(stdout).toMatch(new RegExp(`^bun ${header} v[^\\n]*\\n$`));
+        expect(fetchLines(stderr)).toStrictEqual([
+          `warn: GET ${registry.url.origin}/no-deps - 404`,
+          `error: GET ${registry.url.origin}/no-deps - 404`,
+        ]);
+        expect(exitCode).toBe(1);
+      },
+    );
+  });
+
   describe("git sub-directory dependencies", () => {
     // pnpm/pnpm#8243: `repo#commit&path:sub/dir` has no bun equivalent; refuse instead of installing the repo root
     test.concurrent("type: git resolution with path: is rejected naming the package", async () => {