fix KeyError when stream_status is called on a group

[Links2004]

This pull request improves error handling in the stream_status property of the Snapgroup class and adds a corresponding unit test to ensure robustness when a requested stream is missing from the server.

Error handling improvements:

• Modified the stream_status property in snapcast/control/group.py to catch KeyError exceptions and return "unknown" if the stream ID is not found, preventing unhandled exceptions.

"unknown" matches a valid state of the snapcast server API:
https://github.com/snapcast/snapcast/blob/801c02eaa432e6715fd69a2b5d857d276c0e8c39/server/streamreader/properties.hpp#L62

Testing enhancements:

• Added a new test case test_bad_stream_status in tests/test_group.py to verify that stream_status returns "unknown" when the stream ID does not exist on the server, ensuring the new error handling logic works as expected.

Related to:
home-assistant/core#157636

[luar123]

Adding this check makes sense to me, but we should also fix the root cause.

We know that adding/removing streams do not trigger an update notification (which should be fixed on the server side), see #65
This is why we call synchronize whenever there is an update for an unknown stream

python-snapcast/snapcast/control/server.py

Line 449 in 566c977

asyncio.ensure_future(async_sync())

Same needs to be done if we receive an _on _group_stream_changed notification with an unknown stream.

And I think in both cases it would be good to call the on update callback.

[Links2004]

I am not 100% sure how the async is implemented in this lib.
my idea is the following, feel free to comment.

diff --git a/snapcast/control/server.py b/snapcast/control/server.py
index 6a0e570..62b2fd9 100644
--- a/snapcast/control/server.py
+++ b/snapcast/control/server.py
@@ -272,7 +271,12 @@ class Snapserver():
 
     def stream(self, stream_identifier):
         """Get a stream."""
-        return self._streams[stream_identifier]
+        try:
+            return self._streams[stream_identifier]
+        except KeyError:
+            _LOGGER.info('stream %s not found, synchronize', stream_identifier)
+            asyncio.ensure_future(self.sync_sync())
+            raise KeyError
 
     def client(self, client_identifier):
         """Get a client."""
@@ -443,10 +447,10 @@ class Snapserver():
                 _LOGGER.debug('stream %s is input-only, ignore', data.get('id'))
             else:
                 _LOGGER.info('stream %s not found, synchronize', data.get('id'))
+                asyncio.ensure_future(self.sync_sync())
 
-                async def async_sync():
-                    self.synchronize((await self.status())[0])
-                asyncio.ensure_future(async_sync())
+    async def async_sync(self):
+        self.synchronize((await self.status())[0])
 
     def set_on_update_callback(self, func):
         """Set on update callback function.""
@@ -373,6 +377,10 @@ class Snapserver():
     def _on_group_stream_changed(self, data):
         """Handle group stream change."""
         group = self._groups.get(data.get('id'))
+        stream_id = data.get('stream_id')
+        if stream_id not in self._streams:
+            _LOGGER.info('stream %s not found, synchronize', stream_id)
+            asyncio.ensure_future(self.sync_sync())
         group.update_stream(data)
         for client_id in group.clients:
             self._clients.get(client_id).callback()

[luar123]

A few comments

[luar123]

Not sure if we should add it to stream() as well or let the calling application handle it. Behavior is kind of strange as it raises a keyerror and then updates. @happyleavesaoc what do you think?

[luar123]

Not sure if this unified function makes sense. Here, if stream is missing you need to call _on_update_callback_func, group.update_stream and the client callback after the sync, but in on_stream_update you only need to call _on_update_callback_func.

[luar123]

Proceed only if stream exists.

[Links2004]

did updated the code and added a callback handler to _synchronize_if_stream_missing.