From 8768c0628b4ab960e1c8d9991b7fdecd8eb31f3f Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Fri, 15 Sep 2023 04:21:43 -0700 Subject: [PATCH] Make `bun run --silent` omit `"error: "..." exited with code 1` (#5459) * Make --silent behave as expected * Make the "tsconfig.json extends" error a debug level --------- Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> --- src/cli/run_command.zig | 35 ++++++++----- src/logger.zig | 6 +-- src/resolver/resolver.zig | 2 +- test/cli/install/bun-run.test.ts | 90 +++++++++++++++++++++++--------- 4 files changed, 93 insertions(+), 40 deletions(-) diff --git a/src/cli/run_command.zig b/src/cli/run_command.zig index 42560947a09b2..3ef68b4948c7b 100644 --- a/src/cli/run_command.zig +++ b/src/cli/run_command.zig @@ -282,7 +282,10 @@ pub const RunCommand = struct { child_process.stdout_behavior = .Inherit; const result = child_process.spawnAndWait() catch |err| { - Output.prettyErrorln("error: Failed to run script {s} due to error {s}", .{ name, @errorName(err) }); + if (!silent) { + Output.prettyErrorln("error: Failed to run script {s} due to error {s}", .{ name, @errorName(err) }); + } + Output.flush(); return true; }; @@ -290,7 +293,7 @@ pub const RunCommand = struct { switch (result) { .Exited => |code| { if (code > 0) { - if (code != 2) { + if (code != 2 and !silent) { Output.prettyErrorln("error: script \"{s}\" exited with {any}", .{ name, bun.SignalCode.from(code) }); Output.flush(); } @@ -299,14 +302,18 @@ pub const RunCommand = struct { } }, .Signal => |signal| { - Output.prettyErrorln("error: script \"{s}\" exited with {any}", .{ name, bun.SignalCode.from(signal) }); - Output.flush(); + if (!silent) { + Output.prettyErrorln("error: script \"{s}\" exited with {any}", .{ name, bun.SignalCode.from(signal) }); + Output.flush(); + } Global.exit(1); }, .Stopped => |signal| { - Output.prettyErrorln("error: script \"{s}\" was stopped by signal {any}", .{ name, bun.SignalCode.from(signal) }); - Output.flush(); + if (!silent) { + Output.prettyErrorln("error: script \"{s}\" was stopped by signal {any}", .{ name, bun.SignalCode.from(signal) }); + Output.flush(); + } Global.exit(1); }, @@ -341,6 +348,7 @@ pub const RunCommand = struct { child_process.stderr_behavior = .Inherit; child_process.stdin_behavior = .Inherit; child_process.stdout_behavior = .Inherit; + const silent = ctx.debug.silent; const result = child_process.spawnAndWait() catch |err| { if (err == error.AccessDenied) { @@ -349,7 +357,8 @@ pub const RunCommand = struct { const rc = bun.C.stat(executable[0.. :0].ptr, &stat); if (rc == 0) { if (std.os.S.ISDIR(stat.mode)) { - Output.prettyErrorln("error: Failed to run directory \"{s}\"\n", .{executable}); + if (!silent) + Output.prettyErrorln("error: Failed to run directory \"{s}\"\n", .{executable}); Global.exit(1); } } @@ -361,24 +370,25 @@ pub const RunCommand = struct { switch (result) { .Exited => |sig| { // 2 is SIGINT, which is CTRL + C so that's kind of annoying to show - if (sig > 0 and sig != 2) + if (sig > 0 and sig != 2 and !silent) Output.prettyErrorln("error: \"{s}\" exited with {any}", .{ std.fs.path.basename(executable), bun.SignalCode.from(sig) }); Global.exit(sig); }, .Signal => |sig| { // 2 is SIGINT, which is CTRL + C so that's kind of annoying to show - if (sig > 0 and sig != 2) { + if (sig > 0 and sig != 2 and !silent) { Output.prettyErrorln("error: \"{s}\" exited with {any}", .{ std.fs.path.basename(executable), bun.SignalCode.from(sig) }); } Global.exit(std.mem.asBytes(&sig)[0]); }, .Stopped => |sig| { - if (sig > 0) + if (sig > 0 and !silent) Output.prettyErrorln("error \"{s}\" stopped with {any}", .{ std.fs.path.basename(executable), bun.SignalCode.from(sig) }); Global.exit(std.mem.asBytes(&sig)[0]); }, .Unknown => |sig| { - Output.prettyErrorln("error \"{s}\" stopped: {d}", .{ std.fs.path.basename(executable), sig }); + if (!silent) + Output.prettyErrorln("error \"{s}\" stopped: {d}", .{ std.fs.path.basename(executable), sig }); Global.exit(1); }, } @@ -955,7 +965,8 @@ pub const RunCommand = struct { // "White space after #! is optional." var shebang_buf: [64]u8 = undefined; const shebang_size = file.pread(&shebang_buf, 0) catch |err| { - Output.prettyErrorln("error: Failed to read file {s} due to error {s}", .{ file_path, @errorName(err) }); + if (!ctx.debug.silent) + Output.prettyErrorln("error: Failed to read file {s} due to error {s}", .{ file_path, @errorName(err) }); Global.exit(1); }; diff --git a/src/logger.zig b/src/logger.zig index ba27fcdbe2cc4..d1b89438beb25 100644 --- a/src/logger.zig +++ b/src/logger.zig @@ -703,12 +703,12 @@ pub const Log = struct { }; } - pub fn addVerboseFmt(log: *Log, source: ?*const Source, l: Loc, allocator: std.mem.Allocator, comptime text: string, args: anytype) !void { - if (!Kind.shouldPrint(.verbose, log.level)) return; + pub fn addDebugFmt(log: *Log, source: ?*const Source, l: Loc, allocator: std.mem.Allocator, comptime text: string, args: anytype) !void { + if (!Kind.shouldPrint(.debug, log.level)) return; @setCold(true); try log.addMsg(.{ - .kind = .verbose, + .kind = .debug, .data = try rangeData(source, Range{ .loc = l }, allocPrint(allocator, text, args) catch unreachable).cloneLineText(log.clone_line_text, log.msgs.allocator), }); } diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig index c80fe2adf1b7f..b6f571e230820 100644 --- a/src/resolver/resolver.zig +++ b/src/resolver/resolver.zig @@ -3887,7 +3887,7 @@ pub const Resolver = struct { // not sure why this needs cwd but we'll just pass in the dir of the tsconfig... var abs_path = ResolvePath.joinAbsStringBuf(ts_dir_name, bufs(.tsconfig_path_abs), &[_]string{ ts_dir_name, current.extends }, .auto); var parent_config_maybe = r.parseTSConfig(abs_path, 0) catch |err| { - r.log.addVerboseFmt(null, logger.Loc.Empty, r.allocator, "{s} loading tsconfig.json extends {}", .{ @errorName(err), strings.QuotedFormatter{ + r.log.addDebugFmt(null, logger.Loc.Empty, r.allocator, "{s} loading tsconfig.json extends {}", .{ @errorName(err), strings.QuotedFormatter{ .text = abs_path, } }) catch {}; break; diff --git a/test/cli/install/bun-run.test.ts b/test/cli/install/bun-run.test.ts index 6bf910951207e..3c12086b10bd5 100644 --- a/test/cli/install/bun-run.test.ts +++ b/test/cli/install/bun-run.test.ts @@ -87,39 +87,81 @@ for (let withRun of [false, true]) { expect(exitCode).toBe(0); }); - it("valid tsconfig.json with invalid extends doesn't crash", async () => { - await writeFile( - join(run_dir, "package.json"), - JSON.stringify({ - name: "test", - version: "0.0.0", - scripts: {}, - }), - ); - await writeFile( - join(run_dir, "tsconfig.json"), - JSON.stringify( - { - extends: "!!!bad!!!", - }, - null, - 2, - ), - ); + it("--silent omits error messages", async () => { + const { stdout, stderr, exitCode } = spawnSync({ + cmd: [bunExe(), "run", "--silent", "bash", "-c", "exit 1"], + cwd: run_dir, + env: bunEnv, + }); - await writeFile(join(run_dir, "index.js"), "console.log('hi')"); + expect(stderr.toString()).toBe(""); + expect(stdout.toString()).toBe(""); + expect(exitCode).toBe(1); + }); + it("no --silent includes error messages", async () => { const { stdout, stderr, exitCode } = spawnSync({ - cmd: [bunExe(), "--silent", withRun ? "run" : "", "./index.js"].filter(Boolean), + cmd: [bunExe(), "run", "bash", "-c", "exit 1"], cwd: run_dir, env: bunEnv, }); - expect(stderr.toString().trim()).toContain("FileNotFound loading tsconfig.json extends"); - expect(stdout.toString()).toBe("hi\n"); - expect(exitCode).toBe(0); + expect(stderr.toString()).toStartWith('error: "bash" exited with code 1'); + expect(exitCode).toBe(1); }); + for (let withLogLevel of [true, false]) { + it( + "valid tsconfig.json with invalid extends doesn't crash" + (withLogLevel ? " (log level debug)" : ""), + async () => { + await writeFile( + join(run_dir, "package.json"), + JSON.stringify({ + name: "test", + version: "0.0.0", + scripts: {}, + }), + ); + if (withLogLevel) + await writeFile( + join(run_dir, "bunfig.toml"), + ` +logLevel = "debug" + `, + ); + + await writeFile( + join(run_dir, "tsconfig.json"), + JSON.stringify( + { + extends: "!!!bad!!!", + }, + null, + 2, + ), + ); + + await writeFile(join(run_dir, "index.js"), "console.log('hi')"); + + const { stdout, stderr, exitCode } = spawnSync({ + // TODO: figure out why -c is necessary here. + cmd: [bunExe(), withRun ? "run" : "", "-c=" + join(run_dir, "bunfig.toml"), "./index.js"].filter(Boolean), + cwd: run_dir, + env: bunEnv, + }); + if (withLogLevel) { + expect(stderr.toString().trim()).toContain("FileNotFound loading tsconfig.json extends"); + } else { + expect(stderr.toString().trim()).not.toContain("FileNotFound loading tsconfig.json extends"); + } + + expect(stdout.toString()).toBe("hi\n"); + expect(exitCode).toBe(0); + await rm(join(run_dir, "bunfig.toml"), { force: true }); + }, + ); + } + it("falling back to index with no package.json", async () => { await writeFile(join(run_dir, "index.ts"), "console.log('Hello, world!');");