Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit dfee4e3

Browse filesBrowse files
tflanagancjihrig
tflanagan
authored and
cjihrig
committed
module: fix column offsets in errors
Because Node modules are wrapped, errors on the first line of a file leak the wrapper to the user and report the wrong column number. This commit adds a line break to the module wrapper so that the first line is treated the same as all other lines. To compensate for the additional line, a line offset of -1 is also applied to errors. Fixes: #2860 PR-URL: #2867 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 94c3507 commit dfee4e3
Copy full SHA for dfee4e3

File tree

Expand file treeCollapse file tree

7 files changed

+78
-6
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+78
-6
lines changed
Open diff view settings
Collapse file

‎doc/api/vm.markdown‎

Copy file name to clipboardExpand all lines: doc/api/vm.markdown
+18-2Lines changed: 18 additions & 2 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,16 @@ The options when creating a script are:
2626

2727
- `filename`: allows you to control the filename that shows up in any stack
2828
traces produced from this script.
29+
- `lineOffset`: allows you to add an offset to the line number that is
30+
displayed in stack traces
31+
- `columnOffset`: allows you to add an offset to the column number that is
32+
displayed in stack traces
2933
- `displayErrors`: whether or not to print any errors to stderr, with the
3034
line of code that caused them highlighted, before throwing an exception.
3135
Applies only to syntax errors compiling the code; errors while running the
3236
code are controlled by the options to the script's methods.
37+
- `timeout`: a number of milliseconds to execute `code` before terminating
38+
execution. If execution is terminated, an `Error` will be thrown.
3339

3440
### script.runInContext(contextifiedSandbox[, options])
3541

@@ -124,8 +130,14 @@ multiple times:
124130

125131
The options for running a script are:
126132

127-
- `displayErrors`: whether or not to print any runtime errors to stderr, with
128-
the line of code that caused them highlighted, before throwing an exception.
133+
- `filename`: allows you to control the filename that shows up in any stack
134+
traces produced.
135+
- `lineOffset`: allows you to add an offset to the line number that is
136+
displayed in stack traces
137+
- `columnOffset`: allows you to add an offset to the column number that is
138+
displayed in stack traces
139+
- `displayErrors`: whether or not to print any errors to stderr, with the
140+
line of code that caused them highlighted, before throwing an exception.
129141
Applies only to runtime errors executing the code; it is impossible to create
130142
a `Script` instance with syntax errors, as the constructor will throw.
131143
- `timeout`: a number of milliseconds to execute the script before terminating
@@ -252,6 +264,10 @@ e.g. `(0,eval)('code')`. However, it also has the following additional options:
252264

253265
- `filename`: allows you to control the filename that shows up in any stack
254266
traces produced.
267+
- `lineOffset`: allows you to add an offset to the line number that is
268+
displayed in stack traces
269+
- `columnOffset`: allows you to add an offset to the column number that is
270+
displayed in stack traces
255271
- `displayErrors`: whether or not to print any errors to stderr, with the
256272
line of code that caused them highlighted, before throwing an exception.
257273
Will capture both syntax errors from compiling `code` and runtime errors
Collapse file

‎lib/module.js‎

Copy file name to clipboardExpand all lines: lib/module.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ Module.wrapper = NativeModule.wrapper;
4747
Module.wrap = NativeModule.wrap;
4848
Module._debug = util.debuglog('module');
4949

50-
5150
// We use this alias for the preprocessor that filters it out
5251
const debug = Module._debug;
5352

@@ -401,7 +400,8 @@ Module.prototype._compile = function(content, filename) {
401400
// create wrapper function
402401
var wrapper = Module.wrap(content);
403402

404-
var compiledWrapper = runInThisContext(wrapper, { filename: filename });
403+
var compiledWrapper = runInThisContext(wrapper,
404+
{ filename: filename, lineOffset: -1 });
405405
if (global.v8debug) {
406406
if (!resolvedArgv) {
407407
// we enter the repl if we're not given a filename argument.
Collapse file

‎src/node.js‎

Copy file name to clipboardExpand all lines: src/node.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -955,7 +955,7 @@
955955
};
956956

957957
NativeModule.wrapper = [
958-
'(function (exports, require, module, __filename, __dirname) { ',
958+
'(function (exports, require, module, __filename, __dirname) {\n',
959959
'\n});'
960960
];
961961

Collapse file

‎src/node_contextify.cc‎

Copy file name to clipboardExpand all lines: src/node_contextify.cc
+36-1Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,13 +504,15 @@ class ContextifyScript : public BaseObject {
504504
TryCatch try_catch;
505505
Local<String> code = args[0]->ToString(env->isolate());
506506
Local<String> filename = GetFilenameArg(args, 1);
507+
Local<Integer> lineOffset = GetLineOffsetArg(args, 1);
508+
Local<Integer> columnOffset = GetColumnOffsetArg(args, 1);
507509
bool display_errors = GetDisplayErrorsArg(args, 1);
508510
if (try_catch.HasCaught()) {
509511
try_catch.ReThrow();
510512
return;
511513
}
512514

513-
ScriptOrigin origin(filename);
515+
ScriptOrigin origin(filename, lineOffset, columnOffset);
514516
ScriptCompiler::Source source(code, origin);
515517
Local<UnboundScript> v8_script =
516518
ScriptCompiler::CompileUnbound(env->isolate(), &source);
@@ -675,6 +677,39 @@ class ContextifyScript : public BaseObject {
675677
}
676678

677679

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

‎test/fixtures/test-error-first-line-offset.js‎

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
error
Collapse file

‎test/parallel/test-vm-context.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-vm-context.js
+12Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,15 @@ var ctx = {};
6060
Object.defineProperty(ctx, 'b', { configurable: false });
6161
ctx = vm.createContext(ctx);
6262
assert.equal(script.runInContext(ctx), false);
63+
64+
// Error on the first line of a module should
65+
// have the correct line and column number
66+
assert.throws(function() {
67+
vm.runInContext('throw new Error()', context, {
68+
filename: 'expected-filename.js',
69+
lineOffset: 32,
70+
columnOffset: 123
71+
});
72+
}, function(err) {
73+
return /expected-filename.js:33:130/.test(err.stack);
74+
}, 'Expected appearance of proper offset in Error stack');
Collapse file

‎test/sequential/test-module-loading.js‎

Copy file name to clipboardExpand all lines: test/sequential/test-module-loading.js
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,3 +279,11 @@ process.on('exit', function() {
279279
// #1440 Loading files with a byte order marker.
280280
assert.equal(42, require('../fixtures/utf8-bom.js'));
281281
assert.equal(42, require('../fixtures/utf8-bom.json'));
282+
283+
// Error on the first line of a module should
284+
// have the correct line and column number
285+
assert.throws(function() {
286+
require('../fixtures/test-error-first-line-offset.js');
287+
}, function(err) {
288+
return /test-error-first-line-offset.js:1:1/.test(err.stack);
289+
}, 'Expected appearance of proper offset in Error stack');

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.