Skip to content

A couple of minor UX fixes. #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

A couple of minor UX fixes. #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f47f147
V8 inspector: provide filenames in the form of URLs to the debugger
pavelfeldman May 11, 2016
ffc30e3
Wait for debugger to disconnect after app is done.
pavelfeldman May 11, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/internal/bootstrap_node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
// wrap it
source = Module.wrap(source);
// compile the script, this will throw if it fails
new vm.Script(source, {filename: filename, displayErrors: true});
new vm.Script(source, {filename: 'file://' + filename, displayErrors: true});
process.exit(0);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ Module.prototype._compile = function(content, filename) {
var wrapper = Module.wrap(content);

var compiledWrapper = vm.runInThisContext(wrapper, {
filename: filename,
filename: 'file://' + filename,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a reasonable change. This changes Node.js stack traces in an unacceptable way. Other scenarios, where the file is actually is stdin or an eval are also broken. Examples:

ReferenceError: print is not defined
    at Object.<anonymous> (file:///Users/ofrobots/tmp/6691.js:1:71)
    at Module._compile (module.js:539:32)
    at Object.Module._extensions..js (module.js:548:10)
    at Module.load (module.js:456:32)
    at tryModuleLoad (module.js:415:12)
    at Function.Module._load (module.js:407:3)
    at Function.Module.runMain (module.js:573:10)
    at startup (node.js:164:18)
    at node.js:449:3

and

throw new Error("hello")
^
Error: hello
    at [stdin]:1:7
    at Object.exports.runInThisContext (vm.js:54:17)
    at Object.<anonymous> (file://[stdin]-wrapper:6:22)
    at Module._compile (module.js:539:32)
    at node.js:333:29
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

lineOffset: 0,
displayErrors: true
});
Expand Down
24 changes: 23 additions & 1 deletion src/inspector_agent.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class AsyncWriteRequest {
AsyncWriteRequest(Agent* agent, const String16& message) :
agent_(agent), message_(message) {}
void perform() {
inspector_socket_t* socket = agent_->client_socket();
inspector_socket_t* socket = agent_->client_socket_;
if (socket) {
inspector_write(socket, message_.utf8().c_str(), message_.length());
}
Expand All @@ -94,6 +94,21 @@ class AsyncWriteRequest {
const String16 message_;
};

class SetConnectedTask : public v8::Task {
public:
SetConnectedTask(Agent* agent, bool connected)
: agent_(agent),
connected_(connected) {}

void Run() override {
agent_->connected_ = connected_;
}

private:
Agent* agent_;
bool connected_;
};

static void DisposeAsyncCb(uv_handle_t* handle) {
free(handle);
}
Expand Down Expand Up @@ -171,6 +186,7 @@ static void SendTargentsListResponse(inspector_socket_t* socket) {

Agent::Agent(Environment* env) : port_(5858),
wait_(false),
connected_(false),
parent_env_(env),
client_socket_(nullptr),
platform_(nullptr) {
Expand Down Expand Up @@ -248,6 +264,10 @@ void Agent::OnRemoteData(uv_stream_t* stream, ssize_t read, const uv_buf_t* b) {
agent->client_socket_ = nullptr;
}
DisconnectAndDispose(socket);
} else {
// EOF
agent->platform_->CallOnForegroundThread(agent->parent_env()->isolate(),
new SetConnectedTask(agent, false));
}
}

Expand All @@ -273,6 +293,8 @@ void Agent::OnInspectorConnection(inspector_socket_t* socket) {
client_socket_ = socket;
inspector_read_start(socket, OnBufferAlloc, OnRemoteData);
uv_sem_post(&start_sem_);
platform_->CallOnForegroundThread(parent_env()->isolate(),
new SetConnectedTask(this, true));
}

bool Agent::RespondToGet(inspector_socket_t* socket, const char* path) {
Expand Down
6 changes: 5 additions & 1 deletion src/inspector_agent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class Agent {
// Stop the inspector agent
void Stop();

inspector_socket_t* client_socket() { return client_socket_; }
bool connected() { return connected_; }

protected:
inline node::Environment* parent_env() { return parent_env_; }
void InitAdaptor(Environment* env);
Expand All @@ -52,6 +53,7 @@ class Agent {

int port_;
bool wait_;
bool connected_;

uv_thread_t thread_;
node::Environment* parent_env_;
Expand Down Expand Up @@ -82,6 +84,8 @@ class Agent {

friend class ChannelImpl;
friend class DispatchOnInspectorBackendTask;
friend class SetConnectedTask;
friend class AsyncWriteRequest;
};

} // namespace inspector
Expand Down
3 changes: 3 additions & 0 deletions src/inspector_socket.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ static ws_decode_result decode_frame_hybi17(const char* buffer_begin,

size_t pos = p + actual_masking_key_length + payload_length - buffer_begin;
*bytes_consumed = pos;

return closed ? FRAME_CLOSE : FRAME_OK;
}

Expand Down Expand Up @@ -319,6 +320,7 @@ static void on_close_frame_written(uv_write_t* write, int status) {
static void close_frame_received(inspector_socket_t* inspector) {
inspector->ws_state->received_close = true;
if (!inspector->ws_state->close_sent) {
invoke_read_callback(inspector, 0, 0);
write_to_client(inspector, CLOSE_FRAME, sizeof(CLOSE_FRAME),
on_close_frame_written);
} else {
Expand Down Expand Up @@ -423,6 +425,7 @@ int inspector_read_start(inspector_socket_t* inspector,
inspector->ws_state->close_sent = false;
inspector->ws_state->alloc_cb = alloc_cb;
inspector->ws_state->read_cb = read_cb;
inspector->ws_state->read_cb = read_cb;
int err =
uv_read_start(reinterpret_cast<uv_stream_t*>(&inspector->client),
prepare_buffer,
Expand Down
9 changes: 9 additions & 0 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4430,9 +4430,18 @@ static void StartNodeInstance(void* arg) {
if (uv_run(env->event_loop(), UV_RUN_NOWAIT) != 0)
more = true;
}

} while (more == true);
}

#if HAVE_INSPECTOR
if (env->inspector_agent()->connected())
fprintf(stderr, "Waiting for the debugger to disconnect...\n");
while (env->inspector_agent()->connected()) {
v8::platform::PumpMessageLoop(default_platform, isolate);
}
#endif

env->set_trace_sync_io(false);

int exit_code = EmitExit(env);
Expand Down