From ba776b3a56642d4cade5fa26c27c654bb6dde759 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Wed, 18 Jan 2017 10:18:32 -0800 Subject: [PATCH] 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 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- src/inspector_agent.cc | 5 +++-- src/inspector_socket_server.cc | 24 +++++++++++++-------- src/inspector_socket_server.h | 2 ++ src/node.cc | 2 +- test/cctest/test_inspector_socket_server.cc | 22 ++++++++++--------- 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index 403afe0e120b97..ae2e666384b5af 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(&io_thread_req_), nullptr); uv_loop_close(&child_loop_); diff --git a/src/inspector_socket_server.cc b/src/inspector_socket_server.cc index 5308e9bcf0b924..ee44a69a683690 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& 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(&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(&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(&server_), nullptr); return false; diff --git a/src/inspector_socket_server.h b/src/inspector_socket_server.h index 48ebc5353f123d..b82d9ee6013a40 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 16250a85140d35..b6dd9b59d57a2e 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 e1e9f6f66865d3..6fe8a7548fd4a5 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 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);