images: ignore bad image uris (Fixes #364)

mopidy_spotify/images.py

@@ -15,7 +15,8 @@
 def get_images(web_client, uris):
     result = {}
     uri_type_getter = operator.itemgetter("type")
-    uris = sorted((_parse_uri(u) for u in uris), key=uri_type_getter)
+    uris = (_parse_uri(u) for u in uris)
+    uris = sorted((u for u in uris if u), key=uri_type_getter)
     for uri_type, group in itertools.groupby(uris, uri_type_getter):
         batch = []
         for uri in group:
@@ -33,25 +34,27 @@ def get_images(web_client, uris):
 
 
 def _parse_uri(uri):
-    parsed_uri = urllib.parse.urlparse(uri)
-    uri_type, uri_id = None, None
-
-    if parsed_uri.scheme == "spotify":
-        uri_type, uri_id = parsed_uri.path.split(":")[:2]
-    elif parsed_uri.scheme in ("http", "https"):
-        if parsed_uri.netloc in ("open.spotify.com", "play.spotify.com"):
-            uri_type, uri_id = parsed_uri.path.split("/")[1:3]
-
-    supported_types = ("track", "album", "artist", "playlist")
-    if uri_type and uri_type in supported_types and uri_id:
-        return {
-            "uri": uri,
-            "type": uri_type,
-            "id": uri_id,
-            "key": (uri_type, uri_id),
-        }
-
-    raise ValueError(f"Could not parse {repr(uri)} as a Spotify URI")
+    try:
+        parsed_uri = urllib.parse.urlparse(uri)
+        uri_type, uri_id = None, None
+
+        if parsed_uri.scheme == "spotify":
+            uri_type, uri_id = parsed_uri.path.split(":")[:2]
+        elif parsed_uri.scheme in ("http", "https"):
+            if parsed_uri.netloc in ("open.spotify.com", "play.spotify.com"):
+                uri_type, uri_id = parsed_uri.path.split("/")[1:3]
+
+        supported_types = ("track", "album", "artist", "playlist")
+        if uri_type and uri_type in supported_types and uri_id:
+            return {
+                "uri": uri,
+                "type": uri_type,
+                "id": uri_id,
+                "key": (uri_type, uri_id),
+            }
+        raise ValueError("Unknown error")
+    except Exception as e:
+        logger.exception(f"Could not parse {repr(uri)} as a Spotify URI ({e})")
 
 
 def _process_uri(web_client, uri):
@@ -80,7 +83,10 @@ def _process_uris(web_client, uri_type, uris):
 
         if uri["key"] not in _cache:
             if uri_type == "track":
-                album_key = _parse_uri(item["album"]["uri"])["key"]
+                album = _parse_uri(item["album"]["uri"])
+                if not album:
+                    continue
+                album_key = album["key"]
                 if album_key not in _cache:
                     _cache[album_key] = tuple(
                         _translate_image(i) for i in item["album"]["images"]

tests/test_images.py

@@ -127,6 +127,27 @@ def test_get_track_images(web_client_mock, img_provider):
     assert image.width == 640
 
 
+def test_get_track_images_bad_album_uri(web_client_mock, img_provider):
+    uris = ["spotify:track:41shEpOKyyadtG6lDclooa"]
+
+    web_client_mock.get.return_value = {
+        "tracks": [
+            {
+                "id": "41shEpOKyyadtG6lDclooa",
+                "album": {
+                    "uri": "spotify:bad-data",
+                    "images": [
+                        {"height": 640, "url": "img://1/a", "width": 640}
+                    ],
+                },
+            }
+        ]
+    }
+
+    result = img_provider.get_images(uris)
+    assert result == {}
+
+
 def test_get_relinked_track_images(web_client_mock, img_provider):
     uris = ["spotify:track:4nqN0p0FjfH39G3hxeuKad"]
 
@@ -167,7 +188,7 @@ def test_get_relinked_track_images(web_client_mock, img_provider):
 
 
 def test_get_playlist_image(web_client_mock, img_provider):
-    uris = ["spotify:playlist:41shEpOKyyadtG6lDclooa"]
+    uris = ["spotify:playlist:41shEpOKyyadtG6lDclooa", "foo:bar"]
 
     web_client_mock.get.return_value = {
         "id": "41shEpOKyyadtG6lDclooa",
@@ -181,7 +202,7 @@ def test_get_playlist_image(web_client_mock, img_provider):
     )
 
     assert len(result) == 1
-    assert sorted(result.keys()) == sorted(uris)
+    assert sorted(result.keys()) == ["spotify:playlist:41shEpOKyyadtG6lDclooa"]
     assert len(result[uris[0]]) == 1
 
     image = result[uris[0]][0]
@@ -231,11 +252,11 @@ def test_max_50_ids_per_request(web_client_mock, img_provider):
     assert request_ids_2 == "50"
 
 
-def test_invalid_uri_fails(img_provider):
-    with pytest.raises(ValueError) as exc:
-        img_provider.get_images(["foo:bar"])
-
-    assert str(exc.value) == "Could not parse 'foo:bar' as a Spotify URI"
+def test_invalid_uri(img_provider, caplog):
+    with caplog.at_level(5):
+        result = img_provider.get_images(["foo:bar"])
+    assert result == {}
+    assert "Could not parse 'foo:bar' as a Spotify URI" in caplog.text
 
 
 def test_no_uris_gives_no_results(img_provider):