Node 11.4.0+ fails to resolve index.js when combined with -r and --inspect-brk #25967
Comments
|
Using node -r ./preload.js --inspect-brk ./index.jsNot suggesting that requiring |
|
Some further debugging (using fixtures in Node.js 11.4.0: In 11.3.0 The following testcase fails when run with Node.js v11.4.0 and passes with v11.3.0 (I have it in // Flags: --expose-internals
'use strict';
const common = require('../common');
common.skipIfInspectorDisabled();
const { path: fixture } = require('../common/fixtures');
const { NodeInstance } = require('../common/inspector-helper.js');
const assert = require('assert');
const { join, relative } = require('path');
async function runTest() {
// Script argument should be relative (https://github.com/nodejs/node/issues/25967)
const child = new NodeInstance(
['-r', fixture('printA.js'), '--inspect-brk=0'],
null,
relative(join(__dirname, '..', '..'), fixture('printB.js'))
);
const session = await child.connectInspectorSession();
const commands = [
{ 'method': 'Runtime.enable' },
{ 'method': 'Runtime.runIfWaitingForDebugger' }
];
session.send(commands);
for (const expected of ['A', 'B']) {
const msg = await session.waitForNotification('Runtime.consoleAPICalled');
assert.strictEqual(msg.params.type, 'log');
assert.deepStrictEqual(msg.params.args, [{
type: 'string',
value: expected
}]);
}
session.disconnect();
}
runTest(); |
|
@richardhoehn I found the issue. Before 7b8058a there was this // Make process.argv[1] into a full path.
const path = NativeModule.require('path');
process.argv[1] = path.resolve(process.argv[1]);
const CJSModule = NativeModule.require('internal/modules/cjs/loader');
preloadModules();after it is: function prepareUserCodeExecution() {
// If this is a worker in cluster mode, start up the communication
// channel. This needs to be done before any user code gets executed
// (including preload modules).
if (process.argv[1] && process.env.NODE_UNIQUE_ID) {
const cluster = NativeModule.require('cluster');
cluster._setupWorker();
// Make sure it's not accidentally inherited by child processes.
delete process.env.NODE_UNIQUE_ID;
}
// For user code, we preload modules if `-r` is passed
// TODO(joyeecheung): use internal/options instead of
// process._preload_modules
if (process._preload_modules) {
const {
_preloadModules
} = NativeModule.require('internal/modules/cjs/loader');
_preloadModules(process._preload_modules);
}
}So it's missing setting // Make process.argv[1] into a full path.
const path = NativeModule.require('path');
process.argv[1] = path.resolve(process.argv[1]);I manually patched this in and can confirm it fixed the issue. function prepareUserCodeExecution() {
// Make process.argv[1] into a full path.
const path = NativeModule.require('path');
process.argv[1] = path.resolve(process.argv[1]); |
|
@jdalton I could fix it as part of #26000 but v11 would need a separate change before other refactorings are backported if it's urgent. |
|
Since it's a regression from all earlier (pre 11.4.0) supported Node versions it'd be great to have this fixed for 11.x. and 12. A fix in #26000 for v12 and then another one-off for Node 11.x? |
And make sure that `process.argv` from the preloaded modules is the same as the one in the main module. Refs: nodejs#25967
|
@jdalton Fixing it ad-hoc on v11-staging right now may interfere with backports of other pending backports to v11.x-staging e.g. this would be in conflict with #25667 so it depends on how urgent this is and whether it is worth the conflict I guess. At least for now the branch only gets hit when |
Would fixing it after the pending backports to v11 be an option? I don't see it as urgent enough to complicate pending things but would be nice to have at some 11.x. Update: Fix for Node 12 landed as 69714ab. |
And make sure that `process.argv` from the preloaded modules is the same as the one in the main module. Refs: nodejs#25967
And make sure that `process.argv` from the preloaded modules is the same as the one in the main module. Refs: #25967 PR-URL: #26000 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
And make sure that `process.argv` from the preloaded modules is the same as the one in the main module. Refs: #25967 PR-URL: #26000 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
index.js
preload.js
In Node 11.3.0 and below this works:
and results in:
However, in Node 11.4.0+ it errors with:
The text was updated successfully, but these errors were encountered: