Fix subprocess.kill(undefined) (#9466)

* Fix subprocess.kill()

* Update spawn-kill-signal.test.ts

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>

src/bun.js/api/bun/subprocess.zig

@@ -555,22 +555,45 @@ pub const Subprocess = struct {
         var arguments = callframe.arguments(1);
         // If signal is 0, then no actual signal is sent, but error checking
         // is still performed.
-        var sig: i32 = SignalCode.default;
+        const sig: i32 = brk: {
+            if (arguments.ptr[0].getNumber()) |sig64| {
+                // Node does this:
+                if (std.math.isNan(sig64)) {
+                    break :brk SignalCode.default;
+                }
 
-        if (arguments.len > 0) {
-            if (arguments.ptr[0].isString()) {
+                // This matches node behavior, minus some details with the error messages: https://gist.github.com/Jarred-Sumner/23ba38682bf9d84dff2f67eb35c42ab6
+                if (std.math.isInf(sig64) or @trunc(sig64) != sig64) {
+                    globalThis.throwInvalidArguments("Unknown signal", .{});
+                    return .zero;
+                }
+
+                if (sig64 < 0) {
+                    globalThis.throwInvalidArguments("Invalid signal: must be >= 0", .{});
+                    return .zero;
+                }
+
+                if (sig64 > 31) {
+                    globalThis.throwInvalidArguments("Invalid signal: must be < 32", .{});
+                    return .zero;
+                }
+
+                break :brk @intFromFloat(sig64);
+            } else if (arguments.ptr[0].isString()) {
+                if (arguments.ptr[0].asString().length() == 0) {
+                    break :brk SignalCode.default;
+                }
                 const signal_code = arguments.ptr[0].toEnum(globalThis, "signal", SignalCode) catch return .zero;
-                sig = @intFromEnum(signal_code);
-            } else {
-                sig = arguments.ptr[0].coerce(i32, globalThis);
+                break :brk @intFromEnum(signal_code);
+            } else if (!arguments.ptr[0].isEmptyOrUndefinedOrNull()) {
+                globalThis.throwInvalidArguments("Invalid signal: must be a string or an integer", .{});
+                return .zero;
             }
-            if (globalThis.hasException()) return .zero;
-        }
 
-        if (!(sig >= 0 and sig <= std.math.maxInt(u8))) {
-            globalThis.throwInvalidArguments("Invalid signal: must be >= 0 and <= 255", .{});
-            return .zero;
-        }
+            break :brk SignalCode.default;
+        };
+
+        if (globalThis.hasException()) return .zero;
 
         switch (this.tryKill(sig)) {
             .result => {},

src/bun.js/bindings/bindings.zig

@@ -4981,6 +4981,22 @@ pub const JSValue = enum(JSValueReprInt) {
         });
     }
 
+    /// Check if the JSValue is either a signed 32-bit integer or a double and
+    /// return the value as a f64
+    ///
+    /// This does not call `valueOf` on the JSValue
+    pub fn getNumber(this: JSValue) ?f64 {
+        if (this.isInt32()) {
+            return @as(f64, @floatFromInt(this.asInt32()));
+        }
+
+        if (isNumber(this)) {
+            return asDouble(this);
+        }
+
+        return null;
+    }
+
     pub fn asNumber(this: JSValue) f64 {
         if (this.isInt32()) {
             return @as(f64, @floatFromInt(this.asInt32()));

test/js/bun/spawn/spawn-kill-signal.test.ts

@@ -0,0 +1,53 @@
+import { test, expect, describe } from "bun:test";
+import { isWindows } from "harness";
+import { constants } from "os";
+
+const inputs = {
+  SIGTERM: [["SIGTERM"], [undefined], [""], [null], [], [constants.signals.SIGTERM], [NaN]],
+  SIGKILL: [["SIGKILL"], [constants.signals.SIGKILL]],
+} as const;
+const fails = [["SIGGOD"], [{}], [() => {}], [Infinity], [-Infinity], [Symbol("what")]] as const;
+describe("subprocess.kill", () => {
+  for (const key in inputs) {
+    describe(key, () => {
+      for (let input of inputs[key as keyof typeof inputs]) {
+        test(Bun.inspect(input).replaceAll("\n", "\\n"), async () => {
+          const proc = Bun.spawn({
+            cmd: ["bash", "-c", "sleep infinity"],
+            stdio: ["inherit", "inherit", "inherit"],
+          });
+
+          const { promise, resolve, reject } = Promise.withResolvers();
+          proc.exited.then(resolve, reject);
+          proc.kill(...input);
+
+          await promise;
+          expect(proc.exitCode).toBe(isWindows ? 1 : null);
+          expect(proc.signalCode).toBe(key as any);
+        });
+      }
+    });
+  }
+
+  describe("input validation", () => {
+    for (let input of fails) {
+      test(Bun.inspect(input).replaceAll("\n", "\\n"), async () => {
+        const proc = Bun.spawn({
+          cmd: ["bash", "-c", "sleep infinity"],
+          stdio: ["inherit", "inherit", "inherit"],
+        });
+
+        expect(() => proc.kill(...(input as any))).toThrow();
+
+        const { promise, resolve, reject } = Promise.withResolvers();
+        proc.exited.then(resolve, reject);
+        proc.kill();
+
+        await promise;
+
+        expect(proc.exitCode).toBe(isWindows ? 1 : null);
+        expect(proc.signalCode).toBe("SIGTERM");
+      });
+    }
+  });
+});