search

achingbrain / ssdp

Simple Service Discovery Protocol implemented for Node.js
Other
51 stars 19 forks source link

byebye message not sent when calling bus.stop() #5

Open davidatingenuity opened 5 years ago

davidatingenuity commented 5 years ago

HI @achingbrain, I'm using your library, and integrating it with express, as per your example: const upnp = require('@achingbrain/ssdp')(options.ssdpOptions); const ssdp = require('./ssdp/ssdp'); upnp.on('error', console.error) // Log errors .advertise(ssdp.services) // Advertise the services configured .then(ssdp.serviceHosting) // Configure Express to serve the details.xml file .then(addRoutes); // then add all the other routes

where:

To get to this point, I needed to modify stop-advert.js slightly. To integrate with express, I need to specify a location, but if the location is specified, no shutdownServers function is created in create-location.js, and an unhandled promise rejection is thrown in stop-advert.js when it tries to call plumbing.shutdownServers. All I'm doing is checking if shutdownServers exists before it's called.

The issue I'm having is when I call upnp.stop(), a transport:outgoing-message is emitted for the byebye, but I never see the corresponding packet in wireshark. The socket.send call never seems to return.

I can submit a PR for the unhandled promise rejection, but I'm stuck on the other issue. Any ideas?

achingbrain commented 5 years ago

Please do submit a PR, that sounds like it should be handled - thanks.

The actual sending of the UDP message will likely be done asynchronously - Is the process shutting down before the packet gets sent?

davidatingenuity commented 5 years ago

I've submitted #6, but upon further inspection, it looks like the 2nd issue is a result of my fix for the first - the nested promises are being processed in parallel, not sequentially as I expected. This means the sockets are closed before the broadcastAdvert has finished.

davidatingenuity commented 5 years ago

I couldn't work out how to wait until the byebye broadcast event had finished, so I've put a 1 second delay between the broadcastAdvert and when the sockets are closed. It's not pretty, but it works well enough for our purposes. I'll let someone more capable than I fix it properly, but if you're interested, my fix is here: https://github.com/davidatingenuity/ssdp/commit/7fc64752ec711ee7965395089f5d4a47be728979