Programatically setting the debug flag has no effects

[stelcheck]

• Version: 7.10.0
• Platform: any
• Subsystem: cluster

Given the following code:

const cluster = require('cluster')

if (cluster.isMaster) {
  cluster.setupMaster({
    execArgv: [
      '--debug=1337'
    ]
  })

  cluster.fork()
} else {
  setInterval(() => true, 1000)
}

I get the following output on the console at runtime:

Debugger listening on port 5859

The reason for this is because the current code assumes that whatever flag present in execArgv will be the same as the one being passed to the master process, and therefore extracts the initial port from process.debugPort instead (ref: https://github.com/nodejs/node/blob/master/lib/internal/cluster/master.js#L98-L119). It also assumes that the first port will be used to debug the master, and automatically increment the port for the first worker. Finally, given the submitted code, one would arguably expect no increments to happen at all; in the actual use-case where I wish to make sure of this pattern, I use cluster with one and only one worker at a time, so re-using the same port would be perfectly fine.

I would be more than happy to contribute a fix, but given the current behaviour and the fact that I don't know how I could actually distinguish programatic setup from the initial extraction of execArgv passed to the master process, I am having a bit of a hard time to figure out how to approach this issue. Suggestions more than welcome.

[ronkorving]

disclaimer: I'm a coworker of @stelcheck

It seems very reasonable to me to want to be able to set these on child processes instead of the master process, in situations like process managers etc. I would welcome a fix to this where if the flag was not set on the master process, the port value provided is used, and both process.debugPort and debugPortOffset are ignored.

[refack]

[Just my suggestion]
Add a second argument, opt, to cluster.fork() that will trickle down to createWorkerProcess and allow you to override all of the preset opts:

{
    env: workerEnv,
    silent: cluster.settings.silent,
    execArgv: execArgv,
    stdio: cluster.settings.stdio,
    gid: cluster.settings.gid,
    uid: cluster.settings.uid
}

(maybe not env because that will cause redundancy)

[refack]

[other option]
Get --inspect-port to be whitelisted in NODE_OPTIONS (#12028)

[sam-github]

--inspect-port should have been in NODE_OPTIONS: #13002

[sam-github]

I think what @stelcheck is trying to do is perfectly reasonable, and agree that with the special casing its a bit hard to get all the corner cases.

[arturgvieira-zz]

@refack Hi, I'd like to take this one. I've read over your suggestion, the related code, and made a gist. You mean something like this?
https://gist.github.com/arturgvieira/493772fb633acfb1c8dcaba2d94cfa12

[Trott]

@arturgvieira I could be wrong, but I suspect #13002 is the solution more likely to gain favor than adding a new argument to cluster.fork(). It hasn't landed yet, though, so I'm somewhat speculating.

(Because there's a likely solution proposed and undergoing active review, I'm going to remove the help wanted label from this issue.)