Skip to content

Commit

Permalink
src: reduce redundant "error:" for inspect and findrefs
Browse files Browse the repository at this point in the history
When `inspect` or `findrefs` an invalid expr, llnode will print
an error message prefixed with "error: error: error:". This PR
will reduce one redundant "error:" for such scenario and it
seems that we can't remove the other redundant "error:" by
calling SB API only.

PR-URL: #285
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
  • Loading branch information
oyyd authored and mmarchini committed Jul 19, 2019
1 parent 640799a commit 74414fc
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 13 deletions.
6 changes: 2 additions & 4 deletions src/llnode.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,8 @@ bool PrintCmd::DoExecute(SBDebugger d, char** cmd,
SBExpressionOptions options;
SBValue value = target.EvaluateExpression(full_cmd.c_str(), options);
if (value.GetError().Fail()) {
SBStream desc;
if (value.GetError().GetDescription(desc)) {
result.SetError(desc.GetData());
}
SBError error = value.GetError();
result.SetError(error);
result.SetStatus(eReturnStatusFailed);
return false;
}
Expand Down
6 changes: 2 additions & 4 deletions src/llscan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -504,10 +504,8 @@ bool FindReferencesCmd::DoExecute(SBDebugger d, char** cmd,
SBExpressionOptions options;
SBValue value = target.EvaluateExpression(full_cmd.c_str(), options);
if (value.GetError().Fail()) {
SBStream desc;
if (value.GetError().GetDescription(desc)) {
result.SetError(desc.GetData());
}
SBError error = value.GetError();
result.SetError(error);
result.SetStatus(eReturnStatusFailed);
return false;
}
Expand Down
16 changes: 15 additions & 1 deletion test/plugin/inspect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ function verifyHashMap(t, sess) {
const parent = 'hashmap';
const addresses = collectMembers(
t, lines.join('\n'), hashMapTests, parent);
verifyMembers(t, sess, addresses, hashMapTests, parent, teardown);
verifyMembers(t, sess, addresses, hashMapTests, parent, verifyInvalidExpr);
});
}

Expand Down Expand Up @@ -614,6 +614,20 @@ function verifyMembers(t, sess, addresses, tests, parent, next) {
verifyProperty(t, sess, addresses, 0);
}

function verifyInvalidExpr(t, sess) {
sess.send('v8 inspect invalid_expr');
sess.waitError(/error:/, (err, line) => {
if (err) {
return teardown(t, sess, err);
}
t.ok(
/error: error: use of undeclared identifier 'invalid_expr'/.test(line),
'invalid expression should return an error'
);
teardown(t, sess);
});
}

tape('v8 inspect', (t) => {
t.timeoutAfter(15000);

Expand Down
23 changes: 19 additions & 4 deletions test/plugin/scan-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ tape('v8 findrefs and friends', (t) => {
}
});

function testFindrefsForInvalidExpr(t, sess, next) {
sess.send('v8 findrefs invalid_expr');
sess.waitError(/error:/, (err, line) => {
t.error(err);
t.ok(
/error: error: use of undeclared identifier 'invalid_expr'/.test(line),
'invalid expression should return an error'
);
next();
});
}

function test(executable, core, t) {
const sess = common.Session.loadCore(executable, core, (err) => {
t.error(err);
Expand All @@ -32,7 +44,6 @@ function test(executable, core, t) {
sess.send('version');
});


sess.linesUntil(versionMark, (err, lines) => {
t.error(err);
t.ok(/\d+ Class/.test(lines.join('\n')), 'Class should be in findjsobjects');
Expand Down Expand Up @@ -157,7 +168,6 @@ function test(executable, core, t) {
sess.send(`v8 findrefs ${match[1]}`);
}
t.ok(found, 'Zlib should be in findjsinstances');

// Just a separator
sess.send('version');
});
Expand All @@ -181,8 +191,13 @@ function test(executable, core, t) {
t.error(err)
if(hasSymbol)
t.ok(/Context\.scopedAPI/.test(lines.join('\n')), 'Should find reference #4');
sess.quit();
t.end();

// `waitError()` don't share the same `waitQueue` with `wait()` so that
// we add the function below to delay the event registration.
testFindrefsForInvalidExpr(t, sess, () => {
sess.quit();
t.end();
});
});
});
}

0 comments on commit 74414fc

Please sign in to comment.