Skip to content
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.

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

inspector: add support for uncaught exception #8043

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 54 additions & 0 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "platform/v8_inspector/public/V8Inspector.h"
#include "platform/v8_inspector/public/V8InspectorClient.h"
#include "platform/v8_inspector/public/V8InspectorSession.h"
#include "platform/v8_inspector/public/V8StackTrace.h"
#include "platform/inspector_protocol/FrontendChannel.h"
#include "platform/inspector_protocol/String16.h"
#include "platform/inspector_protocol/Values.h"
Expand Down Expand Up @@ -175,6 +176,9 @@ class AgentImpl {
bool IsConnected() { return state_ == State::kConnected; }
void WaitForDisconnect();

void FatalException(v8::Local<v8::Value> error,
v8::Local<v8::Message> message);

private:
using MessageQueue = std::vector<std::pair<int, String16>>;
enum class State { kNew, kAccepting, kConnected, kDone, kError };
Expand Down Expand Up @@ -334,6 +338,10 @@ class V8NodeInspector : public blink::V8InspectorClient {
session_->dispatchProtocolMessage(message);
}

blink::V8Inspector* inspector() {
return inspector_.get();
}

private:
AgentImpl* agent_;
v8::Isolate* isolate_;
Expand Down Expand Up @@ -495,6 +503,46 @@ void AgentImpl::InstallInspectorOnProcess() {
env->SetMethod(inspector, "wrapConsoleCall", InspectorWrapConsoleCall);
}

String16 ToProtocolString(v8::Local<v8::Value> value) {
if (value.IsEmpty() || value->IsNull() || value->IsUndefined() ||
!value->IsString()) {
return String16();
}
v8::Local<v8::String> string_value = v8::Local<v8::String>::Cast(value);
wstring buffer(string_value->Length(), '\0');
string_value->Write(&buffer[0], 0, string_value->Length());
return String16(buffer);
}

void AgentImpl::FatalException(v8::Local<v8::Value> error,
v8::Local<v8::Message> message) {
if (!IsStarted())
return;
auto env = parent_env_;
v8::Local<v8::Context> context = env->context();

int script_id = message->GetScriptOrigin().ScriptID()->Value();
std::unique_ptr<blink::V8StackTrace> stack_trace =
Copy link
Member

Choose a reason for hiding this comment

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

Can you replace the std::unique_ptr?

Copy link
Member Author

Choose a reason for hiding this comment

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

I need to get this pointer, use it to check scriptId and then pass it back to V8Inspector::exceptionThrown API call and I prefer to use unique_ptr here.
I can use auto instead of unique_ptr to hide real type of stackTrace. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like @ofrobots is right that it picks up a non-system std::unique_ptr because this PR builds for me on OS X 10.8. If you want to restore the other use, go ahead.

inspector_->inspector()->createStackTrace(message->GetStackTrace());

if (stack_trace && !stack_trace->isEmpty() &&
String16::fromInteger(script_id) == stack_trace->topScriptId()) {
script_id = 0;
}

inspector_->inspector()->exceptionThrown(
context,
"Uncaught",
error,
ToProtocolString(message->Get()),
ToProtocolString(message->GetScriptResourceName()),
message->GetLineNumber(context).FromMaybe(0),
message->GetStartColumn(context).FromMaybe(0),
std::move(stack_trace),
script_id);
WaitForDisconnect();
}

// static
void AgentImpl::ThreadCbIO(void* agent) {
static_cast<AgentImpl*>(agent)->WorkerRunIO();
Expand Down Expand Up @@ -714,5 +762,11 @@ void Agent::WaitForDisconnect() {
impl->WaitForDisconnect();
}

void Agent::FatalException(v8::Local<v8::Value> error,
v8::Local<v8::Message> message) {
impl->FatalException(error, message);
}


} // namespace inspector
} // namespace node
7 changes: 7 additions & 0 deletions src/inspector_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ class Environment;

namespace v8 {
class Platform;
template<typename T>
class Local;
class Value;
class Message;
} // namespace v8

namespace node {
Expand All @@ -32,6 +36,9 @@ class Agent {
bool IsStarted();
bool IsConnected();
void WaitForDisconnect();

void FatalException(v8::Local<v8::Value> error,
v8::Local<v8::Message> message);
private:
AgentImpl* impl;
};
Expand Down
40 changes: 26 additions & 14 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2464,31 +2464,43 @@ void FatalException(Isolate* isolate,
Local<Function> fatal_exception_function =
process_object->Get(fatal_exception_string).As<Function>();

int exit_code = 0;
if (!fatal_exception_function->IsFunction()) {
// failed before the process._fatalException function was added!
// this is probably pretty bad. Nothing to do but report and exit.
ReportException(env, error, message);
exit(6);
exit_code = 6;
}

TryCatch fatal_try_catch(isolate);
if (exit_code == 0) {
TryCatch fatal_try_catch(isolate);

// Do not call FatalException when _fatalException handler throws
fatal_try_catch.SetVerbose(false);
// Do not call FatalException when _fatalException handler throws
fatal_try_catch.SetVerbose(false);

// this will return true if the JS layer handled it, false otherwise
Local<Value> caught =
fatal_exception_function->Call(process_object, 1, &error);
// this will return true if the JS layer handled it, false otherwise
Local<Value> caught =
fatal_exception_function->Call(process_object, 1, &error);

if (fatal_try_catch.HasCaught()) {
// the fatal exception function threw, so we must exit
ReportException(env, fatal_try_catch);
exit(7);
if (fatal_try_catch.HasCaught()) {
// the fatal exception function threw, so we must exit
ReportException(env, fatal_try_catch);
exit_code = 7;
}

if (exit_code == 0 && false == caught->BooleanValue()) {
ReportException(env, error, message);
exit_code = 1;
}
}

if (false == caught->BooleanValue()) {
ReportException(env, error, message);
exit(1);
if (exit_code) {
#if HAVE_INSPECTOR
if (use_inspector) {
env->inspector_agent()->FatalException(error, message);
}
#endif
exit(exit_code);
Copy link
Member

Choose a reason for hiding this comment

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

Missing brace? Either that or the indentation is off.

}
}

Expand Down