From 76c8ac597ea093cc9fecd6da51cbf061201dc562 Mon Sep 17 00:00:00 2001 From: Tristian Flanagan Date: Mon, 14 Sep 2015 18:26:59 -0400 Subject: [PATCH] module: fix error first line col offset --- doc/api/vm.markdown | 4 ++ lib/module.js | 4 +- src/node_contextify.cc | 37 ++++++++++++++++++- test/fixtures/test-error-first-line-offset.js | 1 + test/parallel/test-vm-context.js | 20 ++++++++++ test/sequential/test-module-loading.js | 16 ++++++++ 6 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/test-error-first-line-offset.js diff --git a/doc/api/vm.markdown b/doc/api/vm.markdown index dd2593ecb7a7b2..cdcb4d71c39a35 100644 --- a/doc/api/vm.markdown +++ b/doc/api/vm.markdown @@ -41,6 +41,10 @@ e.g. `(0,eval)('code')`. However, it also has the following additional options: - `filename`: allows you to control the filename that shows up in any stack traces produced. +- `lineOffset`: allows you to add an offset to the line number that is + displayed in stack traces +- `columnOffset`: allows you to add an offset to the column number that is + displayed in stack traces - `displayErrors`: whether or not to print any errors to stderr, with the line of code that caused them highlighted, before throwing an exception. Will capture both syntax errors from compiling `code` and runtime errors diff --git a/lib/module.js b/lib/module.js index da8a906f951541..bc1ba26d229d4c 100644 --- a/lib/module.js +++ b/lib/module.js @@ -43,6 +43,7 @@ Module.wrapper = NativeModule.wrapper; Module.wrap = NativeModule.wrap; Module._debug = util.debuglog('module'); +const moduleWrapperColOffset = Module.wrapper[0].length; // We use this alias for the preprocessor that filters it out const debug = Module._debug; @@ -410,7 +411,8 @@ Module.prototype._compile = function(content, filename) { // create wrapper function var wrapper = Module.wrap(content); - var compiledWrapper = runInThisContext(wrapper, { filename: filename }); + var compiledWrapper = runInThisContext(wrapper, + { filename: filename, columnOffset: -moduleWrapperColOffset }); if (global.v8debug) { if (!resolvedArgv) { // we enter the repl if we're not given a filename argument. diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 6520355239ea84..c70f74ffb2aac2 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -503,13 +503,15 @@ class ContextifyScript : public BaseObject { TryCatch try_catch; Local code = args[0]->ToString(env->isolate()); Local filename = GetFilenameArg(args, 1); + Local lineOffset = GetLineOffsetArg(args, 1); + Local columnOffset = GetColumnOffsetArg(args, 1); bool display_errors = GetDisplayErrorsArg(args, 1); if (try_catch.HasCaught()) { try_catch.ReThrow(); return; } - ScriptOrigin origin(filename); + ScriptOrigin origin(filename, lineOffset, columnOffset); ScriptCompiler::Source source(code, origin); Local v8_script = ScriptCompiler::CompileUnbound(env->isolate(), &source); @@ -674,6 +676,39 @@ class ContextifyScript : public BaseObject { } + static Local GetLineOffsetArg( + const FunctionCallbackInfo& args, + const int i) { + Local defaultLineOffset = Integer::New(args.GetIsolate(), 0); + + if (!args[i]->IsObject()) { + return defaultLineOffset; + } + + Local key = FIXED_ONE_BYTE_STRING(args.GetIsolate(), "lineOffset"); + Local value = args[i].As()->Get(key); + + return value->IsUndefined() ? defaultLineOffset : value->ToInteger(); + } + + + static Local GetColumnOffsetArg( + const FunctionCallbackInfo& args, + const int i) { + Local defaultColumnOffset = Integer::New(args.GetIsolate(), 0); + + if (!args[i]->IsObject()) { + return defaultColumnOffset; + } + + Local key = FIXED_ONE_BYTE_STRING(args.GetIsolate(), + "columnOffset"); + Local value = args[i].As()->Get(key); + + return value->IsUndefined() ? defaultColumnOffset : value->ToInteger(); + } + + static bool EvalMachine(Environment* env, const int64_t timeout, const bool display_errors, diff --git a/test/fixtures/test-error-first-line-offset.js b/test/fixtures/test-error-first-line-offset.js new file mode 100644 index 00000000000000..9aeeab047d22c3 --- /dev/null +++ b/test/fixtures/test-error-first-line-offset.js @@ -0,0 +1 @@ +error diff --git a/test/parallel/test-vm-context.js b/test/parallel/test-vm-context.js index 45e19e6638892e..f94dadfd83711d 100644 --- a/test/parallel/test-vm-context.js +++ b/test/parallel/test-vm-context.js @@ -60,3 +60,23 @@ var ctx = {}; Object.defineProperty(ctx, 'b', { configurable: false }); ctx = vm.createContext(ctx); assert.equal(script.runInContext(ctx), false); + +// Issue GH-2860 +// Error on the first line of a module should +// have the correct line and column number +var err; + +try { + vm.runInContext('throw new Error()', context, { + filename: 'expected-filename.js', + lineOffset: 32, + columnOffset: 123 + }); +} catch (e) { + err = e; + + assert.ok(/expected-filename.js:33:130/.test(err.stack), + 'Expected appearance of proper offset in Error stack'); +} + +assert.ok(err, 'expected exception from runInContext offset test'); diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js index dfca2e420263b6..abc77954d9a406 100644 --- a/test/sequential/test-module-loading.js +++ b/test/sequential/test-module-loading.js @@ -283,3 +283,19 @@ process.on('exit', function() { // #1440 Loading files with a byte order marker. assert.equal(42, require('../fixtures/utf8-bom.js')); assert.equal(42, require('../fixtures/utf8-bom.json')); + +// Issue GH-2860 +// Error on the first line of a module should +// have the correct line and column number +var err; + +try { + require('../fixtures/test-error-first-line-offset.js'); +} catch (e) { + err = e; + + assert.ok(/test-error-first-line-offset.js:1:1/.test(err.stack), + 'Expected appearance of proper offset in Error stack'); +} + +assert.ok(err, 'expected exception from runInContext offset test');