Skip to content

Commit 76c8ac5

Browse files
committed
module: fix error first line col offset
1 parent 7f5d6c9 commit 76c8ac5

File tree

6 files changed

+80
-2
lines changed

6 files changed

+80
-2
lines changed

doc/api/vm.markdown

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ e.g. `(0,eval)('code')`. However, it also has the following additional options:
4141

4242
- `filename`: allows you to control the filename that shows up in any stack
4343
traces produced.
44+
- `lineOffset`: allows you to add an offset to the line number that is
45+
displayed in stack traces
46+
- `columnOffset`: allows you to add an offset to the column number that is
47+
displayed in stack traces
4448
- `displayErrors`: whether or not to print any errors to stderr, with the
4549
line of code that caused them highlighted, before throwing an exception.
4650
Will capture both syntax errors from compiling `code` and runtime errors

lib/module.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ Module.wrapper = NativeModule.wrapper;
4343
Module.wrap = NativeModule.wrap;
4444
Module._debug = util.debuglog('module');
4545

46+
const moduleWrapperColOffset = Module.wrapper[0].length;
4647

4748
// We use this alias for the preprocessor that filters it out
4849
const debug = Module._debug;
@@ -410,7 +411,8 @@ Module.prototype._compile = function(content, filename) {
410411
// create wrapper function
411412
var wrapper = Module.wrap(content);
412413

413-
var compiledWrapper = runInThisContext(wrapper, { filename: filename });
414+
var compiledWrapper = runInThisContext(wrapper,
415+
{ filename: filename, columnOffset: -moduleWrapperColOffset });
414416
if (global.v8debug) {
415417
if (!resolvedArgv) {
416418
// we enter the repl if we're not given a filename argument.

src/node_contextify.cc

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,13 +503,15 @@ class ContextifyScript : public BaseObject {
503503
TryCatch try_catch;
504504
Local<String> code = args[0]->ToString(env->isolate());
505505
Local<String> filename = GetFilenameArg(args, 1);
506+
Local<Integer> lineOffset = GetLineOffsetArg(args, 1);
507+
Local<Integer> columnOffset = GetColumnOffsetArg(args, 1);
506508
bool display_errors = GetDisplayErrorsArg(args, 1);
507509
if (try_catch.HasCaught()) {
508510
try_catch.ReThrow();
509511
return;
510512
}
511513

512-
ScriptOrigin origin(filename);
514+
ScriptOrigin origin(filename, lineOffset, columnOffset);
513515
ScriptCompiler::Source source(code, origin);
514516
Local<UnboundScript> v8_script =
515517
ScriptCompiler::CompileUnbound(env->isolate(), &source);
@@ -674,6 +676,39 @@ class ContextifyScript : public BaseObject {
674676
}
675677

676678

679+
static Local<Integer> GetLineOffsetArg(
680+
const FunctionCallbackInfo<Value>& args,
681+
const int i) {
682+
Local<Integer> defaultLineOffset = Integer::New(args.GetIsolate(), 0);
683+
684+
if (!args[i]->IsObject()) {
685+
return defaultLineOffset;
686+
}
687+
688+
Local<String> key = FIXED_ONE_BYTE_STRING(args.GetIsolate(), "lineOffset");
689+
Local<Value> value = args[i].As<Object>()->Get(key);
690+
691+
return value->IsUndefined() ? defaultLineOffset : value->ToInteger();
692+
}
693+
694+
695+
static Local<Integer> GetColumnOffsetArg(
696+
const FunctionCallbackInfo<Value>& args,
697+
const int i) {
698+
Local<Integer> defaultColumnOffset = Integer::New(args.GetIsolate(), 0);
699+
700+
if (!args[i]->IsObject()) {
701+
return defaultColumnOffset;
702+
}
703+
704+
Local<String> key = FIXED_ONE_BYTE_STRING(args.GetIsolate(),
705+
"columnOffset");
706+
Local<Value> value = args[i].As<Object>()->Get(key);
707+
708+
return value->IsUndefined() ? defaultColumnOffset : value->ToInteger();
709+
}
710+
711+
677712
static bool EvalMachine(Environment* env,
678713
const int64_t timeout,
679714
const bool display_errors,
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
error

test/parallel/test-vm-context.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,23 @@ var ctx = {};
6060
Object.defineProperty(ctx, 'b', { configurable: false });
6161
ctx = vm.createContext(ctx);
6262
assert.equal(script.runInContext(ctx), false);
63+
64+
// Issue GH-2860
65+
// Error on the first line of a module should
66+
// have the correct line and column number
67+
var err;
68+
69+
try {
70+
vm.runInContext('throw new Error()', context, {
71+
filename: 'expected-filename.js',
72+
lineOffset: 32,
73+
columnOffset: 123
74+
});
75+
} catch (e) {
76+
err = e;
77+
78+
assert.ok(/expected-filename.js:33:130/.test(err.stack),
79+
'Expected appearance of proper offset in Error stack');
80+
}
81+
82+
assert.ok(err, 'expected exception from runInContext offset test');

test/sequential/test-module-loading.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,3 +283,19 @@ process.on('exit', function() {
283283
// #1440 Loading files with a byte order marker.
284284
assert.equal(42, require('../fixtures/utf8-bom.js'));
285285
assert.equal(42, require('../fixtures/utf8-bom.json'));
286+
287+
// Issue GH-2860
288+
// Error on the first line of a module should
289+
// have the correct line and column number
290+
var err;
291+
292+
try {
293+
require('../fixtures/test-error-first-line-offset.js');
294+
} catch (e) {
295+
err = e;
296+
297+
assert.ok(/test-error-first-line-offset.js:1:1/.test(err.stack),
298+
'Expected appearance of proper offset in Error stack');
299+
}
300+
301+
assert.ok(err, 'expected exception from runInContext offset test');

0 commit comments

Comments
 (0)