use `bun.logger` instead of `bun.Output` (#2099)

4 files changed, 181 insertions, 99 deletions

diff --git a/src/install/extract_tarball.zig b/src/install/extract_tarball.zig
index 1b55d17e1..0df11ffcd 100644
--- a/src/install/extract_tarball.zig
+++ b/src/install/extract_tarball.zig
@@ -31,8 +31,13 @@ package_manager: *PackageManager,
 pub inline fn run(this: ExtractTarball, bytes: []const u8) !Install.ExtractData {
     if (!this.skip_verify and this.integrity.tag.isSupported()) {
         if (!this.integrity.verify(bytes)) {
-            Output.prettyErrorln("<r><red>Integrity check failed<r> for tarball: {s}", .{this.name.slice()});
-            Output.flush();
+            this.package_manager.log.addErrorFmt(
+                null,
+                logger.Loc.Empty,
+                this.package_manager.allocator,
+                "Integrity check failed<r> for tarball: {s}",
+                .{this.name.slice()},
+            ) catch unreachable;
             return error.IntegrityCheckFailed;
         }
     }
@@ -165,7 +170,14 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
     var tmpname = try FileSystem.instance.tmpname(basename[0..@min(basename.len, 32)], &tmpname_buf, tgz_bytes.len);
     {
         var extract_destination = tmpdir.makeOpenPathIterable(std.mem.span(tmpname), .{}) catch |err| {
-            Output.panic("err: {s} when create temporary directory named {s} (while extracting {s})", .{ @errorName(err), tmpname, name });
+            this.package_manager.log.addErrorFmt(
+                null,
+                logger.Loc.Empty,
+                this.package_manager.allocator,
+                "{s} when create temporary directory named \"{s}\" (while extracting \"{s}\")",
+                .{ @errorName(err), tmpname, name },
+            ) catch unreachable;
+            return error.InstallFailed;
         };
 
         defer extract_destination.close();
@@ -183,14 +195,14 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
 
         var zlib_entry = try Zlib.ZlibReaderArrayList.init(tgz_bytes, &zlib_pool.data.list, default_allocator);
         zlib_entry.readAll() catch |err| {
-            Output.prettyErrorln(
-                "<r><red>Error {s}<r> decompressing {s}",
-                .{
-                    @errorName(err),
-                    name,
-                },
-            );
-            Global.crash();
+            this.package_manager.log.addErrorFmt(
+                null,
+                logger.Loc.Empty,
+                this.package_manager.allocator,
+                "{s} decompressing \"{s}\"",
+                .{ @errorName(err), name },
+            ) catch unreachable;
+            return error.InstallFailed;
         };
         switch (this.resolution.tag) {
             .github => {
@@ -270,12 +282,7 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
         }
 
         if (PackageManager.verbose_install) {
-            Output.prettyErrorln(
-                "[{s}] Extracted<r>",
-                .{
-                    name,
-                },
-            );
+            Output.prettyErrorln("[{s}] Extracted<r>", .{name});
             Output.flush();
         }
     }
@@ -296,29 +303,27 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
 
     // Now that we've extracted the archive, we rename.
     std.os.renameatZ(tmpdir.fd, tmpname, cache_dir.fd, folder_name) catch |err| {
-        Output.prettyErrorln(
-            "<r><red>Error {s}<r> moving {s} to cache dir:\n   From: {s}    To: {s}",
-            .{
-                @errorName(err),
-                name,
-                tmpname,
-                folder_name,
-            },
-        );
-        Global.crash();
+        this.package_manager.log.addErrorFmt(
+            null,
+            logger.Loc.Empty,
+            this.package_manager.allocator,
+            "moving \"{s}\" to cache dir failed: {s}\n  From: {s}\n    To: {s}",
+            .{ name, @errorName(err), tmpname, folder_name },
+        ) catch unreachable;
+        return error.InstallFailed;
     };
 
     // We return a resolved absolute absolute file path to the cache dir.
     // To get that directory, we open the directory again.
     var final_dir = cache_dir.openDirZ(folder_name, .{}, true) catch |err| {
-        Output.prettyErrorln(
-            "<r><red>Error {s}<r> failed to verify cache dir for {s}",
-            .{
-                @errorName(err),
-                name,
-            },
-        );
-        Global.crash();
+        this.package_manager.log.addErrorFmt(
+            null,
+            logger.Loc.Empty,
+            this.package_manager.allocator,
+            "failed to verify cache dir for \"{s}\": {s}",
+            .{ name, @errorName(err) },
+        ) catch unreachable;
+        return error.InstallFailed;
     };
 
     defer final_dir.close();
@@ -327,14 +332,14 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
         final_dir.fd,
         &final_path_buf,
     ) catch |err| {
-        Output.prettyErrorln(
-            "<r><red>Error {s}<r> failed to verify cache dir for {s}",
-            .{
-                @errorName(err),
-                name,
-            },
-        );
-        Global.crash();
+        this.package_manager.log.addErrorFmt(
+            null,
+            logger.Loc.Empty,
+            this.package_manager.allocator,
+            "failed to resolve cache dir for \"{s}\": {s}",
+            .{ name, @errorName(err) },
+        ) catch unreachable;
+        return error.InstallFailed;
     };
 
     // create an index storing each version of a package installed
@@ -359,11 +364,14 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
     switch (this.resolution.tag) {
         .github => {
             const json_file = final_dir.openFileZ("package.json", .{ .mode = .read_only }) catch |err| {
-                Output.prettyErrorln("<r><red>Error {s}<r> failed to open package.json for {s}", .{
-                    @errorName(err),
-                    name,
-                });
-                Global.crash();
+                this.package_manager.log.addErrorFmt(
+                    null,
+                    logger.Loc.Empty,
+                    this.package_manager.allocator,
+                    "\"package.json\" for \"{s}\" failed to open: {s}",
+                    .{ name, @errorName(err) },
+                ) catch unreachable;
+                return error.InstallFailed;
             };
             defer json_file.close();
             const json_stat = try json_file.stat();
@@ -374,14 +382,14 @@ fn extract(this: *const ExtractTarball, tgz_bytes: []const u8) !Install.ExtractD
                 json_file.handle,
                 &json_path_buf,
             ) catch |err| {
-                Output.prettyErrorln(
-                    "<r><red>Error {s}<r> failed to open package.json for {s}",
-                    .{
-                        @errorName(err),
-                        name,
-                    },
-                );
-                Global.crash();
+                this.package_manager.log.addErrorFmt(
+                    null,
+                    logger.Loc.Empty,
+                    this.package_manager.allocator,
+                    "\"package.json\" for \"{s}\" failed to resolve: {s}",
+                    .{ name, @errorName(err) },
+                ) catch unreachable;
+                return error.InstallFailed;
             };
             // TODO remove extracted files not matching any globs under "files"
         },
diff --git a/src/install/install.zig b/src/install/install.zig
index 064b79a2c..e0e509f16 100644
--- a/src/install/install.zig
+++ b/src/install/install.zig
@@ -599,10 +599,12 @@ const Task = struct {
                 this.package_manager.resolve_tasks.writeItem(this.*) catch unreachable;
             },
             .git_clone => {
+                const manager = this.package_manager;
                 const dir = Repository.download(
-                    this.package_manager.allocator,
-                    this.package_manager.env,
-                    this.package_manager.getCacheDirectory().dir,
+                    manager.allocator,
+                    manager.env,
+                    manager.log,
+                    manager.getCacheDirectory().dir,
                     this.id,
                     this.request.git_clone.name.slice(),
                     this.request.git_clone.url.slice(),
@@ -610,22 +612,24 @@ const Task = struct {
                     this.err = err;
                     this.status = Status.fail;
                     this.data = .{ .git_clone = std.math.maxInt(std.os.fd_t) };
-                    this.package_manager.resolve_tasks.writeItem(this.*) catch unreachable;
+                    manager.resolve_tasks.writeItem(this.*) catch unreachable;
                     return;
                 };
 
-                this.package_manager.git_repositories.put(this.package_manager.allocator, this.id, dir.fd) catch unreachable;
+                manager.git_repositories.put(manager.allocator, this.id, dir.fd) catch unreachable;
                 this.data = .{
                     .git_clone = dir.fd,
                 };
                 this.status = Status.success;
-                this.package_manager.resolve_tasks.writeItem(this.*) catch unreachable;
+                manager.resolve_tasks.writeItem(this.*) catch unreachable;
             },
             .git_checkout => {
+                const manager = this.package_manager;
                 const data = Repository.checkout(
-                    this.package_manager.allocator,
-                    this.package_manager.env,
-                    this.package_manager.getCacheDirectory().dir,
+                    manager.allocator,
+                    manager.env,
+                    manager.log,
+                    manager.getCacheDirectory().dir,
                     .{ .fd = this.request.git_checkout.repo_dir },
                     this.request.git_checkout.name.slice(),
                     this.request.git_checkout.url.slice(),
@@ -634,7 +638,7 @@ const Task = struct {
                     this.err = err;
                     this.status = Status.fail;
                     this.data = .{ .git_checkout = .{} };
-                    this.package_manager.resolve_tasks.writeItem(this.*) catch unreachable;
+                    manager.resolve_tasks.writeItem(this.*) catch unreachable;
                     return;
                 };
 
@@ -642,7 +646,7 @@ const Task = struct {
                     .git_checkout = data,
                 };
                 this.status = Status.success;
-                this.package_manager.resolve_tasks.writeItem(this.*) catch unreachable;
+                manager.resolve_tasks.writeItem(this.*) catch unreachable;
             },
         }
     }
@@ -2919,6 +2923,7 @@ pub const PackageManager = struct {
                     const resolved = try Repository.findCommit(
                         this.allocator,
                         this.env,
+                        this.log,
                         .{ .fd = repo_fd },
                         alias,
                         this.lockfile.str(&dep.committish),
diff --git a/src/install/repository.zig b/src/install/repository.zig
index ecc084b25..a1a8396c2 100644
--- a/src/install/repository.zig
+++ b/src/install/repository.zig
@@ -1,6 +1,6 @@
 const bun = @import("bun");
 const Global = bun.Global;
-const Output = bun.Output;
+const logger = bun.logger;
 const DotEnv = @import("../env_loader.zig");
 const Environment = @import("../env.zig");
 const FileSystem = @import("../fs.zig").FileSystem;
@@ -119,6 +119,7 @@ pub const Repository = extern struct {
     pub fn download(
         allocator: std.mem.Allocator,
         env: *DotEnv.Loader,
+        log: *logger.Log,
         cache_dir: std.fs.Dir,
         task_id: u64,
         name: string,
@@ -130,9 +131,13 @@ pub const Repository = extern struct {
 
         return if (cache_dir.openDirZ(folder_name, .{}, true)) |dir| fetch: {
             _ = exec(allocator, env, dir, &[_]string{ "git", "fetch", "--quiet" }) catch |err| {
-                Output.prettyErrorln("<r><red>Error<r> \"git fetch\" failed for {s}", .{
-                    name,
-                });
+                log.addErrorFmt(
+                    null,
+                    logger.Loc.Empty,
+                    allocator,
+                    "\"git fetch\" for \"{s}\" failed",
+                    .{name},
+                ) catch unreachable;
                 return err;
             };
             break :fetch dir;
@@ -147,16 +152,27 @@ pub const Repository = extern struct {
                 url,
                 folder_name,
             }) catch |err| {
-                Output.prettyErrorln("<r><red>Error<r> \"git clone\" failed for {s}", .{
-                    name,
-                });
+                log.addErrorFmt(
+                    null,
+                    logger.Loc.Empty,
+                    allocator,
+                    "\"git clone\" for \"{s}\" failed",
+                    .{name},
+                ) catch unreachable;
                 return err;
             };
             break :clone try cache_dir.openDirZ(folder_name, .{}, true);
         };
     }
 
-    pub fn findCommit(allocator: std.mem.Allocator, env: *DotEnv.Loader, repo_dir: std.fs.Dir, name: string, committish: string) !string {
+    pub fn findCommit(
+        allocator: std.mem.Allocator,
+        env: *DotEnv.Loader,
+        log: *logger.Log,
+        repo_dir: std.fs.Dir,
+        name: string,
+        committish: string,
+    ) !string {
         return std.mem.trim(u8, exec(
             allocator,
             env,
@@ -166,10 +182,13 @@ pub const Repository = extern struct {
             else
                 &[_]string{ "git", "log", "--format=%H", "-1" },
         ) catch |err| {
-            Output.prettyErrorln("\n<r><red>Error<r> no commit matching \"{s}\" found for \"{s}\" (but repository exists)", .{
-                committish,
-                name,
-            });
+            log.addErrorFmt(
+                null,
+                logger.Loc.Empty,
+                allocator,
+                "no commit matching \"{s}\" found for \"{s}\" (but repository exists)",
+                .{ committish, name },
+            ) catch unreachable;
             return err;
         }, " \t\r\n");
     }
@@ -177,6 +196,7 @@ pub const Repository = extern struct {
     pub fn checkout(
         allocator: std.mem.Allocator,
         env: *DotEnv.Loader,
+        log: *logger.Log,
         cache_dir: std.fs.Dir,
         repo_dir: std.fs.Dir,
         name: string,
@@ -196,18 +216,26 @@ pub const Repository = extern struct {
                 try bun.getFdPath(repo_dir.fd, &final_path_buf),
                 folder_name,
             }) catch |err| {
-                Output.prettyErrorln("<r><red>Error<r> \"git clone\" failed for {s}", .{
-                    name,
-                });
+                log.addErrorFmt(
+                    null,
+                    logger.Loc.Empty,
+                    allocator,
+                    "\"git clone\" for \"{s}\" failed",
+                    .{name},
+                ) catch unreachable;
                 return err;
             };
 
             var dir = try cache_dir.openDirZ(folder_name, .{}, true);
 
             _ = exec(allocator, env, dir, &[_]string{ "git", "checkout", "--quiet", resolved }) catch |err| {
-                Output.prettyErrorln("<r><red>Error<r> \"git checkout\" failed for {s}", .{
-                    name,
-                });
+                log.addErrorFmt(
+                    null,
+                    logger.Loc.Empty,
+                    allocator,
+                    "\"git checkout\" for \"{s}\" failed",
+                    .{name},
+                ) catch unreachable;
                 return err;
             };
             dir.deleteTree(".git") catch {};
@@ -225,11 +253,14 @@ pub const Repository = extern struct {
         defer package_dir.close();
 
         const json_file = package_dir.openFileZ("package.json", .{ .mode = .read_only }) catch |err| {
-            Output.prettyErrorln("<r><red>Error {s}<r> failed to open package.json for {s}", .{
-                @errorName(err),
-                name,
-            });
-            Global.crash();
+            log.addErrorFmt(
+                null,
+                logger.Loc.Empty,
+                allocator,
+                "\"package.json\" for \"{s}\" failed to open: {s}",
+                .{ name, @errorName(err) },
+            ) catch unreachable;
+            return error.InstallFailed;
         };
         defer json_file.close();
         const json_stat = try json_file.stat();
@@ -240,14 +271,14 @@ pub const Repository = extern struct {
             json_file.handle,
             &json_path_buf,
         ) catch |err| {
-            Output.prettyErrorln(
-                "<r><red>Error {s}<r> failed to open package.json for {s}",
-                .{
-                    @errorName(err),
-                    name,
-                },
-            );
-            Global.crash();
+            log.addErrorFmt(
+                null,
+                logger.Loc.Empty,
+                allocator,
+                "\"package.json\" for \"{s}\" failed to resolve: {s}",
+                .{ name, @errorName(err) },
+            ) catch unreachable;
+            return error.InstallFailed;
         };
 
         const ret_json_path = try FileSystem.instance.dirname_store.append(@TypeOf(json_path), json_path);
diff --git a/test/bun.js/install/bun-install.test.ts b/test/bun.js/install/bun-install.test.ts
index f2535244d..360ab41ae 100644
--- a/test/bun.js/install/bun-install.test.ts
+++ b/test/bun.js/install/bun-install.test.ts
@@ -2200,6 +2200,44 @@ it("should handle Git URL with committish in dependencies", async () => {
   await access(join(package_dir, "bun.lockb"));
 });
 
+it("should fail on invalid Git URL", async () => {
+  const urls: string[] = [];
+  setHandler(dummyRegistry(urls));
+  await writeFile(
+    join(package_dir, "package.json"),
+    JSON.stringify({
+      name: "Foo",
+      version: "0.0.1",
+      dependencies: {
+        uglify: "git+http://bun.sh/no_such_repo",
+      },
+    }),
+  );
+  const { stdout, stderr, exited } = spawn({
+    cmd: [bunExe(), "install", "--config", import.meta.dir + "/basic.toml"],
+    cwd: package_dir,
+    stdout: null,
+    stdin: "pipe",
+    stderr: "pipe",
+    env,
+  });
+  expect(stderr).toBeDefined();
+  const err = await new Response(stderr).text();
+  expect(err.split(/\r?\n/)).toContain('error: "git clone" for "uglify" failed');
+  expect(stdout).toBeDefined();
+  const out = await new Response(stdout).text();
+  expect(out).toBe("");
+  expect(await exited).toBe(1);
+  expect(urls.sort()).toEqual([]);
+  expect(requested).toBe(0);
+  try {
+    await access(join(package_dir, "bun.lockb"));
+    expect(() => {}).toThrow();
+  } catch (err: any) {
+    expect(err.code).toBe("ENOENT");
+  }
+});
+
 it("should fail on Git URL with invalid committish", async () => {
   const urls: string[] = [];
   setHandler(dummyRegistry(urls));
@@ -2224,7 +2262,7 @@ it("should fail on Git URL with invalid committish", async () => {
   expect(stderr).toBeDefined();
   const err = await new Response(stderr).text();
   expect(err.split(/\r?\n/)).toContain(
-    'Error no commit matching "404-no_such_tag" found for "uglify" (but repository exists)',
+    'error: no commit matching "404-no_such_tag" found for "uglify" (but repository exists)',
   );
   expect(stdout).toBeDefined();
   const out = await new Response(stdout).text();