From 49272fadffb78eeef0f4d80538490b901dddaa4e Mon Sep 17 00:00:00 2001 From: Links2004 Date: Sun, 14 Dec 2025 10:08:21 +0100 Subject: [PATCH 1/8] fix KeyError when stream_status is called on a group --- snapcast/control/group.py | 5 ++++- tests/test_group.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/snapcast/control/group.py b/snapcast/control/group.py index 98bbad1..f32e5d6 100644 --- a/snapcast/control/group.py +++ b/snapcast/control/group.py @@ -51,7 +51,10 @@ async def set_stream(self, stream_id): @property def stream_status(self): """Get stream status.""" - return self._server.stream(self.stream).status + try: + return self._server.stream(self.stream).status + except KeyError: + return "unknown" @property def muted(self): diff --git a/tests/test_group.py b/tests/test_group.py index bd99ec2..31b13dc 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -101,3 +101,35 @@ def test_set_callback(self): self.group.set_callback(cb) self.group.update_mute({'mute': True}) cb.assert_called_with(self.group) + + def test_bad_stream_status(self): + # Simulate a server where the requested stream id is missing + class DummyClient: + def __init__(self, identifier, friendly_name): + self.identifier = identifier + self.friendly_name = friendly_name + + class DummyServer: + def __init__(self): + self._streams = {} + # provide clients list used by Snapgroup.friendly_name + self.clients = [DummyClient('a', 'A'), DummyClient('b', 'B')] + + def stream(self, stream_identifier): + return self._streams[stream_identifier] + + def client(self, identifier): + # return a client-like object for friendly_name lookup + for c in self.clients: + if c.identifier == identifier: + return c + raise KeyError(identifier) + + # Replace the group's server with the dummy and set an unknown stream id + self.group._server = DummyServer() + + # Updating the stream should not raise; accessing stream_status should + # not raise KeyError because the stream id is not present on the server. + self.group.update_stream({'stream_id': 'no stream'}) + self.assertEqual(self.group.stream_status, 'unknown') + From 315c44a0f973748a43600341470af462e528bb8b Mon Sep 17 00:00:00 2001 From: Links2004 Date: Sun, 14 Dec 2025 12:59:54 +0100 Subject: [PATCH 2/8] force synchronize if stream is missing for stream and _on_group_stream_changed calls --- snapcast/control/server.py | 21 ++++++++++++++++----- tests/test_server.py | 21 ++++++++++++++++++++- tox.ini | 2 +- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/snapcast/control/server.py b/snapcast/control/server.py index 6a0e570..18ce94f 100644 --- a/snapcast/control/server.py +++ b/snapcast/control/server.py @@ -272,6 +272,9 @@ def group(self, group_identifier): def stream(self, stream_identifier): """Get a stream.""" + if stream_identifier not in self._streams: + self._synchronize_if_stream_missing(stream_identifier) + raise KeyError(f'Stream "{stream_identifier}" not found') return self._streams[stream_identifier] def client(self, client_identifier): @@ -373,6 +376,7 @@ def _on_group_name_changed(self, data): def _on_group_stream_changed(self, data): """Handle group stream change.""" group = self._groups.get(data.get('id')) + self._synchronize_if_stream_missing(data.get('stream_id', None)) group.update_stream(data) for client_id in group.clients: self._clients.get(client_id).callback() @@ -442,11 +446,18 @@ def _on_stream_update(self, data): if data.get('stream', {}).get('uri', {}).get('query', {}).get('codec') == 'null': _LOGGER.debug('stream %s is input-only, ignore', data.get('id')) else: - _LOGGER.info('stream %s not found, synchronize', data.get('id')) - - async def async_sync(): - self.synchronize((await self.status())[0]) - asyncio.ensure_future(async_sync()) + self._synchronize_if_stream_missing(data.get('id')) + + def _synchronize_if_stream_missing(self, stream_id): + """Ensure stream exists, otherwise synchronize.""" + if stream_id is None: + return + if stream_id not in self._streams: + _LOGGER.info('stream %s not found, synchronize', stream_id) + + async def async_sync(): + self.synchronize((await self.status())[0]) + asyncio.ensure_future(async_sync()) def set_on_update_callback(self, func): """Set on update callback function.""" diff --git a/tests/test_server.py b/tests/test_server.py index 01ac701..73b5298 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -282,13 +282,15 @@ def test_on_group_mute(self): self.server._on_group_mute(data) self.assertEqual(self.server.group('test').muted, True) - def test_on_group_stream_changed(self): + @mock.patch.object(Snapserver, '_synchronize_if_stream_missing') + def test_on_group_stream_changed(self, mock_sync): data = { 'id': 'test', 'stream_id': 'other' } self.server._on_group_stream_changed(data) self.assertEqual(self.server.group('test').stream, 'other') + mock_sync.assert_called_with('other') def test_on_client_connect(self): cb = mock.MagicMock() @@ -361,6 +363,23 @@ def test_on_stream_update(self): self.server._on_stream_update(data) self.assertEqual(self.server.stream('stream').status, 'idle') + @mock.patch.object(Snapserver, '_synchronize_if_stream_missing') + def test_on_stream_update_new(self, mock_sync): + data = { + 'id': 'stream_new', + 'stream': { + 'id': 'stream_new', + 'status': 'idle', + 'uri': { + 'query': { + 'name': 'stream_new' + } + } + } + } + self.server._on_stream_update(data) + mock_sync.assert_called_with('stream_new') + def test_on_meta_update(self): data = { 'id': 'stream', diff --git a/tox.ini b/tox.ini index 739e98d..e515fd7 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py310, py311, lint +envlist = py310, py311, py313, lint skip_missing_interpreters = True [tool:pytest] From 759e5d6f7727e93318170779f6de4545763ca57f Mon Sep 17 00:00:00 2001 From: Markus <974709+Links2004@users.noreply.github.com> Date: Sun, 14 Dec 2025 18:43:08 +0100 Subject: [PATCH 3/8] Update snapcast/control/server.py Co-authored-by: luar123 <49960470+luar123@users.noreply.github.com> --- snapcast/control/server.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/snapcast/control/server.py b/snapcast/control/server.py index 18ce94f..4e64111 100644 --- a/snapcast/control/server.py +++ b/snapcast/control/server.py @@ -457,6 +457,8 @@ def _synchronize_if_stream_missing(self, stream_id): async def async_sync(): self.synchronize((await self.status())[0]) + if self._on_update_callback_func and callable(self._on_update_callback_func): + self._on_update_callback_func() asyncio.ensure_future(async_sync()) def set_on_update_callback(self, func): From 223f7320cf46e4fbc4a604a53cdd2203e85cc6f9 Mon Sep 17 00:00:00 2001 From: Links2004 Date: Sun, 14 Dec 2025 19:36:17 +0100 Subject: [PATCH 4/8] better callback handling --- snapcast/control/server.py | 22 ++++++++++++++----- tests/test_server.py | 43 +++++++++++++++++++++++++++++++++----- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/snapcast/control/server.py b/snapcast/control/server.py index 4e64111..66ee4d2 100644 --- a/snapcast/control/server.py +++ b/snapcast/control/server.py @@ -376,7 +376,18 @@ def _on_group_name_changed(self, data): def _on_group_stream_changed(self, data): """Handle group stream change.""" group = self._groups.get(data.get('id')) - self._synchronize_if_stream_missing(data.get('stream_id', None)) + stream_id = data.get('stream_id', None) + + if stream_id not in self._streams: + def update_callback(): + self._on_update_callback_func() + group.update_stream(data) + for client_id in group.clients: + self._clients.get(client_id).callback() + + self._synchronize_if_stream_missing(stream_id, update_callback) + return + group.update_stream(data) for client_id in group.clients: self._clients.get(client_id).callback() @@ -446,9 +457,9 @@ def _on_stream_update(self, data): if data.get('stream', {}).get('uri', {}).get('query', {}).get('codec') == 'null': _LOGGER.debug('stream %s is input-only, ignore', data.get('id')) else: - self._synchronize_if_stream_missing(data.get('id')) + self._synchronize_if_stream_missing(data.get('id'), self._on_update_callback_func) - def _synchronize_if_stream_missing(self, stream_id): + def _synchronize_if_stream_missing(self, stream_id, callback=None): """Ensure stream exists, otherwise synchronize.""" if stream_id is None: return @@ -457,8 +468,9 @@ def _synchronize_if_stream_missing(self, stream_id): async def async_sync(): self.synchronize((await self.status())[0]) - if self._on_update_callback_func and callable(self._on_update_callback_func): - self._on_update_callback_func() + if callback and callable(callback): + callback() + asyncio.ensure_future(async_sync()) def set_on_update_callback(self, func): diff --git a/tests/test_server.py b/tests/test_server.py index 73b5298..65aff74 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -74,6 +74,21 @@ 'title': 'Happy!', } } + }, + { + 'id': 'stream2', + 'status': 'playing', + 'uri': { + 'query': { + 'name': 'stream2' + } + }, + 'properties': { + 'canControl': False, + 'metadata': { + 'title': 'Happy2!', + } + } } ] } @@ -168,7 +183,7 @@ def test_init(self): self.assertEqual(self.server.version, '0.26.0') self.assertEqual(len(self.server.clients), 1) self.assertEqual(len(self.server.groups), 1) - self.assertEqual(len(self.server.streams), 1) + self.assertEqual(len(self.server.streams), 2) self.assertEqual(self.server.group('test').identifier, 'test') self.assertEqual(self.server.stream('stream').identifier, 'stream') self.assertEqual(self.server.client('test').identifier, 'test') @@ -284,13 +299,27 @@ def test_on_group_mute(self): @mock.patch.object(Snapserver, '_synchronize_if_stream_missing') def test_on_group_stream_changed(self, mock_sync): + data = { + 'id': 'test', + 'stream_id': 'stream2' + } + self.server._on_group_stream_changed(data) + self.assertEqual(self.server.group('test').stream, 'stream2') + + mock_sync.assert_not_called() + + @mock.patch.object(Snapserver, '_synchronize_if_stream_missing') + def test_on_group_stream_changed_no_stream(self, mock_sync): data = { 'id': 'test', 'stream_id': 'other' } self.server._on_group_stream_changed(data) - self.assertEqual(self.server.group('test').stream, 'other') - mock_sync.assert_called_with('other') + self.assertEqual(self.server.group('test').stream, 'stream2') + + mock_sync.assert_called_once() + _, args, _ = mock_sync.mock_calls[0] + self.assertEqual('other', args[0]) def test_on_client_connect(self): cb = mock.MagicMock() @@ -347,7 +376,8 @@ def test_on_client_latency_changed(self): self.server._on_client_latency_changed(data) self.assertEqual(self.server.client('test').latency, 50) - def test_on_stream_update(self): + @mock.patch.object(Snapserver, '_synchronize_if_stream_missing') + def test_on_stream_update(self, mock_sync): data = { 'id': 'stream', 'stream': { @@ -362,6 +392,7 @@ def test_on_stream_update(self): } self.server._on_stream_update(data) self.assertEqual(self.server.stream('stream').status, 'idle') + mock_sync.assert_not_called() @mock.patch.object(Snapserver, '_synchronize_if_stream_missing') def test_on_stream_update_new(self, mock_sync): @@ -378,7 +409,9 @@ def test_on_stream_update_new(self, mock_sync): } } self.server._on_stream_update(data) - mock_sync.assert_called_with('stream_new') + mock_sync.assert_called_once() + _, args, _ = mock_sync.mock_calls[0] + self.assertEqual('stream_new', args[0]) def test_on_meta_update(self): data = { From 99558b9417ba6bd9ee95931356be65e2c3cd6fbb Mon Sep 17 00:00:00 2001 From: Links2004 Date: Sun, 14 Dec 2025 19:41:46 +0100 Subject: [PATCH 5/8] add found check --- snapcast/control/server.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/snapcast/control/server.py b/snapcast/control/server.py index 66ee4d2..0d8b711 100644 --- a/snapcast/control/server.py +++ b/snapcast/control/server.py @@ -379,8 +379,10 @@ def _on_group_stream_changed(self, data): stream_id = data.get('stream_id', None) if stream_id not in self._streams: - def update_callback(): + def update_callback(found): self._on_update_callback_func() + if not found: + return group.update_stream(data) for client_id in group.clients: self._clients.get(client_id).callback() @@ -464,12 +466,15 @@ def _synchronize_if_stream_missing(self, stream_id, callback=None): if stream_id is None: return if stream_id not in self._streams: - _LOGGER.info('stream %s not found, synchronize', stream_id) + _LOGGER.info('stream "%s" not found, synchronize', stream_id) async def async_sync(): self.synchronize((await self.status())[0]) + found = stream_id in self._streams + if not found: + _LOGGER.warning('stream "%s" still not found after synchronization', stream_id) if callback and callable(callback): - callback() + callback(found, stream_id) asyncio.ensure_future(async_sync()) From 11a4087106b9beb22233c75cd552afc5c2d3a12b Mon Sep 17 00:00:00 2001 From: Links2004 Date: Sun, 14 Dec 2025 19:47:56 +0100 Subject: [PATCH 6/8] do not force update in stream func --- snapcast/control/server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/snapcast/control/server.py b/snapcast/control/server.py index 0d8b711..75cbc61 100644 --- a/snapcast/control/server.py +++ b/snapcast/control/server.py @@ -273,7 +273,6 @@ def group(self, group_identifier): def stream(self, stream_identifier): """Get a stream.""" if stream_identifier not in self._streams: - self._synchronize_if_stream_missing(stream_identifier) raise KeyError(f'Stream "{stream_identifier}" not found') return self._streams[stream_identifier] From e694288a2d58264ddd3bd738fb3ff50bfafbaeba Mon Sep 17 00:00:00 2001 From: Links2004 Date: Sun, 21 Dec 2025 09:57:44 +0100 Subject: [PATCH 7/8] fix argument count --- snapcast/control/server.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/snapcast/control/server.py b/snapcast/control/server.py index 75cbc61..a732c7d 100644 --- a/snapcast/control/server.py +++ b/snapcast/control/server.py @@ -378,7 +378,7 @@ def _on_group_stream_changed(self, data): stream_id = data.get('stream_id', None) if stream_id not in self._streams: - def update_callback(found): + def update_callback(found, stream_id): self._on_update_callback_func() if not found: return @@ -458,7 +458,10 @@ def _on_stream_update(self, data): if data.get('stream', {}).get('uri', {}).get('query', {}).get('codec') == 'null': _LOGGER.debug('stream %s is input-only, ignore', data.get('id')) else: - self._synchronize_if_stream_missing(data.get('id'), self._on_update_callback_func) + def update_callback(found, stream_id): + if self._on_update_callback_func and callable(self._on_update_callback_func): + self._on_update_callback_func() + self._synchronize_if_stream_missing(data.get('id'), update_callback) def _synchronize_if_stream_missing(self, stream_id, callback=None): """Ensure stream exists, otherwise synchronize.""" From 69b77293d08bc80c12678326df725624102684ed Mon Sep 17 00:00:00 2001 From: Markus <974709+Links2004@users.noreply.github.com> Date: Tue, 23 Dec 2025 21:47:30 +0100 Subject: [PATCH 8/8] Update snapcast/control/server.py Co-authored-by: luar123 <49960470+luar123@users.noreply.github.com> --- snapcast/control/server.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/snapcast/control/server.py b/snapcast/control/server.py index a732c7d..d4ce27d 100644 --- a/snapcast/control/server.py +++ b/snapcast/control/server.py @@ -379,7 +379,8 @@ def _on_group_stream_changed(self, data): if stream_id not in self._streams: def update_callback(found, stream_id): - self._on_update_callback_func() + if self._on_update_callback_func and callable(self._on_update_callback_func): + self._on_update_callback_func() if not found: return group.update_stream(data)