From 931addba0b4c63259a40d8d12f71cf4ffdeb377a 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 882c72a67eb330..f4218ca6795933 100644 --- a/src/node.cc +++ b/src/node.cc @@ -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(); @@ -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; diff --git a/test/parallel/test-vm-context.js b/test/parallel/test-vm-context.js index 33116f52bb08e7..a9cf7a1c8adab5 100644 --- a/test/parallel/test-vm-context.js +++ b/test/parallel/test-vm-context.js @@ -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