From b56a8061089ddf50af7408174c28097de4b01d73 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 15 Dec 2015 17:37:16 -0500 Subject: [PATCH 1/2] module,src: do not wrap modules with -1 lineOffset In b799a74709af69daf13901390df9428c4c38adfc and dfee4e3712ac4673b5fc472a8f77ac65bdc65f87 the module wrapping mechanism was changed for better error reporting. However, the changes causes issues with debuggers and profilers. This commit reverts the wrapping changes. Fixes: https://github.com/nodejs/node/issues/4297 --- lib/module.js | 2 +- src/node.js | 4 ++-- test/sequential/test-module-loading.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/module.js b/lib/module.js index 6a4cd885a3a3f5..8feba15b0fe7c0 100644 --- a/lib/module.js +++ b/lib/module.js @@ -372,7 +372,7 @@ Module.prototype._compile = function(content, filename) { var wrapper = Module.wrap(content); var compiledWrapper = runInThisContext(wrapper, - { filename: filename, lineOffset: -1 }); + { filename: filename, lineOffset: 0 }); if (global.v8debug) { if (!resolvedArgv) { // we enter the repl if we're not given a filename argument. diff --git a/src/node.js b/src/node.js index f1449fb34b4906..a19718dc36d7b4 100644 --- a/src/node.js +++ b/src/node.js @@ -958,7 +958,7 @@ }; NativeModule.wrapper = [ - '(function (exports, require, module, __filename, __dirname) {\n', + '(function (exports, require, module, __filename, __dirname) { ', '\n});' ]; @@ -968,7 +968,7 @@ var fn = runInThisContext(source, { filename: this.filename, - lineOffset: -1 + lineOffset: 0 }); fn(this.exports, NativeModule.require, this, this.filename); diff --git a/test/sequential/test-module-loading.js b/test/sequential/test-module-loading.js index a8696f84cd2ce9..43e667fa930300 100644 --- a/test/sequential/test-module-loading.js +++ b/test/sequential/test-module-loading.js @@ -281,9 +281,9 @@ assert.equal(42, require('../fixtures/utf8-bom.js')); assert.equal(42, require('../fixtures/utf8-bom.json')); // Error on the first line of a module should -// have the correct line and column number +// have the correct line number assert.throws(function() { require('../fixtures/test-error-first-line-offset.js'); }, function(err) { - return /test-error-first-line-offset.js:1:1/.test(err.stack); + return /test-error-first-line-offset.js:1:/.test(err.stack); }, 'Expected appearance of proper offset in Error stack'); From 2031e6067c9cc049b4c228485909ab1de91a5b5a Mon Sep 17 00:00:00 2001 From: cjihrig Date: Tue, 15 Dec 2015 17:38:00 -0500 Subject: [PATCH 2/2] test: add test for debugging one line files This commit adds a regression test for debugging of single line files. Refs: https://github.com/nodejs/node/issues/4297 --- test/fixtures/exports-function-with-param.js | 1 + test/parallel/test-vm-debug-context.js | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 test/fixtures/exports-function-with-param.js diff --git a/test/fixtures/exports-function-with-param.js b/test/fixtures/exports-function-with-param.js new file mode 100644 index 00000000000000..263b9064422108 --- /dev/null +++ b/test/fixtures/exports-function-with-param.js @@ -0,0 +1 @@ +module.exports = function foo(arg) { return arg; } diff --git a/test/parallel/test-vm-debug-context.js b/test/parallel/test-vm-debug-context.js index 2e86d136bea2fa..b648592eaea901 100644 --- a/test/parallel/test-vm-debug-context.js +++ b/test/parallel/test-vm-debug-context.js @@ -53,6 +53,24 @@ assert.strictEqual(vm.runInDebugContext(undefined), undefined); assert.equal(breaks, 1); })(); +// Can set listeners and breakpoints on a single line file +(function() { + const Debug = vm.runInDebugContext('Debug'); + const fn = require(common.fixturesDir + '/exports-function-with-param'); + let called = false; + + Debug.setListener(function(event, state, data) { + if (data.constructor.name === 'BreakEvent') { + called = true; + } + }); + + Debug.setBreakPoint(fn); + fn('foo'); + assert.strictEqual(Debug.showBreakPoints(fn), '(arg) { [B0]return arg; }'); + assert.strictEqual(called, true); +})(); + // See https://github.com/nodejs/node/issues/1190, fatal errors should not // crash the process. var script = common.fixturesDir + '/vm-run-in-debug-context.js';