inspector: no crash when WS server can't start

This change also changes error message to make it consistent with the
one printed by the debugger.

Fixes: https://github.com/nodejs/node/issues/10858
PR-URL: https://github.com/nodejs/node/pull/10878
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

5 files changed, 33 insertions, 22 deletions

diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
index 403afe0e12..ae2e666384 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -556,9 +556,10 @@ void AgentImpl::WorkerRunIO() {
   }
   InspectorAgentDelegate delegate(this, script_path, script_name_, wait_);
   delegate_ = &delegate;
-  InspectorSocketServer server(&delegate, options_.port());
+  InspectorSocketServer server(&delegate,
+                               options_.host_name(),
+                               options_.port());
   if (!server.Start(&child_loop_)) {
-    fprintf(stderr, "Unable to open devtools socket: %s\n", uv_strerror(err));
     state_ = State::kError;  // Safe, main thread is waiting on semaphore
     uv_close(reinterpret_cast<uv_handle_t*>(&io_thread_req_), nullptr);
     uv_loop_close(&child_loop_);
diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc
index 5308e9bcf0..ee44a69a68 100644
--- a/src/inspector_socket_server.cc
+++ b/src/inspector_socket_server.cc
@@ -24,9 +24,9 @@ void Escape(std::string* string) {
   }
 }
 
-std::string GetWsUrl(int port, const std::string& id) {
+std::string GetWsUrl(const std::string& host, int port, const std::string& id) {
   char buf[1024];
-  snprintf(buf, sizeof(buf), "127.0.0.1:%d/%s", port, id.c_str());
+  snprintf(buf, sizeof(buf), "%s:%d/%s", host.c_str(), port, id.c_str());
   return buf;
 }
 
@@ -74,7 +74,8 @@ void OnBufferAlloc(uv_handle_t* handle, size_t len, uv_buf_t* buf) {
   buf->len = len;
 }
 
-void PrintDebuggerReadyMessage(int port,
+void PrintDebuggerReadyMessage(const std::string& host,
+                               int port,
                                const std::vector<std::string>& ids,
                                FILE* out) {
   if (out == NULL) {
@@ -92,7 +93,8 @@ void PrintDebuggerReadyMessage(int port,
   for (const std::string& id : ids) {
     fprintf(out,
             "    chrome-devtools://devtools/bundled/inspector.html?"
-            "experiments=true&v8only=true&ws=%s\n", GetWsUrl(port, id).c_str());
+            "experiments=true&v8only=true&ws=%s\n",
+            GetWsUrl(host, port, id).c_str());
   }
   fflush(out);
 }
@@ -229,9 +231,11 @@ class SocketSession {
 };
 
 InspectorSocketServer::InspectorSocketServer(SocketServerDelegate* delegate,
+                                             const std::string& host,
                                              int port,
                                              FILE* out) : loop_(nullptr),
                                                           delegate_(delegate),
+                                                          host_(host),
                                                           port_(port),
                                                           server_(uv_tcp_t()),
                                                           closer_(nullptr),
@@ -284,7 +288,7 @@ void InspectorSocketServer::SessionTerminated(int session_id) {
   delegate_->EndSession(session_id);
   if (connected_sessions_.empty() &&
       uv_is_active(reinterpret_cast<uv_handle_t*>(&server_))) {
-    PrintDebuggerReadyMessage(port_, delegate_->GetTargetIds(), out_);
+    PrintDebuggerReadyMessage(host_, port_, delegate_->GetTargetIds(), out_);
   }
 }
 
@@ -337,7 +341,7 @@ void InspectorSocketServer::SendListResponse(InspectorSocket* socket) {
       }
     }
     if (!connected) {
-      std::string address = GetWsUrl(port_, id);
+      std::string address = GetWsUrl(host_, port_, id);
       std::ostringstream frontend_url;
       frontend_url << "chrome-devtools://devtools/bundled";
       frontend_url << "/inspector.html?experiments=true&v8only=true&ws=";
@@ -353,7 +357,7 @@ bool InspectorSocketServer::Start(uv_loop_t* loop) {
   loop_ = loop;
   sockaddr_in addr;
   uv_tcp_init(loop_, &server_);
-  uv_ip4_addr("0.0.0.0", port_, &addr);
+  uv_ip4_addr(host_.c_str(), port_, &addr);
   int err = uv_tcp_bind(&server_,
                         reinterpret_cast<const struct sockaddr*>(&addr), 0);
   if (err == 0)
@@ -363,11 +367,13 @@ bool InspectorSocketServer::Start(uv_loop_t* loop) {
                     SocketConnectedCallback);
   }
   if (err == 0 && connected_sessions_.empty()) {
-    PrintDebuggerReadyMessage(port_, delegate_->GetTargetIds(), out_);
+    PrintDebuggerReadyMessage(host_, port_, delegate_->GetTargetIds(), out_);
   }
   if (err != 0 && connected_sessions_.empty()) {
     if (out_ != NULL) {
-      fprintf(out_, "Unable to open devtools socket: %s\n", uv_strerror(err));
+      fprintf(out_, "Starting inspector on %s:%d failed: %s\n",
+              host_.c_str(), port_, uv_strerror(err));
+      fflush(out_);
     }
     uv_close(reinterpret_cast<uv_handle_t*>(&server_), nullptr);
     return false;
diff --git a/src/inspector_socket_server.h b/src/inspector_socket_server.h
index 48ebc5353f..b82d9ee601 100644
--- a/src/inspector_socket_server.h
+++ b/src/inspector_socket_server.h
@@ -33,6 +33,7 @@ class InspectorSocketServer {
  public:
   using ServerCallback = void (*)(InspectorSocketServer*);
   InspectorSocketServer(SocketServerDelegate* delegate,
+                        const std::string& host,
                         int port,
                         FILE* out = stderr);
   bool Start(uv_loop_t* loop);
@@ -65,6 +66,7 @@ class InspectorSocketServer {
 
   uv_loop_t* loop_;
   SocketServerDelegate* const delegate_;
+  const std::string host_;
   int port_;
   std::string path_;
   uv_tcp_t server_;
diff --git a/src/node.cc b/src/node.cc
index 16250a8514..b6dd9b59d5 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -4362,7 +4362,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
   if (debug_enabled) {
     const char* path = argc > 1 ? argv[1] : nullptr;
     StartDebug(&env, path, debug_options);
-    if (debug_options.debugger_enabled() && !debugger_running)
+    if (!debugger_running)
       return 12;  // Signal internal error.
   }
 
diff --git a/test/cctest/test_inspector_socket_server.cc b/test/cctest/test_inspector_socket_server.cc
index e1e9f6f668..6fe8a7548f 100644
--- a/test/cctest/test_inspector_socket_server.cc
+++ b/test/cctest/test_inspector_socket_server.cc
@@ -8,6 +8,8 @@
 
 static uv_loop_t loop;
 
+static const char HOST[] = "127.0.0.1";
+
 static const char CLIENT_CLOSE_FRAME[] = "\x88\x80\x2D\x0E\x1E\xFA";
 static const char SERVER_CLOSE_FRAME[] = "\x88\x00";
 
@@ -249,7 +251,7 @@ class SocketWrapper {
   }
 
   static void Connected_(uv_connect_t* connect, int status) {
-    EXPECT_EQ(0, status);
+    EXPECT_EQ(0, status) << "Unable to connect: " << uv_strerror(status);
     SocketWrapper* wrapper =
         node::ContainerOf(&SocketWrapper::connect_, connect);
     wrapper->connected_ = true;
@@ -301,7 +303,7 @@ class ServerHolder {
   template <typename Delegate>
   ServerHolder(Delegate* delegate, int port, FILE* out = NULL)
                : closed(false), paused(false), sessions_terminated(false),
-                 server_(delegate, port, out) {
+                 server_(delegate, HOST, port, out) {
     delegate->Connect(&server_);
   }
 
@@ -362,7 +364,7 @@ class ServerDelegateNoTargets : public SocketServerDelegate {
 static void TestHttpRequest(int port, const std::string& path,
                             const std::string& expected_body) {
   SocketWrapper socket(&loop);
-  socket.Connect("0.0.0.0", port);
+  socket.Connect(HOST, port);
   socket.TestHttpRequest(path, expected_body);
   socket.Close();
 }
@@ -385,7 +387,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {
 
   SocketWrapper well_behaved_socket(&loop);
   // Regular connection
-  well_behaved_socket.Connect("0.0.0.0", server.port());
+  well_behaved_socket.Connect(HOST, server.port());
   well_behaved_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID));
   well_behaved_socket.Expect(WS_HANDSHAKE_RESPONSE);
 
@@ -408,7 +410,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {
 
   // Declined connection
   SocketWrapper declined_target_socket(&loop);
-  declined_target_socket.Connect("127.0.0.1", server.port());
+  declined_target_socket.Connect(HOST, server.port());
   declined_target_socket.Write(WsHandshakeRequest(UNCONNECTABLE_TARGET_ID));
   declined_target_socket.Expect("HTTP/1.0 400 Bad Request");
   declined_target_socket.ExpectEOF();
@@ -417,7 +419,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {
 
   // Bogus target - start session callback should not even be invoked
   SocketWrapper bogus_target_socket(&loop);
-  bogus_target_socket.Connect("127.0.0.1", server.port());
+  bogus_target_socket.Connect(HOST, server.port());
   bogus_target_socket.Write(WsHandshakeRequest("bogus_target"));
   bogus_target_socket.Expect("HTTP/1.0 400 Bad Request");
   bogus_target_socket.ExpectEOF();
@@ -426,7 +428,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {
 
   // Drop connection (no proper close frames)
   SocketWrapper dropped_connection_socket(&loop);
-  dropped_connection_socket.Connect("127.0.0.1", server.port());
+  dropped_connection_socket.Connect(HOST, server.port());
   dropped_connection_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID));
   dropped_connection_socket.Expect(WS_HANDSHAKE_RESPONSE);
 
@@ -440,7 +442,7 @@ TEST_F(InspectorSocketServerTest, InspectorSessions) {
 
   // Reconnect regular connection
   SocketWrapper stays_till_termination_socket(&loop);
-  stays_till_termination_socket.Connect("127.0.0.1", server.port());
+  stays_till_termination_socket.Connect(HOST, server.port());
   stays_till_termination_socket.Write(WsHandshakeRequest(MAIN_TARGET_ID));
   stays_till_termination_socket.Expect(WS_HANDSHAKE_RESPONSE);
 
@@ -484,7 +486,7 @@ TEST_F(InspectorSocketServerTest, ServerWithoutTargets) {
 
   // Declined connection
   SocketWrapper socket(&loop);
-  socket.Connect("0.0.0.0", server.port());
+  socket.Connect(HOST, server.port());
   socket.Write(WsHandshakeRequest(UNCONNECTABLE_TARGET_ID));
   socket.Expect("HTTP/1.0 400 Bad Request");
   socket.ExpectEOF();
@@ -512,7 +514,7 @@ TEST_F(InspectorSocketServerTest, StoppingServerDoesNotKillConnections) {
   ServerHolder server(&delegate, 0);
   ASSERT_TRUE(server->Start(&loop));
   SocketWrapper socket1(&loop);
-  socket1.Connect("0.0.0.0", server.port());
+  socket1.Connect(HOST, server.port());
   socket1.TestHttpRequest("/json/list", "[ ]");
   server->Stop(ServerHolder::CloseCallback);
   SPIN_WHILE(!server.closed);