From 3aea4c884ba3b5fff1f497f1918be747a4aa3887 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Tue, 3 Oct 2017 20:51:12 -0700 Subject: [PATCH] src: fix ^ in stack trace with vm's columnOffset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.:1 throw new Error() ^ Error at evalmachine.: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.:1 throw new Error() ^ Error at evalmachine.:1:12 at ContextifyScript.Script.runInThisContext (vm.js:50:33) at Object.runInThisContext (vm.js:139:38) at repl:1:4 ``` PR-URL: https://github.com/nodejs/node/pull/15771 Refs: https://github.com/tmpvar/jsdom/pull/2003 Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott Reviewed-By: Colin Ihrig Reviewed-By: Tobias Nießen --- src/node.cc | 9 +++++++++ test/parallel/test-vm-context.js | 5 +++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index 6c1b11d4ca..9883a5c524 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1731,6 +1731,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(); @@ -1759,8 +1760,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; diff --git a/test/parallel/test-vm-context.js b/test/parallel/test-vm-context.js index 7e5404796e..29e3a86fab 100644 --- a/test/parallel/test-vm-context.js +++ b/test/parallel/test-vm-context.js @@ -93,13 +93,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