Skip to content

Commit 760d089

Browse files
joyeecheungBethGriggs
joyeecheung
authored and
BethGriggs
committed
inspector: display error when ToggleAsyncHook fails
This patch refactors `AppendExceptionLine` and `PrintSyncTrace` to reuse the error formatting logic and use them to print uncaught error in ``ToggleAsyncHook` PR-URL: #26859 Refs: #26798 Reviewed-By: Matteo Collina <[email protected]> Signed-off-by: Beth Griggs <[email protected]>
1 parent e776b01 commit 760d089

File tree

4 files changed

+129
-88
lines changed

4 files changed

+129
-88
lines changed

‎src/env.cc‎

Lines changed: 6 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,12 @@ using v8::HandleScope;
3333
using v8::Integer;
3434
using v8::Isolate;
3535
using v8::Local;
36-
using v8::Message;
3736
using v8::NewStringType;
3837
using v8::Number;
3938
using v8::Object;
4039
using v8::Private;
4140
using v8::Promise;
4241
using v8::PromiseHookType;
43-
using v8::StackFrame;
4442
using v8::StackTrace;
4543
using v8::String;
4644
using v8::Symbol;
@@ -492,48 +490,15 @@ void Environment::StopProfilerIdleNotifier() {
492490
}
493491

494492
void Environment::PrintSyncTrace() const {
495-
if (!options_->trace_sync_io)
496-
return;
493+
if (!options_->trace_sync_io) return;
497494

498495
HandleScope handle_scope(isolate());
499-
Local<StackTrace> stack =
500-
StackTrace::CurrentStackTrace(isolate(), 10, StackTrace::kDetailed);
501-
502-
fprintf(stderr, "(node:%d) WARNING: Detected use of sync API\n",
503-
uv_os_getpid());
504-
505-
for (int i = 0; i < stack->GetFrameCount() - 1; i++) {
506-
Local<StackFrame> stack_frame = stack->GetFrame(isolate(), i);
507-
node::Utf8Value fn_name_s(isolate(), stack_frame->GetFunctionName());
508-
node::Utf8Value script_name(isolate(), stack_frame->GetScriptName());
509-
const int line_number = stack_frame->GetLineNumber();
510-
const int column = stack_frame->GetColumn();
511-
512-
if (stack_frame->IsEval()) {
513-
if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) {
514-
fprintf(stderr, " at [eval]:%i:%i\n", line_number, column);
515-
} else {
516-
fprintf(stderr,
517-
" at [eval] (%s:%i:%i)\n",
518-
*script_name,
519-
line_number,
520-
column);
521-
}
522-
break;
523-
}
524496

525-
if (fn_name_s.length() == 0) {
526-
fprintf(stderr, " at %s:%i:%i\n", *script_name, line_number, column);
527-
} else {
528-
fprintf(stderr,
529-
" at %s (%s:%i:%i)\n",
530-
*fn_name_s,
531-
*script_name,
532-
line_number,
533-
column);
534-
}
535-
}
536-
fflush(stderr);
497+
fprintf(
498+
stderr, "(node:%d) WARNING: Detected use of sync API\n", uv_os_getpid());
499+
PrintStackTrace(
500+
isolate(),
501+
StackTrace::CurrentStackTrace(isolate(), 10, StackTrace::kDetailed));
537502
}
538503

539504
void Environment::RunCleanup() {

‎src/inspector_agent.cc‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -839,11 +839,12 @@ void Agent::ToggleAsyncHook(Isolate* isolate,
839839
HandleScope handle_scope(isolate);
840840
CHECK(!fn.IsEmpty());
841841
auto context = parent_env_->context();
842-
auto result = fn.Get(isolate)->Call(context, Undefined(isolate), 0, nullptr);
843-
if (result.IsEmpty()) {
844-
FatalError(
845-
"node::inspector::Agent::ToggleAsyncHook",
846-
"Cannot toggle Inspector's AsyncHook, please report this.");
842+
v8::TryCatch try_catch(isolate);
843+
USE(fn.Get(isolate)->Call(context, Undefined(isolate), 0, nullptr));
844+
if (try_catch.HasCaught()) {
845+
PrintCaughtException(isolate, context, try_catch);
846+
FatalError("\nnode::inspector::Agent::ToggleAsyncHook",
847+
"Cannot toggle Inspector's AsyncHook, please report this.");
847848
}
848849
}
849850

‎src/node_errors.cc‎

Lines changed: 112 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@ using v8::Local;
2121
using v8::Maybe;
2222
using v8::MaybeLocal;
2323
using v8::Message;
24-
using v8::NewStringType;
2524
using v8::Number;
2625
using v8::Object;
2726
using v8::ScriptOrigin;
27+
using v8::StackFrame;
28+
using v8::StackTrace;
2829
using v8::String;
2930
using v8::Undefined;
3031
using v8::Value;
@@ -44,30 +45,20 @@ namespace per_process {
4445
static Mutex tty_mutex;
4546
} // namespace per_process
4647

47-
void AppendExceptionLine(Environment* env,
48-
Local<Value> er,
49-
Local<Message> message,
50-
enum ErrorHandlingMode mode) {
51-
if (message.IsEmpty()) return;
48+
static const int kMaxErrorSourceLength = 1024;
5249

53-
HandleScope scope(env->isolate());
54-
Local<Object> err_obj;
55-
if (!er.IsEmpty() && er->IsObject()) {
56-
err_obj = er.As<Object>();
57-
}
50+
static std::string GetErrorSource(Isolate* isolate,
51+
Local<Context> context,
52+
Local<Message> message,
53+
bool* added_exception_line) {
54+
MaybeLocal<String> source_line_maybe = message->GetSourceLine(context);
55+
node::Utf8Value encoded_source(isolate, source_line_maybe.ToLocalChecked());
56+
std::string sourceline(*encoded_source, encoded_source.length());
5857

59-
// Print (filename):(line number): (message).
60-
ScriptOrigin origin = message->GetScriptOrigin();
61-
node::Utf8Value filename(env->isolate(), message->GetScriptResourceName());
62-
const char* filename_string = *filename;
63-
int linenum = message->GetLineNumber(env->context()).FromJust();
64-
// Print line of source code.
65-
MaybeLocal<String> source_line_maybe = message->GetSourceLine(env->context());
66-
node::Utf8Value sourceline(env->isolate(),
67-
source_line_maybe.ToLocalChecked());
68-
const char* sourceline_string = *sourceline;
69-
if (strstr(sourceline_string, "node-do-not-add-exception-line") != nullptr)
70-
return;
58+
if (sourceline.find("node-do-not-add-exception-line") != std::string::npos) {
59+
*added_exception_line = false;
60+
return sourceline;
61+
}
7162

7263
// Because of how node modules work, all scripts are wrapped with a
7364
// "function (module, exports, __filename, ...) {"
@@ -90,53 +81,131 @@ void AppendExceptionLine(Environment* env,
9081
// sourceline to 78 characters, and we end up not providing very much
9182
// useful debugging info to the user if we remove 62 characters.
9283

84+
// Print (filename):(line number): (message).
85+
ScriptOrigin origin = message->GetScriptOrigin();
86+
node::Utf8Value filename(isolate, message->GetScriptResourceName());
87+
const char* filename_string = *filename;
88+
int linenum = message->GetLineNumber(context).FromJust();
89+
9390
int script_start = (linenum - origin.ResourceLineOffset()->Value()) == 1
9491
? origin.ResourceColumnOffset()->Value()
9592
: 0;
96-
int start = message->GetStartColumn(env->context()).FromMaybe(0);
97-
int end = message->GetEndColumn(env->context()).FromMaybe(0);
93+
int start = message->GetStartColumn(context).FromMaybe(0);
94+
int end = message->GetEndColumn(context).FromMaybe(0);
9895
if (start >= script_start) {
9996
CHECK_GE(end, start);
10097
start -= script_start;
10198
end -= script_start;
10299
}
103100

104-
char arrow[1024];
105-
int max_off = sizeof(arrow) - 2;
101+
int max_off = kMaxErrorSourceLength - 2;
106102

107-
int off = snprintf(arrow,
108-
sizeof(arrow),
103+
char buf[kMaxErrorSourceLength];
104+
int off = snprintf(buf,
105+
kMaxErrorSourceLength,
109106
"%s:%i\n%s\n",
110107
filename_string,
111108
linenum,
112-
sourceline_string);
109+
sourceline.c_str());
113110
CHECK_GE(off, 0);
114111
if (off > max_off) {
115112
off = max_off;
116113
}
117114

118115
// Print wavy underline (GetUnderline is deprecated).
119116
for (int i = 0; i < start; i++) {
120-
if (sourceline_string[i] == '\0' || off >= max_off) {
117+
if (sourceline[i] == '\0' || off >= max_off) {
121118
break;
122119
}
123120
CHECK_LT(off, max_off);
124-
arrow[off++] = (sourceline_string[i] == '\t') ? '\t' : ' ';
121+
buf[off++] = (sourceline[i] == '\t') ? '\t' : ' ';
125122
}
126123
for (int i = start; i < end; i++) {
127-
if (sourceline_string[i] == '\0' || off >= max_off) {
124+
if (sourceline[i] == '\0' || off >= max_off) {
128125
break;
129126
}
130127
CHECK_LT(off, max_off);
131-
arrow[off++] = '^';
128+
buf[off++] = '^';
132129
}
133130
CHECK_LE(off, max_off);
134-
arrow[off] = '\n';
135-
arrow[off + 1] = '\0';
131+
buf[off] = '\n';
132+
buf[off + 1] = '\0';
133+
134+
*added_exception_line = true;
135+
return std::string(buf);
136+
}
137+
138+
void PrintStackTrace(Isolate* isolate, Local<StackTrace> stack) {
139+
for (int i = 0; i < stack->GetFrameCount() - 1; i++) {
140+
Local<StackFrame> stack_frame = stack->GetFrame(isolate, i);
141+
node::Utf8Value fn_name_s(isolate, stack_frame->GetFunctionName());
142+
node::Utf8Value script_name(isolate, stack_frame->GetScriptName());
143+
const int line_number = stack_frame->GetLineNumber();
144+
const int column = stack_frame->GetColumn();
145+
146+
if (stack_frame->IsEval()) {
147+
if (stack_frame->GetScriptId() == Message::kNoScriptIdInfo) {
148+
fprintf(stderr, " at [eval]:%i:%i\n", line_number, column);
149+
} else {
150+
fprintf(stderr,
151+
" at [eval] (%s:%i:%i)\n",
152+
*script_name,
153+
line_number,
154+
column);
155+
}
156+
break;
157+
}
158+
159+
if (fn_name_s.length() == 0) {
160+
fprintf(stderr, " at %s:%i:%i\n", *script_name, line_number, column);
161+
} else {
162+
fprintf(stderr,
163+
" at %s (%s:%i:%i)\n",
164+
*fn_name_s,
165+
*script_name,
166+
line_number,
167+
column);
168+
}
169+
}
170+
fflush(stderr);
171+
}
172+
173+
void PrintCaughtException(Isolate* isolate,
174+
Local<Context> context,
175+
const v8::TryCatch& try_catch) {
176+
CHECK(try_catch.HasCaught());
177+
Local<Value> err = try_catch.Exception();
178+
Local<Message> message = try_catch.Message();
179+
Local<v8::StackTrace> stack = message->GetStackTrace();
180+
node::Utf8Value reason(isolate,
181+
err->ToDetailString(context).ToLocalChecked());
182+
bool added_exception_line = false;
183+
std::string source =
184+
GetErrorSource(isolate, context, message, &added_exception_line);
185+
fprintf(stderr, "%s\n", source.c_str());
186+
fprintf(stderr, "%s\n", *reason);
187+
PrintStackTrace(isolate, stack);
188+
}
189+
190+
void AppendExceptionLine(Environment* env,
191+
Local<Value> er,
192+
Local<Message> message,
193+
enum ErrorHandlingMode mode) {
194+
if (message.IsEmpty()) return;
195+
196+
HandleScope scope(env->isolate());
197+
Local<Object> err_obj;
198+
if (!er.IsEmpty() && er->IsObject()) {
199+
err_obj = er.As<Object>();
200+
}
136201

137-
Local<String> arrow_str =
138-
String::NewFromUtf8(env->isolate(), arrow, NewStringType::kNormal)
139-
.ToLocalChecked();
202+
bool added_exception_line = false;
203+
std::string source = GetErrorSource(
204+
env->isolate(), env->context(), message, &added_exception_line);
205+
if (!added_exception_line) {
206+
return;
207+
}
208+
MaybeLocal<Value> arrow_str = ToV8Value(env->context(), source);
140209

141210
const bool can_set_arrow = !arrow_str.IsEmpty() && !err_obj.IsEmpty();
142211
// If allocating arrow_str failed, print it out. There's not much else to do.
@@ -150,13 +219,14 @@ void AppendExceptionLine(Environment* env,
150219
env->set_printed_error(true);
151220

152221
uv_tty_reset_mode();
153-
PrintErrorString("\n%s", arrow);
222+
PrintErrorString("\n%s", source.c_str());
154223
return;
155224
}
156225

157226
CHECK(err_obj
158-
->SetPrivate(
159-
env->context(), env->arrow_message_private_symbol(), arrow_str)
227+
->SetPrivate(env->context(),
228+
env->arrow_message_private_symbol(),
229+
arrow_str.ToLocalChecked())
160230
.FromMaybe(false));
161231
}
162232

‎src/node_internals.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ void GetSockOrPeerName(const v8::FunctionCallbackInfo<v8::Value>& args) {
8686
args.GetReturnValue().Set(err);
8787
}
8888

89+
void PrintStackTrace(v8::Isolate* isolate, v8::Local<v8::StackTrace> stack);
90+
void PrintCaughtException(v8::Isolate* isolate,
91+
v8::Local<v8::Context> context,
92+
const v8::TryCatch& try_catch);
93+
8994
void WaitForInspectorDisconnect(Environment* env);
9095
void SignalExit(int signo);
9196
#ifdef __POSIX__

0 commit comments

Comments
 (0)