net: throw on an invalid socket handler config instead of aborting

src/runtime/socket/Handlers.rs

@@ -302,6 +302,42 @@ impl Handlers {
         generated: &GeneratedSocketConfigHandlers,
         is_server: bool,
     ) -> JsResult<Handlers> {
+        // Validate the config before constructing a `Handlers`: an invalid
+        // handler must return an error while there is nothing to clean up.
+        // A constructed-but-not-yet-`protect()`ed `Handlers` whose `Drop` runs
+        // `unprotect()` would trip `debug_assert!(protection_count > 0)` (debug
+        // abort) and unprotect callbacks `protect()` never rooted (release).
+        macro_rules! validate_callback {
+            ($field:ident, $name:literal) => {{
+                let value = generated.$field;
+                if !value.is_undefined_or_null() && !value.is_callable() {
+                    return Err(global_object.throw_invalid_arguments(format_args!(
+                        "Expected \"{}\" callback to be a function",
+                        $name
+                    )));
+                }
+            }};
+        }
+        validate_callback!(on_open, "onOpen");
+        validate_callback!(on_close, "onClose");
+        validate_callback!(on_data, "onData");
+        validate_callback!(on_writable, "onWritable");
+        validate_callback!(on_timeout, "onTimeout");
+        validate_callback!(on_connect_error, "onConnectError");
+        validate_callback!(on_end, "onEnd");
+        validate_callback!(on_error, "onError");
+        validate_callback!(on_handshake, "onHandshake");
+        validate_callback!(on_session, "onSession");
+        validate_callback!(on_keylog, "onKeylog");
+        validate_callback!(on_server_name, "onServerName");
+        validate_callback!(on_alpn_callback, "onALPNCallback");
+
+        if !generated.on_data.is_callable() && !generated.on_writable.is_callable() {
+            return Err(global_object.throw_invalid_arguments(format_args!(
+                "Expected at least \"data\" or \"drain\" callback"
+            )));
+        }
+
         let mut result = Handlers {
             on_open: JSValue::ZERO,
             on_close: JSValue::ZERO,
@@ -336,40 +372,27 @@ impl Handlers {
             protection_count: 0,
         };
 
-        // inline for (callback_fields) |field| { ... @field(generated, field) ... }
+        // Callbacks are validated above; assign the ones that are present.
         macro_rules! assign_callback {
-            ($field:ident, $name:literal) => {{
-                let value = generated.$field;
-                if value.is_undefined_or_null() {
-                } else if !value.is_callable() {
-                    return Err(global_object.throw_invalid_arguments(format_args!(
-                        "Expected \"{}\" callback to be a function",
-                        $name
-                    )));
-                } else {
-                    result.$field = value;
+            ($field:ident) => {{
+                if generated.$field.is_callable() {
+                    result.$field = generated.$field;
                 }
             }};
         }
-        assign_callback!(on_open, "onOpen");
-        assign_callback!(on_close, "onClose");
-        assign_callback!(on_data, "onData");
-        assign_callback!(on_writable, "onWritable");
-        assign_callback!(on_timeout, "onTimeout");
-        assign_callback!(on_connect_error, "onConnectError");
-        assign_callback!(on_end, "onEnd");
-        assign_callback!(on_error, "onError");
-        assign_callback!(on_handshake, "onHandshake");
-        assign_callback!(on_session, "onSession");
-        assign_callback!(on_keylog, "onKeylog");
-        assign_callback!(on_server_name, "onServerName");
-        assign_callback!(on_alpn_callback, "onALPNCallback");
-
-        if result.on_data.is_empty() && result.on_writable.is_empty() {
-            return Err(global_object.throw_invalid_arguments(format_args!(
-                "Expected at least \"data\" or \"drain\" callback"
-            )));
-        }
+        assign_callback!(on_open);
+        assign_callback!(on_close);
+        assign_callback!(on_data);
+        assign_callback!(on_writable);
+        assign_callback!(on_timeout);
+        assign_callback!(on_connect_error);
+        assign_callback!(on_end);
+        assign_callback!(on_error);
+        assign_callback!(on_handshake);
+        assign_callback!(on_session);
+        assign_callback!(on_keylog);
+        assign_callback!(on_server_name);
+        assign_callback!(on_alpn_callback);
         result.with_async_context_if_needed(global_object);
         result.protect();
         Ok(result)

test/js/bun/net/socket.test.ts

@@ -1558,3 +1558,38 @@ it("node:net connect() reusing a server-accepted handle keeps the listener's han
   expect(exitCode).toBe(0);
   void stderr;
 });
+
+it("Bun.listen with an invalid socket handler throws ERR_INVALID_ARG_TYPE instead of aborting", async () => {
+  // Regression: an invalid handlers object used to abort the debug process on
+  // both validation error paths (Drop ran unprotect() on a Handlers that
+  // protect() never rooted). It must throw a catchable error. Run in a
+  // subprocess so a regression (abort) shows up as a non-zero exit rather than
+  // killing the test runner.
+  await using proc = Bun.spawn({
+    cmd: [
+      bunExe(),
+      "-e",
+      `try {
+        Bun.listen({ port: 0, hostname: "127.0.0.1", socket: { data: 123 } });
+        process.exit(2); // did not throw
+      } catch (e) {
+        console.log(e.code + ": " + e.message);
+      }
+      try {
+        Bun.listen({ port: 0, hostname: "127.0.0.1", socket: {} });
+        process.exit(2); // did not throw
+      } catch (e) {
+        console.log(e.code + ": " + e.message);
+      }`,
+    ],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect(stdout).toBe(
+    'ERR_INVALID_ARG_TYPE: Expected "onData" callback to be a function\n' +
+      'ERR_INVALID_ARG_TYPE: Expected at least "data" or "drain" callback\n',
+  );
+  expect(exitCode).toBe(0);
+});