module: fix error first line col offset

nodejs · Sep 14, 2015 · 76c8ac5 · 76c8ac5
1 parent 7f5d6c9
commit 76c8ac5
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 2 deletions.
diff --git 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
@@ -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
@@ -503,13 +503,15 @@ class ContextifyScript : public BaseObject {
     TryCatch try_catch;
     Local<String> code = args[0]->ToString(env->isolate());
     Local<String> filename = GetFilenameArg(args, 1);
+    Local<Integer> lineOffset = GetLineOffsetArg(args, 1);
+    Local<Integer> 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<UnboundScript> v8_script =
         ScriptCompiler::CompileUnbound(env->isolate(), &source);
@@ -674,6 +676,39 @@ class ContextifyScript : public BaseObject {
   }
 
 
+  static Local<Integer> GetLineOffsetArg(
+                                      const FunctionCallbackInfo<Value>& args,
+                                      const int i) {
+    Local<Integer> defaultLineOffset = Integer::New(args.GetIsolate(), 0);
+
+    if (!args[i]->IsObject()) {
+      return defaultLineOffset;
+    }
+
+    Local<String> key = FIXED_ONE_BYTE_STRING(args.GetIsolate(), "lineOffset");
+    Local<Value> value = args[i].As<Object>()->Get(key);
+
+    return value->IsUndefined() ? defaultLineOffset : value->ToInteger();
+  }
+
+
+  static Local<Integer> GetColumnOffsetArg(
+                                      const FunctionCallbackInfo<Value>& args,
+                                      const int i) {
+    Local<Integer> defaultColumnOffset = Integer::New(args.GetIsolate(), 0);
+
+    if (!args[i]->IsObject()) {
+      return defaultColumnOffset;
+    }
+
+    Local<String> key = FIXED_ONE_BYTE_STRING(args.GetIsolate(),
+                                              "columnOffset");
+    Local<Value> value = args[i].As<Object>()->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
@@ -0,0 +1 @@
+error
diff --git 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
@@ -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');