Repository: node
Issue: cluster: forked children eagerly ignore/replace inspect port from cluster settings
Labels: cluster

* **Version**: 8.11.3
* **Platform**: Linux 4.13.0-46-generic #<span></span>51-Ubuntu SMP Tue Jun 12 12:36:29 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
* **Subsystem**: cluster

## The Setup
Here's a small example that can be used to show this behavior:
```js
const cluster = require('cluster');

const args = [
		process.argv[0],
		...process.execArgv,
		...process.argv.slice(1)
	],
	prefix = cluster.isMaster ? '[MASTER]' : '[WORKER]';

console.log(prefix, ...args);

if (cluster.isMaster) {
	const options = {
		execArgv: [],
		args: ['worker']
	};

	if (process.execArgv.some(arg => arg.startsWith('--inspect'))) {
		options.execArgv.push('--inspect=20002')
	}

	cluster.setupMaster(options);

	cluster.fork();
}
```
In this example, when the master process has been started with any of the `--inspect` arguments, we want to start the worker with the inspector active on a _specific_ port. Here's some example output:
```
$ node --inspect=10001 cluster.js master
Debugger listening on ws://127.0.0.1:10001/cb5774d6-7c8b-4a91-93fd-4c700a5349f4
For help see https://nodejs.org/en/docs/inspector
[MASTER] /usr/bin/node --inspect=10001 cluster.js master
Debugger listening on ws://127.0.0.1:10002/ffb4f474-3e4d-42c7-a8d7-45f0b19d9924
For help see https://nodejs.org/en/docs/inspector
[WORKER] /usr/bin/node --inspect=20002 --inspect-port=10002 cluster.js worker
```
What can be seen, here, is that the worker is forked with the configured argument from the cluster options' `execArgv`; but, the `--inspect-port=10002` argument is _also_ present, though it wasn't configured.

## The References
### The `cluster` Module Code
The code that does this appears to be the `createWorkerProcess(id, env)` function at [/lib/internal/cluster/master.js:101](https://github.com/nodejs/node/blob/v8.x/lib/internal/cluster/master.js#L101). From this, we can see that this behavior appears to be intentional, at least as far as what the code says. Here's what I read from there:
1. If any of the cluster's configured `execArgv` arguments look like `--inspect`, `--inspect-brk`, `--inspect-port` or `--debug-port`...
    1. If an `inspectPort` is defined in the cluster settings...
        1. Get its value or run it as a getter function, saving the value as `inspectPort`.
    1. ...else...
        1. increment the master's debug port and save that value as `inspectPort`.
    1. Push `` `--inspect-port=${inspectPort}` `` into the worker's `execArgv` array.

### The Documentation
In [the CLI docs](https://nodejs.org/dist/latest-v8.x/docs/api/cli.html#cli_inspect_host_port), all three of `--inspect`, `--inspect-brk` and `--inspect-port` are indicated to support the `[=[host:]port]` value. However, _conceptually_, based on what they each do, it seems like they would **never be used together**.

In [the `cluster` docs](https://nodejs.org/dist/latest-v8.x/docs/api/cluster.html#cluster_cluster_settings), `settings.inspectPort` is indicated to set the inspect port of the worker.

## The Problems
So, here are the problems, as I see them:
* Initially, without reading the code, it would seem that setting `settings.inspectPort` would allow one to configure the inspect port; **however**, that setting _is completely ignored_ unless `execArgv` already has one of the inspect arguments present (which _could be_ `--inspect-port`, resulting in two of the same argument).
* When there _is_ an inspect argument present in `execArgv`, the selected `inspectPort` value ultimately results in the addition of the `--inspect-port` option; **however**, in the documentation, that argument is indicated to only configure what the inspect port _will_ be when/if inspection is latently activated.
* While it seems, conceptually, that each of the inspect arguments would be mutually exclusive in practice, understandably there must be an order of precedence when they are combined; **however**, the `cluster` code _explicitly combines them_ by intentionally adding `--inspect-port` when it _knows_ there's already an inspect argument present.

Ultimately, the only option one has to be able to configure the inspect port for a worker is to **both** (1) add one of the inspect arguments _and_ (2) set the `settings.inspectPort`. This will result in `execArgv` having something like `--inspect --inspect-port=####`. In that case, one might as well leave off the port from the original argument as it will be overridden by the added one.

All of this is fairly confusing to me. Why would the `cluster` code intentionally combine inspect arguments? If there is a precedence, it's opaque (i.e. it's not indicated in the documentation). Is this operating as expected? Am I just missing something and thinking about this "wrong"?

------
P.S. The code for this [in **10.x**](https://github.com/nodejs/node/blob/v10.x/lib/internal/cluster/master.js#L102), while it has received some changes, does still appear to exhibit this behavior -- though I have not tested it myself.