Skip to content

Commit

Permalink
src: fix ^ in stack trace with vm's columnOffset
Browse files Browse the repository at this point in the history
While VM module's columnOffset option does succeed in applying an offset
to the column number in the stack trace, the wavy diagram printed does
not account for potential offsets, resulting in erroneous location of
`^` in the first line of the script.

Before:

```
> vm.runInThisContext('throw new Error()', { columnOffset: 5 })
evalmachine.<anonymous>:1
throw new Error()
     ^

Error
    at evalmachine.<anonymous>:1:12
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at Object.runInThisContext (vm.js:116:38)
```

After:

```
> vm.runInThisContext('throw new Error()', { columnOffset: 5 })
evalmachine.<anonymous>:1
throw new Error()
^

Error
    at evalmachine.<anonymous>:1:12
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at Object.runInThisContext (vm.js:139:38)
    at repl:1:4
```

PR-URL: #15771
Refs: jsdom/jsdom#2003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
TimothyGu authored and MylesBorins committed Oct 25, 2017
1 parent eaec35d commit 931addb
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
9 changes: 9 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,7 @@ void AppendExceptionLine(Environment* env,
}

// Print (filename):(line number): (message).
ScriptOrigin origin = message->GetScriptOrigin();
node::Utf8Value filename(env->isolate(), message->GetScriptResourceName());
const char* filename_string = *filename;
int linenum = message->GetLineNumber();
Expand Down Expand Up @@ -1613,8 +1614,16 @@ void AppendExceptionLine(Environment* env,
// sourceline to 78 characters, and we end up not providing very much
// useful debugging info to the user if we remove 62 characters.

int script_start =
(linenum - origin.ResourceLineOffset()->Value()) == 1 ?
origin.ResourceColumnOffset()->Value() : 0;
int start = message->GetStartColumn(env->context()).FromMaybe(0);
int end = message->GetEndColumn(env->context()).FromMaybe(0);
if (start >= script_start) {
CHECK_GE(end, start);
start -= script_start;
end -= script_start;
}

char arrow[1024];
int max_off = sizeof(arrow) - 2;
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-vm-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,14 @@ assert.strictEqual(script.runInContext(ctx), false);
// Error on the first line of a module should
// have the correct line and column number
assert.throws(() => {
vm.runInContext('throw new Error()', context, {
vm.runInContext(' throw new Error()', context, {
filename: 'expected-filename.js',
lineOffset: 32,
columnOffset: 123
});
}, (err) => {
return /expected-filename\.js:33:130/.test(err.stack);
return /^ \^/m.test(err.stack) &&
/expected-filename\.js:33:131/.test(err.stack);
}, 'Expected appearance of proper offset in Error stack');

// https://github.com/nodejs/node/issues/6158
Expand Down

0 comments on commit 931addb

Please sign in to comment.