Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Guard browsing Spotify if setup failed #65074

Merged
merged 2 commits into from Jan 27, 2022
Merged

Guard browsing Spotify if setup failed #65074

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50455bf
Guard browsing Spotify if setup failed
jjlawren Jan 27, 2022
970e383
Apply suggestions from code review
jjlawren Jan 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion homeassistant/components/spotify/__init__.py
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@
from spotipy import Spotify, SpotifyException
import voluptuous as vol

from homeassistant.components.media_player import BrowseError
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import (
ATTR_CREDENTIALS,
Expand Down Expand Up @@ -60,7 +61,9 @@ async def async_browse_media(
hass, media_content_type, media_content_id, *, can_play_artist=True
):
"""Browse Spotify media."""
info = list(hass.data[DOMAIN].values())[0]
info = next(iter(hass.data[DOMAIN].values()), None)
if not info:
raise BrowseError("No Spotify accounts available")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we catch this inside Sonos?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes a lot of sense to do here, mainly because anything else can just use it without having to worry about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I meant catch BrowseError in Sonos. The check here is indeed correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's necessary. It should only hit this codepath if the browser is explicitly trying to browse into Spotify. It'll only hit this at the root level is Spotify is the only source available.

jjlawren marked this conversation as resolved.
Show resolved Hide resolved
return await async_browse_media_internal(
hass,
info[DATA_SPOTIFY_CLIENT],
Expand Down