agnat / node_mdns

mdns/zeroconf/bonjour service discovery add-on for node.js
http://agnat.github.com/node_mdns
MIT License
871 stars 146 forks source link

Discovery crashes when running in Docker #244

Open ErwinSteffens opened 4 years ago

ErwinSteffens commented 4 years ago

We are running into an issue when using the library in docker.

Our node process (using the library) in running in a docker container. The avahi daemon is running on the host OS. We are mounting the dbus system bus socket into the docker container so we can connect to the avahi daemon.

The issue is that when an item is discovered it tries to resolve the interface name in the library, but the interfaces in the container are different then on the host OS. The library crashes on this:

uncaughtException: index has no corresponding interface
Error: index has no corresponding interface
    at Browser.on_service_changed (/app/node_modules/mdns/lib/browser.js:61:45)
    at SocketWatcher.MDNSService.self.watcher.callback (/app/node_modules/mdns/lib/mdns_service.js:18:40) Error: index has no corresponding interface Error: index has no corresponding interface
    at Browser.on_service_changed (/app/node_modules/mdns/lib/browser.js:61:45)
    at SocketWatcher.MDNSService.self.watcher.callback (/app/node_modules/mdns/lib/mdns_service.js:18:40) {"error":{},"stack":"Error: index has no corresponding interface\n    at Browser.on_service_changed (/app/node_modules/mdns/lib/browser.js:61:45)\n    at SocketWatcher.MDNSService.self.watcher.callback (/app/node_modules/mdns/lib/mdns_service.js:18:40)","exception":true,"date":"Mon Nov 16 2020 13:11:00 GMT+0000 (Coordinated Universal Time)","process":{"pid":6,"uid":0,"gid":0,"cwd":"/app","execPath":"/usr/local/bin/node","version":"v14.13.1","argv":["/usr/local/bin/node","/app/build/index"],"memoryUsage":{"rss":99598336,"heapTotal":58724352,"heapUsed":40492784,"external":1774653,"arrayBuffers":216155}},"os":{"loadavg":[0.72,0.24,0.12],"uptime":12932},"trace":[{"column":45,"file":"/app/node_modules/mdns/lib/browser.js","function":"Browser.on_service_changed","line":61,"method":"on_service_changed","native":false},{"column":40,"file":"/app/node_modules/mdns/lib/mdns_service.js","function":"SocketWatcher.MDNSService.self.watcher.callback","line":18,"method":"callback","native":false}]}

We would like to make the name resolution of the interface optional. We see there is already a NODE_MDNS_HAVE_INTERFACE_NAME_CONVERSION define for making this configurable based on OS detection.

We would like to suggest to make this configurable by variable.

agnat commented 4 years ago

Hi @ErwinSteffens, making that optional sounds good to me. However, instead of making NODE_MDNS_HAVE_INTERFACE_NAME_CONVERSION a compile-time variable, I'd just use a runtime JS option:

const browser = new mdns.Browser(type, {lookupInterfaceNames: false});

Or is that what you mean?

On a side note, I believe it only terminates your application because you don't have an error handler on the browser. You do want an error handler...

ErwinSteffens commented 4 years ago

That's not what I meant, but I think it is a better solution. I have made an PR: https://github.com/agnat/node_mdns/pull/245

I agree about the error handler ;-)