Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 931addb

Browse filesBrowse files
TimothyGuMylesBorins
authored andcommitted
src: fix ^ in stack trace with vm's columnOffset
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>
1 parent eaec35d commit 931addb
Copy full SHA for 931addb

File tree

Expand file treeCollapse file tree

2 files changed

+12
-2
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+12
-2
lines changed
Open diff view settings
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,6 +1585,7 @@ void AppendExceptionLine(Environment* env,
15851585
}
15861586

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

1617+
int script_start =
1618+
(linenum - origin.ResourceLineOffset()->Value()) == 1 ?
1619+
origin.ResourceColumnOffset()->Value() : 0;
16161620
int start = message->GetStartColumn(env->context()).FromMaybe(0);
16171621
int end = message->GetEndColumn(env->context()).FromMaybe(0);
1622+
if (start >= script_start) {
1623+
CHECK_GE(end, start);
1624+
start -= script_start;
1625+
end -= script_start;
1626+
}
16181627

16191628
char arrow[1024];
16201629
int max_off = sizeof(arrow) - 2;
Collapse file

‎test/parallel/test-vm-context.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-vm-context.js
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,14 @@ assert.strictEqual(script.runInContext(ctx), false);
7272
// Error on the first line of a module should
7373
// have the correct line and column number
7474
assert.throws(() => {
75-
vm.runInContext('throw new Error()', context, {
75+
vm.runInContext(' throw new Error()', context, {
7676
filename: 'expected-filename.js',
7777
lineOffset: 32,
7878
columnOffset: 123
7979
});
8080
}, (err) => {
81-
return /expected-filename\.js:33:130/.test(err.stack);
81+
return /^ \^/m.test(err.stack) &&
82+
/expected-filename\.js:33:131/.test(err.stack);
8283
}, 'Expected appearance of proper offset in Error stack');
8384

8485
// https://github.com/nodejs/node/issues/6158

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.