If local endpoint changes, subscribe will keep failing to reconnect.
pfiaux opened this issue · 6 comments
If the local end point changes, new ip is given by the router our such the subscribe call will throw a EADDRNOTAVAIL error. But since discovery.localEndpoint is only setup on init it will keep failing indefinitely or until restarted.
I fixed this on a legacy branch: pfiaux@67201dd
I handled the EADDRNOTAVAIL specifically can updated the localEndpoint in that case. I looked quickly, I could be wrong but it seems like 1.0.0 on master still only does it on init as well:
https://github.com/jishi/node-sonos-discovery/blob/master/lib/SonosSystem.js#L123
I'm aware that the system doesn't really work if the system this is running on, or the Sonos players changes IP all of a sudden. It has however been low priority since that is not a common scenario.
Is there a specific setup that gives a higher probability that this might occur for you? The EADDRNOTAVAIL error will no longer occur the same way, since I'm no longer binding requests to a certain IP (I rely on the system routing table to make it's decision), so the only thing that happens is that the wrong URL is sent as subscription endpoint, this will fail silently.
So either a periodic update of local endpoint, or update it every time it sends an action to any player (since at that point, I know which local endpoint it used).
No specific setup, just trying to be foolproof and cover edge cases. The silent failure is a problem, the subscriber using discovery can then have "a dead player" without knowing it. Perhaps wrapping the error if unhandled and forwarding it via discovery.on('error', cb) would allow the subscriber to chose to do something with it.
In this case I'm interested in keeping IP change handling encapsulated in node-sonos-discovery. My solution above works for the local endpoint.
However like you said I run into a similar problem if the Sonos player changes IP. I'm looking into a way to try updating a player's address if the Sonos was to change IP. The error in this case would show up as ETIMEDOUT (in this case it could also be the sonos was turned off).
The sonos address is found in Discovery and it's passed to the Player as a constructor parameter. So either:
- Discovery could start looking on the dgram socket again and create a new Player
- Player could look (even via discovery) for it's specific device on a new IP and update its address when found
I'm not sure which one would be the best approach. It looks like we can use the id from /root/device/UDN to find the same device (assuming the IP changed and it wasn't turned off)
It doesn't have to be that complicated. The ZoneTopology would eventually change to reflect the changes, so a player that no longer occurs in the ZoneTopology, could be removed, and any new players that would show up, is already automatically added. Likewise, you could correlate the IP with the UUID and remove players that has a mismatch.
One edge case here is when the player/bridge/boost that it subscribes for topology changes dies or changes IP, it would need to reset the whole discovery process from scratch. But that should be fairly easy to fix, anyway.
Thanks for the feedback, that makes more sense indeed. If the use case is only one Sonos is used, it would be the topology subscribed player at all times.
I took sonos-ssdp.js into the legacy branch and I was actually able to get the uuid inside of ssdp's message handler since its included in the USN header. As you said it probably doesn't need to be that complicated. pfiaux@a0a6fa8 I tested it, the subscriber will be able to find the Player again if either IPs change.
Right now I'm only handling this passively for the event tho, soap actions wont trigger my 'rediscovery' process. I'm probably going to look at getting that working as well. If you're not interested in bringing the handling of IP changes into the 1.0.0 branch we can close this issue :)
I want to eventually support this scenario, so I'll keep this open. Feel free to comment if you have any observations worth mentioning that could be beneficial.
I think I have fixed this now. Haven't verified what happens during an IP change, but when the initial player that it gets topology notifications from seems to have gone away, it will reset everything and find a new one.