use ipv6 if that is actually supported

[bobvanderlinden]

This makes sure that if an address in one of the two address families
is found, it won't raise an error due to it not being found in the
other family. This resolves issues on Raspberry Pi and inside docker
containers.

[agnat]

Thanks for the PR @bobvanderlinden.

Unfortunately it is not that easy. Line 151 in resolver_sequence_tasks.js states:

// XXX in older versions of node a zero family value is not supported

So your current patch will break things on older versions of node on linux. Unfortunately the comment does not mention a specific version (my bad). So, that would require a bit of software archeology...

Another thing is: This specific error only happens on Raspberry Pi (distros). I've seen a couple of similar work arounds but I never got a good explanation what is causing it. Could you take a minute to explain what is actually happening?

Raspberry Pi and node_mdns are such a nice fit. Still many people struggle to get it working. So I will accept a patch that works around what ever the issue is. However, I'd like the patch to be rather defensive (probe behavior carefully and only modify things if necessary).

Let me know what you think.

[agnat]

I'm a bit cautious here because the address resolution is the most brittle part of node_mdns... by far. It is very easy to break things in subtle ways... :-/

That's mostly because the avahi compat library does not cope... You've seen the warnings.

[bobvanderlinden]

Indeed. Another way to solve this problem is to resolve any of the address-families, instead of all of the address-families. That way older versions of node should still work.
Do you think this is a viable method?

[agnat]

The thing is: The linux implementation that works around missing functionality in the compat lib, tries hard to mimic the behavior of the original Bonjour SDK. I'm sure you'll agree that that is a desirable property. So breaking that by globally modifying behavior on linux is kind of like a last resort. At the very least I want to understand what the problem is before going down that road.

In any case I would be much more comfortable with a solution that probes the behavior of whatever it is that causes the actual error at startup and then carefully modifies a default.

Even if you don't know what is actually causing it, maybe you could... like ... walk me through it? Maybe paste a stack and write down what you learned while debugging?

[bobvanderlinden]

We have 2 families: 4 and 6.

getaddrinfo is used with both families [4, 6].
This results in multiple calls to _getaddrinfo: https://github.com/bobvanderlinden/node_mdns/blob/fix-addressfamily/lib/resolver_sequence_tasks.js#L153
When one of the calls results in an error, last_error is set: https://github.com/bobvanderlinden/node_mdns/blob/fix-addressfamily/lib/resolver_sequence_tasks.js#L155
When the other call results in no errors, it sets the addresses: https://github.com/bobvanderlinden/node_mdns/blob/fix-addressfamily/lib/resolver_sequence_tasks.js#L157

Even though one of the families had succeeded, getaddrinfo will result in an error because the other family resulted in an error: https://github.com/bobvanderlinden/node_mdns/blob/fix-addressfamily/lib/resolver_sequence_tasks.js#L160

Making sure always one family is used in getaddrinfo fixes the problem (as shown with [0]). However, like you said, this is not viable, because of compatibility reasons.

I think it can be changed somehow to have it ignore errors of other families if one family succeeds. I just don't know if this is the right approach:
should errors be propagated when any of the calls failed; or should errors be ignored when any of the calls succeeded?

[agnat]

Right. Thanks for the explanation. It seems the root cause of this is lack of ipv6 support on these systems. Looks like on Raspbian ipv6 is disabled by default.

So let's sketch out a plan:

1. Find out what the XXX above is about. If it only affects historic versions of node we can go ahead and merge this.
2. After finding the exact version where the zero family was introduced we should put it in code: node_has_zero_family() ? [0] : [4,6]. This will help us to phase out the whole thing when time comes.
3. Now, if we feel the second case is still an issue because lots of people still use a node version on raspberry pi that does not support the zero family we should probe for ipv6 support by testing if /proc/net/if_inet6 exists.

This feels like the right thing to test for and also side-steps the issue of changing the error behavior. ;)

It's obviously more work than just changing the default. So, how about it? Feel like putting in the work? Just let me know what you think...

[bobvanderlinden]

Sounds doable. However, why not just propagate errors when all resolve
attempts have failed? And return the combined addresses for the successful
attempts. It avoids version checking and potentially errorous
ipv6-support-probing (should we probe for ipv6 on Windows and Mac OSX as
well?). It seems simpler, but I do not know what the exact implications are
of ignoring the errors.

On Fri, Dec 25, 2015 at 5:26 PM David Siegel [email protected]
wrote:

Right. Thanks for the explanation. It seems the root cause of this is lack
of ipv6 support on these systems. Looks like on Raspbian ipv6 is disabled
by default
https://www.raspbian.org/RaspbianFAQ#How_do_I_enable_or_use_IPv6.3F.

So let's sketch out a plan:

1. Find out what the XXX above is about. If it only affects historic
   versions of node we can go ahead and merge this.
2. After finding the exact version where the zero family was
   introduced we should put it in code: node_has_zero_family() ? [0] :
   [4,6]. This will help us to phase out the whole thing when time comes.
3. Now, if we feel the second case is still an issue because lots of
   people still use a node version on raspberry pi that does not support the
   zero family we should probe for ipv6 support
   http://www.cyberciti.biz/faq/check-for-ipv6-support-in-linux-kernel/
   by testing if /proc/net/if_inet6 exists.

This feels like the right thing to test for and also side-steps the issue
of changing the error behavior. ;)

It's obviously more work than just changing the default. So, how about it?
Feel like putting in the work? Just let me know what you think...

—
Reply to this email directly or view it on GitHub
#149 (comment).

[agnat]

but I do not know what the exact implications are of ignoring the errors

Hehe... me neither. Call me paranoid but I'd rather not ignore them.

should we probe for ipv6 on Windows and Mac OS X as well?

Nope. This only affects linux (ahavi).

[prawnsalad]

Was there any progress on this? The only way I can use this lib is with this PR in place.

[bobvanderlinden]

Forgot about this issue, since I'm not using node_mdns on docker nor the raspberry pi anymore. However, I thought I'd give this a try anyway.

@agnat Agreed. The code of resolver_sequence_tasks does get more complex, but at least we're not ignoring errors. Apparently nodejs v0.11.5 introduced support for family 0: https://github.com/nodejs/node/blob/v0.11.5/src/cares_wrap.cc#L893-L895

The version comparing could be reused as well for (https://github.com/bobvanderlinden/node_mdns/blob/776ea07d3b8aacc518928434343aef09567128bf/lib/resolver_sequence_tasks.js#L185), but I thought to keep the PR minimal. Maybe using a library for version comparison is a better option, let me know.

@prawnsalad Could you give this PR a try again? I don't have the right setup at the moment to test whether this will actually work. On my system all tests do pass.