Ensure Snapcast client has a valid current group before accessing group attributes. (#164683)

homeassistant/components/snapcast/media_player.py

@@ -131,7 +131,7 @@ class SnapcastClientDevice(SnapcastCoordinatorEntity, MediaPlayerEntity):
         return f"{CLIENT_PREFIX}{host}_{id}"
 
     @property
-    def _current_group(self) -> Snapgroup:
+    def _current_group(self) -> Snapgroup | None:
         """Return the group the client is associated with."""
         return self._device.group
 
@@ -158,12 +158,17 @@ class SnapcastClientDevice(SnapcastCoordinatorEntity, MediaPlayerEntity):
     def state(self) -> MediaPlayerState | None:
         """Return the state of the player."""
         if self._device.connected:
-            if self.is_volume_muted or self._current_group.muted:
+            if (
+                self.is_volume_muted
+                or self._current_group is None
+                or self._current_group.muted
+            ):
                 return MediaPlayerState.IDLE
             try:
                 return STREAM_STATUS.get(self._current_group.stream_status)
             except KeyError:
                 pass
+
         return MediaPlayerState.OFF
 
     @property
@@ -182,15 +187,31 @@ class SnapcastClientDevice(SnapcastCoordinatorEntity, MediaPlayerEntity):
     @property
     def source(self) -> str | None:
         """Return the current input source."""
+        if self._current_group is None:
+            return None
+
         return self._current_group.stream
 
     @property
     def source_list(self) -> list[str]:
         """List of available input sources."""
+        if self._current_group is None:
+            return []
+
         return list(self._current_group.streams_by_name().keys())
 
     async def async_select_source(self, source: str) -> None:
         """Set input source."""
+        if self._current_group is None:
+            raise ServiceValidationError(
+                translation_domain=DOMAIN,
+                translation_key="select_source_no_group",
+                translation_placeholders={
+                    "entity_id": self.entity_id,
+                    "source": source,
+                },
+            )
+
         streams = self._current_group.streams_by_name()
         if source in streams:
             await self._current_group.set_stream(streams[source].identifier)
@@ -233,6 +254,9 @@ class SnapcastClientDevice(SnapcastCoordinatorEntity, MediaPlayerEntity):
     @property
     def group_members(self) -> list[str] | None:
         """List of player entities which are currently grouped together for synchronous playback."""
+        if self._current_group is None:
+            return None
+
         entity_registry = er.async_get(self.hass)
         return [
             entity_id
@@ -248,6 +272,15 @@ class SnapcastClientDevice(SnapcastCoordinatorEntity, MediaPlayerEntity):
 
     async def async_join_players(self, group_members: list[str]) -> None:
         """Add `group_members` to this client's current group."""
+        if self._current_group is None:
+            raise ServiceValidationError(
+                translation_domain=DOMAIN,
+                translation_key="join_players_no_group",
+                translation_placeholders={
+                    "entity_id": self.entity_id,
+                },
+            )
+
         # Get the client entity for each group member excluding self
         entity_registry = er.async_get(self.hass)
         clients = [
@@ -271,13 +304,25 @@ class SnapcastClientDevice(SnapcastCoordinatorEntity, MediaPlayerEntity):
         self.async_write_ha_state()
 
     async def async_unjoin_player(self) -> None:
-        """Remove this client from it's current group."""
+        """Remove this client from its current group."""
+        if self._current_group is None:
+            raise ServiceValidationError(
+                translation_domain=DOMAIN,
+                translation_key="unjoin_no_group",
+                translation_placeholders={
+                    "entity_id": self.entity_id,
+                },
+            )
+
         await self._current_group.remove_client(self._device.identifier)
         self.async_write_ha_state()
 
     @property
     def metadata(self) -> Mapping[str, Any]:
         """Get metadata from the current stream."""
+        if self._current_group is None:
+            return {}
+
         try:
             if metadata := self.coordinator.server.stream(
                 self._current_group.stream
@@ -341,6 +386,9 @@ class SnapcastClientDevice(SnapcastCoordinatorEntity, MediaPlayerEntity):
     @property
     def media_position(self) -> int | None:
         """Position of current playing media in seconds."""
+        if self._current_group is None:
+            return None
+
         try:
             # Position is part of properties object, not metadata object
             if properties := self.coordinator.server.stream(

homeassistant/components/snapcast/strings.json

@@ -21,6 +21,17 @@
       }
     }
   },
+  "exceptions": {
+    "join_players_no_group": {
+      "message": "Client {entity_id} has no group. Unable to join players."
+    },
+    "select_source_no_group": {
+      "message": "Client {entity_id} has no group. Unable to select source {source}."
+    },
+    "unjoin_no_group": {
+      "message": "Client {entity_id} has no group. Unable to unjoin player."
+    }
+  },
   "services": {
     "restore": {
       "description": "Restores a previously taken snapshot of a media player.",

tests/components/snapcast/test_media_player.py

@@ -7,9 +7,12 @@ from syrupy.assertion import SnapshotAssertion
 
 from homeassistant.components.media_player import (
     ATTR_GROUP_MEMBERS,
+    ATTR_INPUT_SOURCE,
     DOMAIN as MEDIA_PLAYER_DOMAIN,
     SERVICE_JOIN,
+    SERVICE_SELECT_SOURCE,
     SERVICE_UNJOIN,
+    MediaPlayerState,
 )
 from homeassistant.config_entries import ConfigEntryState
 from homeassistant.const import ATTR_ENTITY_ID
@@ -175,3 +178,116 @@ async def test_state_stream_not_found(
 
     state = hass.states.get("media_player.test_client_1_snapcast_client")
     assert state.state == "off"
+
+
+async def test_attributes_group_is_none(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    mock_create_server: AsyncMock,
+    mock_client_1: AsyncMock,
+) -> None:
+    """Test exceptions are not thrown when a client has no group."""
+    # Force nonexistent group
+    mock_client_1.group = None
+
+    # Setup and verify the integration is loaded
+    with patch("secrets.token_hex", return_value="mock_token"):
+        await setup_integration(hass, mock_config_entry)
+        assert mock_config_entry.state is ConfigEntryState.LOADED
+
+    state = hass.states.get("media_player.test_client_1_snapcast_client")
+
+    # Assert accessing state and attributes doesn't throw
+    assert state.state == MediaPlayerState.IDLE
+
+    assert state.attributes["group_members"] is None
+    assert "source" not in state.attributes
+    assert "source_list" not in state.attributes
+    assert "metadata" not in state.attributes
+    assert "media_position" not in state.attributes
+
+
+async def test_select_source_group_is_none(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    mock_create_server: AsyncMock,
+    mock_client_1: AsyncMock,
+    mock_group_1: AsyncMock,
+) -> None:
+    """Test the select source action throws a service validation error when a client has no group."""
+    # Force nonexistent group
+    mock_client_1.group = None
+
+    # Setup and verify the integration is loaded
+    with patch("secrets.token_hex", return_value="mock_token"):
+        await setup_integration(hass, mock_config_entry)
+        assert mock_config_entry.state is ConfigEntryState.LOADED
+
+    with pytest.raises(ServiceValidationError):
+        await hass.services.async_call(
+            MEDIA_PLAYER_DOMAIN,
+            SERVICE_SELECT_SOURCE,
+            {
+                ATTR_ENTITY_ID: "media_player.test_client_1_snapcast_client",
+                ATTR_INPUT_SOURCE: "fake_source",
+            },
+            blocking=True,
+        )
+    mock_group_1.set_stream.assert_not_awaited()
+
+
+async def test_join_group_is_none(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    mock_create_server: AsyncMock,
+    mock_group_1: AsyncMock,
+    mock_client_1: AsyncMock,
+) -> None:
+    """Test join action throws a service validation error when a client has no group."""
+    # Force nonexistent group
+    mock_client_1.group = None
+
+    # Setup and verify the integration is loaded
+    with patch("secrets.token_hex", return_value="mock_token"):
+        await setup_integration(hass, mock_config_entry)
+        assert mock_config_entry.state is ConfigEntryState.LOADED
+
+    with pytest.raises(ServiceValidationError):
+        await hass.services.async_call(
+            MEDIA_PLAYER_DOMAIN,
+            SERVICE_JOIN,
+            {
+                ATTR_ENTITY_ID: "media_player.test_client_1_snapcast_client",
+                ATTR_GROUP_MEMBERS: ["media_player.test_client_2_snapcast_client"],
+            },
+            blocking=True,
+        )
+    mock_group_1.add_client.assert_not_awaited()
+
+
+async def test_unjoin_group_is_none(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    mock_create_server: AsyncMock,
+    mock_client_1: AsyncMock,
+    mock_group_1: AsyncMock,
+) -> None:
+    """Test the unjoin action throws a service validation error when a client has no group."""
+    # Force nonexistent group
+    mock_client_1.group = None
+
+    # Setup and verify the integration is loaded
+    with patch("secrets.token_hex", return_value="mock_token"):
+        await setup_integration(hass, mock_config_entry)
+        assert mock_config_entry.state is ConfigEntryState.LOADED
+
+    with pytest.raises(ServiceValidationError):
+        await hass.services.async_call(
+            MEDIA_PLAYER_DOMAIN,
+            SERVICE_UNJOIN,
+            {
+                ATTR_ENTITY_ID: "media_player.test_client_1_snapcast_client",
+            },
+            blocking=True,
+        )
+    mock_group_1.remove_client.assert_not_awaited()