Improve ipv6 detection

[remcohaszing]

Previously, ipv6 detection relied on the IP address. It didn’t work for domain names. The new implementation uses socket.getaddrinfo() to determine the address family.

@FrogTheFrog This is based on your suggestion. Would you like to review these changes?

Notably, I decided not to loop over all address families found. I just picked the first one.

Closes #34

[FrogTheFrog]

I agree with your approach to not iterate over all of the addresses.

I still do it in my project, but it has some issues, like some "the interface is unavailable" exceptions on some interfaces and that I have to handle 101 explicitly:

 except OSError as err:
    acceptable_errors = [101]
    if err.errno in acceptable_errors:
        logger.warning(f"WOL failed: {str(err)}")
    else:
        raise

Handling if properly would require some extra functionality or a different approach even. Maybe instead of allowing address_family is None, add a new function to return all addresses that people can iterate over, call send_magic_packet and catch exceptions.

[FrogTheFrog]

What is the benefit of assigning some fallback instead of raising an exception here?

If the address_family cannot be resolved by getaddrinfo, it is very likely that we would also fail down below. At best it would raise something, but it could just silently fail (which is worse IMO). However, if we raise an exception here, then the user could at least somehow handle it.

[remcohaszing]

Honestly, this was just to satisfy the type checker. Maybe raising an error is better indeed.

[FrogTheFrog]

Review done 👍

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (892f722) to head (b2453c7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main       #40      +/-   ##
===========================================
+ Coverage   96.15%   100.00%   +3.84%     
===========================================
  Files           1         1              
  Lines          52        46       -6     
  Branches       11         9       -2     
===========================================
- Hits           50        46       -4     
+ Misses          2         0       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[remcohaszing]

I pushed a new approach. This iterates over all the addr infos. If there aren’t any, it raises an exception. If none of the addresses succeeds, it raises the first exception. Is this better?

from https://man7.org/linux/man-pages/man3/getaddrinfo.3.html:

Normally, the application should try using the addresses in the order in which they are returned.

It feels a bit weird to me that getaddrinfo is called without the interface.

[FrogTheFrog]

Another round done

[FrogTheFrog]

No longer used

[remcohaszing]

Good catch. I considered leaving it out, but I decided to re-add support for specifying the address family. It’s not needed for typical use cases anymore though.

[FrogTheFrog]

What does proto=socket.IPPROTO_UDP do in this use-case?

[remcohaszing]

It’s pointless. Nice catch!

[FrogTheFrog]

Since you're iterating over these already I would recommend to introduce a "error_callback = None" parameter for this function and just try to create a socket for each address, because:

1. What if the last address is the one that will actually do WOL? We would be breaking on first.
2. What if first raises an exception, but the 2nd succeeds for real - wakes a PC. You would be raising an exception, but the function has worked as expected.

Maybe something like this? No ambiguity for error handling - if the user is unhappy they can use callback.

    for family, type, proto, canonname, addr in address_infos:
        try:
            with socket.socket(family, type, proto) as sock:
                if interface is not None:
                    sock.bind((interface, 0))
                sock.setsockopt(socket.SOL_SOCKET, socket.SO_BROADCAST, 1)
                sock.connect(addr)
                for packet in packets:
                    sock.send(packet)
        except Exception as e:
            if error_callback:
                if error_callback(e):
                    break
           else:
                raise error

[remcohaszing]

I don’t really want to bother the user with exception handling for this. Normally the library handles this. For example, have you ever dealt with resolving a hostname to an IP address when making an HTTP request? I don’t think I ever have.