Skip to content

Commit fb25c91

Browse files
committed
src: fix JSError inspection with stringified stack
When the `stack` property of an Error object is accessed in V8, a stringified version of the stack is generated, and then the "raw stack" (with FP and arguments) is removed from memory. Our current inspection code couldn't handle this because it was not checking if the raw stack was a valid object. This commit makes that code more robust and thus fixes inspection of error objects with stringified stacks. PR-URL: #291 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 9449d99 commit fb25c91

File tree

4 files changed

+52
-16
lines changed

4 files changed

+52
-16
lines changed

src/llv8.cc

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,6 +1271,13 @@ StackTrace::Iterator StackTrace::end() {
12711271

12721272
StackTrace::StackTrace(JSArray frame_array, Error& err)
12731273
: frame_array_(frame_array) {
1274+
if (!frame_array.Check()) {
1275+
PRINT_DEBUG("JS Array is not a valid object");
1276+
len_ = -1;
1277+
multiplier_ = -1;
1278+
return;
1279+
}
1280+
12741281
v8::Value maybe_stack_len = frame_array.GetArrayElement(0, err);
12751282

12761283
if (err.Fail()) {

src/printer.cc

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -526,27 +526,31 @@ std::string Printer::Stringify(v8::JSError js_error, Error& err) {
526526
if (options_.detailed) {
527527
output << StringifyJSObjectFields(js_error, err);
528528

529-
v8::StackTrace stack_trace = js_error.GetStackTrace(err);
529+
if (js_error.HasStackTrace(err)) {
530+
v8::StackTrace stack_trace = js_error.GetStackTrace(err);
530531

531-
std::stringstream error_stack;
532-
error_stack << std::endl
533-
<< rang::fg::red << " error stack" << rang::fg::reset << " {"
534-
<< std::endl;
532+
std::stringstream error_stack;
533+
error_stack << std::endl
534+
<< rang::fg::red << " error stack" << rang::fg::reset << " {"
535+
<< std::endl;
535536

536-
Printer printer(llv8_);
537-
for (v8::StackFrame frame : stack_trace) {
538-
v8::JSFunction js_function = frame.GetFunction(err);
539-
if (err.Fail()) {
540-
error_stack << rang::fg::gray << " <unknown>" << std::endl;
541-
continue;
537+
538+
Printer printer(llv8_);
539+
for (v8::StackFrame frame : stack_trace) {
540+
v8::JSFunction js_function = frame.GetFunction(err);
541+
if (err.Fail()) {
542+
error_stack << rang::fg::gray << " <unknown>" << std::endl;
543+
continue;
544+
}
545+
546+
error_stack << " "
547+
<< printer.Stringify<v8::HeapObject>(js_function, err)
548+
<< std::endl;
542549
}
543550

544-
error_stack << " "
545-
<< printer.Stringify<v8::HeapObject>(js_function, err)
546-
<< std::endl;
551+
error_stack << " }";
552+
output << error_stack.str();
547553
}
548-
error_stack << " }";
549-
output << error_stack.str();
550554
}
551555

552556
output << rang::fg::yellow << ">" << rang::fg::reset;

test/fixtures/inspect-scenario.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ function closure() {
6464
c.hashmap['error'].code = 'ERR_TEST';
6565
c.hashmap['error'].errno = 1;
6666

67+
c.hashmap['stringifiedError'] = new Error('test');
68+
c.hashmap['stringifiedErrorStack'] = c.hashmap['stringifiedError'].stack;
69+
6770
c.hashmap[0] = null;
6871
c.hashmap[4] = undefined;
6972
c.hashmap[23] = /regexp/;

test/plugin/inspect-test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,28 @@ const hashMapTests = {
161161
});
162162
}
163163
},
164+
// .stringifiedError=0x0000392d5d661119:<Object: Error>
165+
'error': {
166+
re: /.stringifiedError=(0x[0-9a-f]+):<Object: Error>/,
167+
desc: '.stringifiedError Error property with stringified stack',
168+
validator(t, sess, addresses, name, cb) {
169+
const address = addresses[name];
170+
sess.send(`v8 inspect ${address}`);
171+
172+
sess.linesUntil(/}>/, (err, lines) => {
173+
if (err) return cb(err);
174+
lines = lines.join('\n');
175+
176+
let strStackMatch = lines.match(/stack=(0x[0-9a-f]+):<String: /i);
177+
t.ok(strStackMatch, 'hashmap.stringifiedError should have stringified stack');
178+
179+
let stackMatch = lines.match(/error stack {/i);
180+
t.notOk(stackMatch, 'Error object with stringified stack should not have an error stack');
181+
182+
cb(null);
183+
});
184+
}
185+
},
164186
// .array=0x000003df9cbe7919:<Array: length=6>,
165187
'array': {
166188
re: /.array=(0x[0-9a-f]+):<Array: length=6>/,

0 commit comments

Comments
 (0)