Merge branch 'main' into nektro-patch-5668

src/bun.js/bindings/BunProcess.cpp

@@ -108,7 +108,7 @@ JSC_DECLARE_HOST_FUNCTION(Process_functionCwd);
 static bool processIsExiting = false;
 
 extern "C" uint8_t Bun__getExitCode(void*);
-extern "C" uint8_t Bun__setExitCode(void*, uint8_t);
+extern "C" uint8_t Bun__setExitCode(void*, uint8_t, bool);
 extern "C" void* Bun__getVM();
 extern "C" Zig::GlobalObject* Bun__getDefaultGlobalObject();
 extern "C" bool Bun__GlobalObject__hasIPC(JSGlobalObject*);
@@ -220,7 +220,6 @@ static JSValue constructProcessReleaseObject(VM& vm, JSObject* processObject)
 
 static void dispatchExitInternal(JSC::JSGlobalObject* globalObject, Process* process, int exitCode)
 {
-
     if (processIsExiting)
         return;
     processIsExiting = true;
@@ -486,18 +485,16 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionExit,
 {
     auto throwScope = DECLARE_THROW_SCOPE(globalObject->vm());
     uint8_t exitCode = 0;
+    bool is_explicit = false;
     JSValue arg0 = callFrame->argument(0);
     if (arg0.isAnyInt()) {
         int extiCode32 = arg0.toInt32(globalObject) % 256;
         RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::JSValue {}));
-
         exitCode = static_cast<uint8_t>(extiCode32);
-        Bun__setExitCode(Bun__getVM(), exitCode);
+        is_explicit = true;
     } else if (!arg0.isUndefinedOrNull()) {
         throwTypeError(globalObject, throwScope, "The \"code\" argument must be an integer"_s);
         return JSC::JSValue::encode(JSC::JSValue {});
-    } else {
-        exitCode = Bun__getExitCode(Bun__getVM());
     }
 
     auto* zigGlobal = jsDynamicCast<Zig::GlobalObject*>(globalObject);
@@ -507,8 +504,7 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionExit,
     auto process = jsCast<Process*>(zigGlobal->processObject());
     process->m_isExitCodeObservable = true;
 
-    Process__dispatchOnExit(zigGlobal, exitCode);
-    Bun__Process__exit(zigGlobal, exitCode);
+    Bun__Process__exit(zigGlobal, exitCode, is_explicit);
     return JSC::JSValue::encode(jsUndefined());
 }
 
@@ -850,7 +846,7 @@ extern "C" int Bun__handleUncaughtException(JSC::JSGlobalObject* lexicalGlobalOb
             scope.clearException();
             // if an exception is thrown in the uncaughtException handler, we abort
             Bun__logUnhandledException(JSValue::encode(JSValue(ex)));
-            Bun__Process__exit(lexicalGlobalObject, 1);
+            Bun__Process__exit(lexicalGlobalObject, 1, true);
         }
     } else if (wrapped.listenerCount(uncaughtExceptionIdent) > 0) {
         wrapped.emit(uncaughtExceptionIdent, args);
@@ -1125,7 +1121,7 @@ JSC_DEFINE_CUSTOM_SETTER(setProcessExitCode, (JSC::JSGlobalObject * lexicalGloba
 
     process->m_isExitCodeObservable = true;
     void* ptr = jsCast<Zig::GlobalObject*>(process->globalObject())->bunVM();
-    Bun__setExitCode(ptr, static_cast<uint8_t>(exitCodeInt));
+    Bun__setExitCode(ptr, static_cast<uint8_t>(exitCodeInt), true);
 
     return true;
 }
@@ -2145,22 +2141,22 @@ JSC_DEFINE_HOST_FUNCTION(Process_functionReallyExit, (JSGlobalObject * globalObj
     auto& vm = globalObject->vm();
     auto throwScope = DECLARE_THROW_SCOPE(vm);
     uint8_t exitCode = 0;
+    bool is_explicit = false;
     JSValue arg0 = callFrame->argument(0);
     if (arg0.isAnyInt()) {
         exitCode = static_cast<uint8_t>(arg0.toInt32(globalObject) % 256);
+        is_explicit = true;
         RETURN_IF_EXCEPTION(throwScope, JSC::JSValue::encode(JSC::JSValue {}));
     } else if (!arg0.isUndefinedOrNull()) {
         throwTypeError(globalObject, throwScope, "The \"code\" argument must be an integer"_s);
         return JSC::JSValue::encode(JSC::JSValue {});
-    } else {
-        exitCode = Bun__getExitCode(Bun__getVM());
     }
 
     auto* zigGlobal = jsDynamicCast<Zig::GlobalObject*>(globalObject);
     if (UNLIKELY(!zigGlobal)) {
         zigGlobal = Bun__getDefaultGlobalObject();
     }
-    Bun__Process__exit(zigGlobal, exitCode);
+    Bun__Process__exit(zigGlobal, exitCode, is_explicit);
     return JSC::JSValue::encode(jsUndefined());
 }
 

src/bun.js/bindings/headers.h

@@ -737,7 +737,7 @@ ZIG_DECL void Bun__WebSocketClientTLS__writeString(WebSocketClientTLS* arg0, con
 
 #ifdef __cplusplus
 
-ZIG_DECL void Bun__Process__exit(JSC__JSGlobalObject* arg0, unsigned char arg1);
+ZIG_DECL void Bun__Process__exit(JSC__JSGlobalObject* arg0, uint8_t arg1, bool arg2);
 ZIG_DECL JSC__JSValue Bun__Process__getArgv(JSC__JSGlobalObject* arg0);
 ZIG_DECL JSC__JSValue Bun__Process__getArgv0(JSC__JSGlobalObject* arg0);
 ZIG_DECL JSC__JSValue Bun__Process__getCwd(JSC__JSGlobalObject* arg0);

src/bun.js/javascript.zig

@@ -542,13 +542,16 @@ comptime {
 
 pub const ExitHandler = struct {
     exit_code: u8 = 0,
+    explicit: bool = false,
 
     pub export fn Bun__getExitCode(vm: *VirtualMachine) u8 {
         return vm.exit_handler.exit_code;
     }
 
-    pub export fn Bun__setExitCode(vm: *VirtualMachine, code: u8) void {
+    pub export fn Bun__setExitCode(vm: *VirtualMachine, code: u8, explicit: bool) void {
+        if (vm.exit_handler.explicit and !explicit) return;
         vm.exit_handler.exit_code = code;
+        vm.exit_handler.explicit = explicit;
     }
 
     extern fn Process__dispatchOnBeforeExit(*JSC.JSGlobalObject, code: u8) void;
@@ -1051,7 +1054,7 @@ pub const VirtualMachine = struct {
 
     extern fn Bun__handleUncaughtException(*JSC.JSGlobalObject, err: JSC.JSValue, is_rejection: c_int) c_int;
     extern fn Bun__handleUnhandledRejection(*JSC.JSGlobalObject, reason: JSC.JSValue, promise: JSC.JSValue) c_int;
-    extern fn Bun__Process__exit(*JSC.JSGlobalObject, code: c_int) noreturn;
+    extern fn Bun__Process__exit(*JSC.JSGlobalObject, code: c_int, explicit: bool) noreturn;
 
     pub fn unhandledRejection(this: *JSC.VirtualMachine, globalObject: *JSC.JSGlobalObject, reason: JSC.JSValue, promise: JSC.JSValue) bool {
         if (this.isShuttingDown()) {
@@ -1087,7 +1090,7 @@ pub const VirtualMachine = struct {
 
         if (this.is_handling_uncaught_exception) {
             this.runErrorHandler(err, null);
-            Bun__Process__exit(globalObject, 7);
+            Bun__Process__exit(globalObject, 7, true);
             @panic("Uncaught exception while handling uncaught exception");
         }
         this.is_handling_uncaught_exception = true;

src/bun.js/node/types.zig

@@ -2108,16 +2108,18 @@ pub const Process = struct {
         }
     }
 
-    pub fn exit(globalObject: *JSC.JSGlobalObject, code: u8) callconv(.C) void {
+    extern fn Bun__setExitCode(vm: *JSC.VirtualMachine, code: u8, explicit: bool) void;
+    pub fn exit(globalObject: *JSC.JSGlobalObject, code: u8, explicit: bool) callconv(.C) void {
         var vm = globalObject.bunVM();
+        Bun__setExitCode(vm, code, explicit);
+
         if (vm.worker) |worker| {
-            vm.exit_handler.exit_code = code;
             worker.requestTerminate();
             return;
         }
 
-        vm.exit_handler.exit_code = code;
         vm.onExit();
+        if (vm.unhandled_error_counter > 0 and vm.exit_handler.exit_code == 0) Bun__setExitCode(vm, 1, false);
         vm.globalExit();
     }
 

src/bun.js/web_worker.zig

@@ -8,6 +8,7 @@ const Async = bun.Async;
 const WTFStringImpl = @import("../string.zig").WTFStringImpl;
 
 const Bool = std.atomic.Value(bool);
+extern fn Bun__setExitCode(vm: *JSC.VirtualMachine, code: u8, explicit: bool) void;
 
 /// Shared implementation of Web and Node `Worker`
 pub const WebWorker = struct {
@@ -316,7 +317,7 @@ pub const WebWorker = struct {
             const handled = vm.uncaughtException(vm.global, promise.result(vm.global.vm()), true);
 
             if (!handled) {
-                vm.exit_handler.exit_code = 1;
+                Bun__setExitCode(vm, 1, false);
                 this.exitAndDeinit();
                 return;
             }

src/bun_js.zig

@@ -285,6 +285,7 @@ pub const Run = struct {
     }
 
     extern fn Bun__ExposeNodeModuleGlobals(*JSC.JSGlobalObject) void;
+    extern fn Bun__setExitCode(vm: *JSC.VirtualMachine, code: u8, explicit: bool) void;
 
     pub fn start(this: *Run) void {
         var vm = this.vm;
@@ -313,7 +314,7 @@ pub const Run = struct {
                     vm.eventLoop().tick();
                     vm.eventLoop().tickPossiblyForever();
                 } else {
-                    vm.exit_handler.exit_code = 1;
+                    Bun__setExitCode(vm, 1, true);
                     vm.onExit();
 
                     if (run.any_unhandled) {
@@ -347,7 +348,7 @@ pub const Run = struct {
                 vm.eventLoop().tick();
                 vm.eventLoop().tickPossiblyForever();
             } else {
-                vm.exit_handler.exit_code = 1;
+                Bun__setExitCode(vm, 1, true);
                 vm.onExit();
                 if (run.any_unhandled) {
                     bun.JSC.SavedSourceMap.MissingSourceMapNoteInfo.print();
@@ -454,7 +455,7 @@ pub const Run = struct {
         vm.onExit();
 
         if (this.any_unhandled and this.vm.exit_handler.exit_code == 0) {
-            this.vm.exit_handler.exit_code = 1;
+            Bun__setExitCode(vm, 1, true);
 
             bun.JSC.SavedSourceMap.MissingSourceMapNoteInfo.print();
 

test/cli/test/bun-test.test.ts

@@ -27,6 +27,7 @@ describe("bun test", () => {
           });
         `,
       ],
+      expectedExitCode: 1,
     });
     expect(stderr).toContain("test #1");
     expect(stderr).toContain("test #2");
@@ -160,6 +161,12 @@ describe("bun test", () => {
     });
     expect(stderr).toContain("test #1");
   });
+  test("works if you call process.exit()", () => {
+    runTest({
+      args: [],
+      input: [`process.exit();`],
+    });
+  });
   test.todo("can provide a mix of files and directories");
   describe("--rerun-each", () => {
     test.todo("can rerun with a default value");
@@ -186,6 +193,7 @@ describe("bun test", () => {
             console.error("should run");
           });
         `,
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("should run");
     });
@@ -262,6 +270,7 @@ describe("bun test", () => {
     test("must provide a number bail", () => {
       const stderr = runTest({
         args: ["--bail=foo"],
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("expects a number");
     });
@@ -269,6 +278,7 @@ describe("bun test", () => {
     test("must provide non-negative bail", () => {
       const stderr = runTest({
         args: ["--bail=-1"],
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("expects a number");
     });
@@ -276,6 +286,7 @@ describe("bun test", () => {
     test("should not be 0", () => {
       const stderr = runTest({
         args: ["--bail=0"],
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("expects a number");
     });
@@ -292,6 +303,7 @@ describe("bun test", () => {
             expect(true).toBe(true);
           });
         `,
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("Bailed out after 1 failure");
       expect(stderr).not.toContain("test #2");
@@ -315,6 +327,7 @@ describe("bun test", () => {
             expect(true).toBe(true);
           });
         `,
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("Bailed out after 3 failures");
       expect(stderr).not.toContain("test #4");
@@ -324,12 +337,14 @@ describe("bun test", () => {
     test("must provide a number timeout", () => {
       const stderr = runTest({
         args: ["--timeout", "foo"],
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("Invalid timeout");
     });
     test("must provide non-negative timeout", () => {
       const stderr = runTest({
         args: ["--timeout", "-1"],
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("Invalid timeout");
     });
@@ -350,6 +365,7 @@ describe("bun test", () => {
             await expect(sleep(64)).resolves.toBeUndefined();
           });
         `,
+        expectedExitCode: 1,
       });
       expect(stderr).toHaveTestTimedOutAfter(30);
     });
@@ -363,6 +379,7 @@ describe("bun test", () => {
             await sleep(${time});
           });
         `,
+        expectedExitCode: 1,
       });
       expect(stderr).toHaveTestTimedOutAfter(5000);
     }, 10000);
@@ -420,6 +437,7 @@ describe("bun test", () => {
         env: {
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("::group::");
       expect(stderr.match(/::group::/g)).toHaveLength(3);
@@ -446,6 +464,7 @@ describe("bun test", () => {
         env: {
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toContain("::group::");
       expect(stderr.match(/::group::/g)).toHaveLength(6);
@@ -463,6 +482,7 @@ describe("bun test", () => {
         env: {
           GITHUB_ACTIONS: undefined,
         },
+        expectedExitCode: 1,
       });
       expect(stderr).not.toContain("::error");
     });
@@ -506,6 +526,7 @@ describe("bun test", () => {
         env: {
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toMatch(/::error file=.*,line=\d+,col=\d+,title=error::/);
     });
@@ -520,6 +541,7 @@ describe("bun test", () => {
         env: {
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toMatch(/::error file=.*,line=\d+,col=\d+,title=error::/);
     });
@@ -535,6 +557,7 @@ describe("bun test", () => {
         env: {
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toMatch(/::error file=.*,line=\d+,col=\d+,title=error::/);
     });
@@ -550,6 +573,7 @@ describe("bun test", () => {
           FORCE_COLOR: "1",
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toMatch(/::error file=.*,line=\d+,col=\d+,title=.*::/);
       expect(stderr).toMatch(/error: expect\(received\)\.toBe\(expected\)/); // stripped ansi
@@ -567,6 +591,7 @@ describe("bun test", () => {
           FORCE_COLOR: "1",
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toMatch(/::error title=error: Oops!::/);
     });
@@ -582,6 +607,7 @@ describe("bun test", () => {
           FORCE_COLOR: "1",
           GITHUB_ACTIONS: "true",
         },
+        expectedExitCode: 1,
       });
       expect(stderr).toMatch(/::error title=error: Test \"time out\" timed out after \d+ms::/);
     });
@@ -752,6 +778,7 @@ describe("bun test", () => {
             expect(s).toBeType("string");
           });
         `,
+        expectedExitCode: 1,
       });
       strings.forEach(s => {
         expect(stderr).toContain(`with a string: ${s}`);
@@ -909,22 +936,26 @@ function runTest({
   cwd,
   args = [],
   env = {},
+  expectedExitCode = 0,
 }: {
   input?: string | (string | { filename: string; contents: string })[];
   cwd?: string;
   args?: string[];
   env?: Record<string, string | undefined>;
+  expectedExitCode?: number;
 } = {}): string {
   cwd ??= createTest(input);
   try {
-    const { stderr } = spawnSync({
+    const proc = spawnSync({
       cwd,
       cmd: [bunExe(), "test", ...args],
       env: { ...bunEnv, ...env },
       stderr: "pipe",
       stdout: "ignore",
     });
-    return stderr.toString();
+    expect(proc.signalCode).toBe(undefined);
+    expect(proc.exitCode).toBe(expectedExitCode);
+    return proc.stderr.toString();
   } finally {
     rmSync(cwd, { recursive: true });
   }

test/js/node/process/process.test.js

@@ -401,6 +401,65 @@ describe("process.onExit", () => {
     expect(proc.stderr.toString("utf8")).toInclude("error: boom");
     expect(proc.stdout.toString("utf8")).toBeEmpty();
   });
+
+  it("process.exit() doesn't clobber error thrown inside exit handler", () => {
+    const proc = Bun.spawnSync({
+      cmd: [bunExe(), "-e", `process.on("exit", () => {throw new Error("boom")}); process.exit();`],
+      env: bunEnv,
+      stdio: ["inherit", "pipe", "pipe"],
+    });
+    expect(proc.exitCode).toBe(1);
+    expect(proc.stderr.toString("utf8")).toInclude("error: boom");
+    expect(proc.stdout.toString("utf8")).toBeEmpty();
+  });
+
+  it("process.exit() doesn't clobber error thrown inside exit handler (with uncaughtException handler)", () => {
+    const proc = Bun.spawnSync({
+      cmd: [
+        bunExe(),
+        "-e",
+        `process.on("uncaughtException", () => {}); process.on("exit", () => {throw new Error("boom")}); process.exit();`,
+      ],
+      env: bunEnv,
+      stdio: ["inherit", "pipe", "pipe"],
+    });
+    expect(proc.exitCode).toBe(0);
+    expect(proc.stderr.toString("utf8")).toBeEmpty();
+    expect(proc.stdout.toString("utf8")).toBeEmpty();
+  });
+
+  it("process.exit() has higher precedence than throw in exit handler", () => {
+    const proc = Bun.spawnSync({
+      cmd: [bunExe(), "-e", `process.on("exit", () => {throw new Error("boom")}); process.exit(5);`],
+      env: bunEnv,
+      stdio: ["inherit", "pipe", "pipe"],
+    });
+    expect(proc.exitCode).toBe(5);
+    expect(proc.stderr.toString("utf8")).toInclude("error: boom");
+    expect(proc.stdout.toString("utf8")).toBeEmpty();
+  });
+
+  it("process.exitCode has higher precedence than throw in exit handler", () => {
+    const proc = Bun.spawnSync({
+      cmd: [bunExe(), "-e", `process.on("exit", () => {process.exitCode=5; throw new Error("boom")}); process.exit();`],
+      env: bunEnv,
+      stdio: ["inherit", "pipe", "pipe"],
+    });
+    expect(proc.exitCode).toBe(5);
+    expect(proc.stderr.toString("utf8")).toInclude("error: boom");
+    expect(proc.stdout.toString("utf8")).toBeEmpty();
+  });
+
+  it("process.exit() has higher precedence than throw in exit handler, even with zero", () => {
+    const proc = Bun.spawnSync({
+      cmd: [bunExe(), "-e", `process.on("exit", () => {throw new Error("boom")}); process.exit(0);`],
+      env: bunEnv,
+      stdio: ["inherit", "pipe", "pipe"],
+    });
+    expect(proc.exitCode).toBe(0);
+    expect(proc.stderr.toString("utf8")).toInclude("error: boom");
+    expect(proc.stdout.toString("utf8")).toBeEmpty();
+  });
 });
 
 it("process.memoryUsage", () => {