search

wankdanker / node-discover

Automatic and decentralized discovery and monitoring of nodejs instances with built in support for a variable number of master processes, service advertising and channel messaging.
229 stars 59 forks source link

Using Discover inside cluster causes unhandled exception on windows #18

Closed cwi-vvatsos closed 4 years ago

cwi-vvatsos commented 8 years ago

According to NodeJS archives , sharing a UDP port on windows causes an ENOTSUP unhandled exception. This ofcourse happens with Discover.

The solution offered is to mark UDP ports as exclusive. This does not solve the issue for someone wanting to have multiple cluster childs in the some discovery cluster, but it does for those using different childs for different Discovery clusters on different ports.

I managed to fix this on my development by changing self.socket.bind(self.port, self.address, function () { to self.socket.bind({port:self.port, address:self.address,exclusive:true}, function () { on Network.prototype.start in network.js

I think this can be also changed to self.socket.bind({port:self.port, address:self.address,exclusive:!self.reuseAddr}, function () { so that no extra option is created for this issue.

If some other solution exists for Windows, please forgive my ignorance. I am quite the beginner in nodejs-cluster development

Goran-n commented 4 years ago

@wankdanker Why is this not a part of the master

wankdanker commented 4 years ago

@Goran-n There was no pull request. Not, that I'm good at merging those in a timely mannger.

Anyway, I just modified master if you want to try it out.

Goran-n commented 4 years ago

@wankdanker I created this fix in a fork.. Also, sorry for my premature comment, after doing some testing the above code suggested actually conflcits with other reuseAddr uses within network.js.

What i did was add a new variable called "exclusive". I've created a pull request if you want to merge it. If you think both can coexist then probably dont merge the pr. https://github.com/wankdanker/node-discover/pull/42

wankdanker commented 4 years ago

@Goran-n I like the granular approach. I think having it as a separate option is good and avoids conflicting with reuseAddr. I'll merge that. Thanks!

wankdanker commented 4 years ago

node-discover@1.1.3 is on npm. Can you confirm that this fixes this bug on Windows?

Goran-n commented 4 years ago

@wankdanker working on my end. I think you can close this. Thanks

wankdanker commented 4 years ago

@Goran-n Awesome. Thanks for helping to push this along.