Debug SocketCluster in VS Code

MegaGM commented 5 years ago

What's wrong?

Visual Studio Code doesn't respect breakpoints in child processes, such as workers and brokers, scworkercluster and sc-broker-cluster, when I try to debug SocketCluster with VS Code's internal debugger.

How could it be fixed?

We could change a couple of lines of code, a line in socketcluster and a line in sc-broker

from

execOptions.execArgv.push('--inspect=' + inspectPort);

to

execOptions.execArgv.push('--inspect-brk=' + inspectPort);

and breakpoints will work as expected.

What do I propose?

To add a couple of new CLI arguments.
--inspect-brk-workers and
--inspect-brk-brokers

Checklist

[ ] The new CLI arguments should accept the same options as the original --inspect-brk
```
--inspect-brk-workers
--inspect-brk-workers=port
--inspect-brk-workers=host:port
```

--inspect-brk-brokers --inspect-brk-brokers=port --inspect-brk-brokers=host:port

- [ ] The type signature of `--inspect-workers` and `--inspect-brokers` CLI arguments should be updated as well, to comply with [the original `--inspect`](https://nodejs.org/en/docs/guides/debugging-getting-started/)
```conf
--inspect-workers
--inspect-workers=port
--inspect-workers=host:port

--inspect-brokers
--inspect-brokers=port
--inspect-brokers=host:port

[ ] The docs should be updated
[ ] An example .vscode/launch.json configuration should be added to the SocketCluster sample app
[ ] The new CLI arguments should fix socketcluster#510

seiyria commented 4 years ago

If I were to start a PR for this, is this something that would be accepted and merged fairly quickly?

jondubois commented 4 years ago

@seiyria The latest SC version v16 does not have this problem because it no longer spawns multiple processes (it focuses on horizontal scalability across multiple hosts instead) so we can now debug each process individually.

That said if you want to make this change and maintain it yourself in a forked repo, then we could link to this repo from the website somewhere. I just don't have the bandwidth to maintain 2 different versions of SC anymore.

seiyria commented 4 years ago

@jondubois thanks for replying. If I were to fork it, it would really only be for this particular feature since I don't have the technical knowhow to really maintain SC, but if I do, I'll be sure to post back here. Thanks!

SocketCluster / socketcluster