Skip to content

Commit

Permalink
inspector: no crash when WS server can't start
Browse files Browse the repository at this point in the history 
This change also changes error message to make it consistent with the
one printed by the debugger.

Fixes: nodejs#10858
PR-URL: nodejs#10878
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
Eugene Ostroukhov committed Jan 20, 2017
1 parent 974ecb0 commit ba776b3
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 22 deletions.
5 changes: 3 additions & 2 deletions src/inspector_agent.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down
24 changes: 15 additions & 9 deletions src/inspector_socket_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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_);
}
}

Expand Down Expand Up @@ -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=";
Expand All @@ -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)
Expand All @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/inspector_socket_server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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_;
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}

Expand Down
22 changes: 12 additions & 10 deletions test/cctest/test_inspector_socket_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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_);
}

Expand Down Expand Up @@ -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();
}
Expand All @@ -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);

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ba776b3

Please sign in to comment.