fix(runtime): emit node:net connect error event vs throw (oven-sh#5336

) * fix(runtime): emit `node:net` connect error event vs throw * oops * finally * ok * we are good
SuperAuguste · Sep 18, 2023 · a4a1eed · a4a1eed
1 parent 152f542
commit a4a1eed
Show file tree

Hide file tree

Showing 5 changed files with 122 additions and 63 deletions.
diff --git a/src/bun.js/api/bun/socket.zig b/src/bun.js/api/bun/socket.zig
@@ -996,7 +996,12 @@ pub const Listener = struct {
                 handlers.vm.allocator.destroy(handlers_ptr);
                 handlers.promise.deinit();
                 bun.default_allocator.destroy(tls);
-                exception.* = ZigString.static("Failed to connect").toErrorInstance(globalObject).asObjectRef();
+                const err = JSC.SystemError{
+                    .message = bun.String.static("Failed to connect"),
+                    .syscall = bun.String.static("connect"),
+                    .code = if (port == null) bun.String.static("ENOENT") else bun.String.static("ECONNREFUSED"),
+                };
+                exception.* = err.toErrorInstance(globalObject).asObjectRef();
                 return .zero;
             };
             tls.poll_ref.ref(handlers.vm);
@@ -1022,7 +1027,12 @@ pub const Listener = struct {
                 handlers.vm.allocator.destroy(handlers_ptr);
                 handlers.promise.deinit();
                 bun.default_allocator.destroy(tcp);
-                exception.* = ZigString.static("Failed to connect").toErrorInstance(globalObject).asObjectRef();
+                const err = JSC.SystemError{
+                    .message = bun.String.static("Failed to connect"),
+                    .syscall = bun.String.static("connect"),
+                    .code = if (port == null) bun.String.static("ENOENT") else bun.String.static("ECONNREFUSED"),
+                };
+                exception.* = err.toErrorInstance(globalObject).asObjectRef();
                 return .zero;
             };
             tcp.poll_ref.ref(handlers.vm);
@@ -1205,6 +1215,12 @@ fn NewSocket(comptime ssl: bool) type {
                 .errno = errno,
                 .message = bun.String.static("Failed to connect"),
                 .syscall = bun.String.static("connect"),
+
+                // For some reason errno is 0 which causes this to be success.
+                // Unix socket case wont hit this callback because it instantly errors.
+                .code = bun.String.static("ECONNREFUSED"),
+                // .code = bun.String.static(@tagName(bun.sys.getErrno(errno))),
+                // .code = bun.String.static(@tagName(@as(bun.C.E, @enumFromInt(errno)))),
             };
 
             if (callback == .zero) {

diff --git a/src/js/node/net.js b/src/js/node/net.js
@@ -66,6 +66,11 @@ const bunSocketServerOptions = Symbol.for("::bunnetserveroptions::");
 const bunSocketInternal = Symbol.for("::bunnetsocketinternal::");
 const bunTLSConnectOptions = Symbol.for("::buntlsconnectoptions::");
 
+function endNT(socket, callback, err) {
+  socket.end();
+  callback(err);
+}
+
 var SocketClass;
 const Socket = (function (InternalSocket) {
   SocketClass = InternalSocket;
@@ -446,42 +451,18 @@ const Socket = (function (InternalSocket) {
       } else if (connectListener) this.on("connect", connectListener);
 
       // start using existing connection
-      if (connection) {
-        const socket = connection[bunSocketInternal];
-
-        if (socket) {
-          this.connecting = true;
-          this.#upgraded = true;
-          const result = socket.upgradeTLS({
-            data: this,
-            tls,
-            socket: Socket.#Handlers,
-          });
-          if (result) {
-            const [raw, tls] = result;
-            // replace socket
-            connection[bunSocketInternal] = raw;
-            raw.timeout(raw.timeout);
-            raw.connecting = false;
-            this[bunSocketInternal] = tls;
-          } else {
-            this[bunSocketInternal] = null;
-            throw new Error("Invalid socket");
-          }
-        } else {
-          // wait to be connected
-          connection.once("connect", () => {
-            const socket = connection[bunSocketInternal];
-            if (!socket) return;
+      try {
+        if (connection) {
+          const socket = connection[bunSocketInternal];
 
+          if (socket) {
             this.connecting = true;
             this.#upgraded = true;
             const result = socket.upgradeTLS({
               data: this,
               tls,
               socket: Socket.#Handlers,
             });
-
             if (result) {
               const [raw, tls] = result;
               // replace socket
@@ -493,38 +474,66 @@ const Socket = (function (InternalSocket) {
               this[bunSocketInternal] = null;
               throw new Error("Invalid socket");
             }
+          } else {
+            // wait to be connected
+            connection.once("connect", () => {
+              const socket = connection[bunSocketInternal];
+              if (!socket) return;
+
+              this.connecting = true;
+              this.#upgraded = true;
+              const result = socket.upgradeTLS({
+                data: this,
+                tls,
+                socket: Socket.#Handlers,
+              });
+
+              if (result) {
+                const [raw, tls] = result;
+                // replace socket
+                connection[bunSocketInternal] = raw;
+                raw.timeout(raw.timeout);
+                raw.connecting = false;
+                this[bunSocketInternal] = tls;
+              } else {
+                this[bunSocketInternal] = null;
+                throw new Error("Invalid socket");
+              }
+            });
+          }
+        } else if (path) {
+          // start using unix socket
+          bunConnect({
+            data: this,
+            unix: path,
+            socket: Socket.#Handlers,
+            tls,
+          }).catch(error => {
+            this.emit("error", error);
+            this.emit("close");
+          });
+        } else {
+          // default start
+          bunConnect({
+            data: this,
+            hostname: host || "localhost",
+            port: port,
+            socket: Socket.#Handlers,
+            tls,
+          }).catch(error => {
+            this.emit("error", error);
+            this.emit("close");
           });
         }
-      } else if (path) {
-        // start using unix socket
-        bunConnect({
-          data: this,
-          unix: path,
-          socket: Socket.#Handlers,
-          tls,
-        }).catch(error => {
-          this.emit("error", error);
-          this.emit("close");
-        });
-      } else {
-        // default start
-        bunConnect({
-          data: this,
-          hostname: host || "localhost",
-          port: port,
-          socket: Socket.#Handlers,
-          tls,
-        }).catch(error => {
-          this.emit("error", error);
-          this.emit("close");
-        });
+      } catch (error) {
+        process.nextTick(emitErrorAndCloseNextTick, this, error);
       }
       return this;
     }
 
     _destroy(err, callback) {
-      this[bunSocketInternal]?.end();
-      callback(err);
+      const socket = this[bunSocketInternal];
+      socket && process.nextTick(endNT, socket, callback, err);
     }
 
     _final(callback) {
@@ -864,6 +873,11 @@ function emitErrorNextTick(self, error) {
   self.emit("error", error);
 }
 
+function emitErrorAndCloseNextTick(self, error) {
+  self.emit("error", error);
+  self.emit("close");
+}
+
 function emitListeningNextTick(self, onListen) {
   if (typeof onListen === "function") {
     try {