fix: multiple Advertisements without network#13
Open
tkurki wants to merge 2 commits intoDeMille:masterfrom
Open
fix: multiple Advertisements without network#13tkurki wants to merge 2 commits intoDeMille:masterfrom
tkurki wants to merge 2 commits intoDeMille:masterfrom
Conversation
Add a simple example that fails when run without a network, for example with Docker. See the file for an example on how to execute it.
tkurki
added a commit
to SignalK/signalk-server
that referenced
this pull request
Sep 1, 2019
Handle errors from dsnsd Advertisements and use a version of dnssd that does not throw out of band errors. Once or if DeMille/dnssd.js#13 or something equivalent makes it to npm we should go back to the upstream version of dnssd.
tkurki
added a commit
to SignalK/signalk-server
that referenced
this pull request
Sep 1, 2019
Handle errors from dsnsd Advertisements and use a version of dnssd that does not throw out of band errors. Once or if DeMille/dnssd.js#13 or something equivalent makes it to npm we should go back to the upstream version of dnssd.
tkurki
added a commit
to SignalK/signalk-server
that referenced
this pull request
Sep 8, 2019
Handle errors from dsnsd Advertisements and use a version of dnssd that does not throw out of band errors. Once or if DeMille/dnssd.js#13 or something equivalent makes it to npm we should go back to the upstream version of dnssd.
Ignore errors from NetworkInterface, when there are no listeners. Without this multiple Advertisements on a host with no network fails when NetworkInterface emits errors after the first emitted error has already stopped the interface, but there are pending errors from the socket.
44b1347 to
819538b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Dnssd fails with
if you create multiple Advertisements on a host with no network available. As far I can tell the reason for this is that the Advertisements share one NetworkInterface, that is shut down when the first Advertisement fails, but there are outstanding that nobody is listening to.
Just adding error handlers to Advertisements do not catch the secondary errors, so somebody using dnssd has no way to guard against this condition.
The PR includes a simple way to demonstrate this with Docker.
I am not familiar enough with dnssd if the fix I included is the way to go. If you have a better way forward please let me know.
My motivation for using dnssd is to be able to work without mdns's native dependencies and possible separate Avahi installation. The original problem that I am solving is that this fails on setups with no internet connection, eg. no gateway and 224.0.0.251:5353 throws ENETUNREACH - the docker incantation is just an easier way to reproduce the issue.