Repository: postlund/pyatv

Issue:
Fetching artwork sometime fails with bad error handling
**Describe the bug**
As reported here: https://github.com/home-assistant/core/issues/44305#issuecomment-756771464

Stack trace:

```python
Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/aiohttp/web_protocol.py", line 422, in _handle_request
    resp = await self._request_handler(request)
  File "/usr/local/lib/python3.8/site-packages/aiohttp/web_app.py", line 499, in _handle
    resp = await handler(request)
  File "/usr/local/lib/python3.8/site-packages/aiohttp/web_middlewares.py", line 118, in impl
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/request_context.py", line 18, in request_context_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/ban.py", line 72, in ban_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/auth.py", line 127, in auth_middleware
    return await handler(request)
  File "/usr/src/homeassistant/homeassistant/components/http/view.py", line 129, in handle
    result = await result
  File "/usr/src/homeassistant/homeassistant/components/media_player/__init__.py", line 971, in get
    data, content_type = await player.async_get_media_image()
  File "/config/custom_components/apple_tv/media_player.py", line 205, in async_get_media_image
    artwork = await self.atv.metadata.artwork()
  File "/usr/local/lib/python3.8/site-packages/pyatv/mrp/__init__.py", line 441, in artwork
    artwork = await self._fetch_artwork(width or 0, height or -1)
  File "/usr/local/lib/python3.8/site-packages/pyatv/mrp/__init__.py", line 456, in _fetch_artwork
    item = resp.inner().playbackQueue.contentItems[playing.location]
  File "/usr/local/lib/python3.8/site-packages/google/protobuf/internal/containers.py", line 209, in __getitem__
    return self._values[key]
IndexError: list index out of range
```

I should add better error handling and just fail silently.

**To Reproduce**
I expect that this can happen when trying to fetch artwork when it's not available, even though pyatv thinks so. One such case is when the metadata is wrong (bug in app) or if metadata is stuck in pyatv so that it doesn't match what's playing (bug in pyatv). I might be able to reproduce in a functional test.

**Expected behavior**
Maybe put a debug log in there and fail silently.

**System Setup (please complete the following information):**
 - OS: any
 - Python: any
 - pyatv: 0.7.5
- Apple TV: any with tvOS

**Additional context**


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/postlund/pyatv/pull/1033?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) Report
> Merging [#1033](https://codecov.io/gh/postlund/pyatv/pull/1033?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) (db82b9c) into [master](https://codecov.io/gh/postlund/pyatv/commit/021cc7a1f90538bf92889dca455ed0250dfd3a49?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) (021cc7a) will **increase** coverage by `0.04%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/postlund/pyatv/pull/1033/graphs/tree.svg?width=650&height=150&src=pr&token=wb6azJjRSY&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)](https://codecov.io/gh/postlund/pyatv/pull/1033?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)

```diff
@@            Coverage Diff             @@
##           master    #1033      +/-   ##
==========================================
+ Coverage   93.98%   94.03%   +0.04%     
==========================================
  Files          47       47              
  Lines        4471     4473       +2     
==========================================
+ Hits         4202     4206       +4     
+ Misses        269      267       -2     
```


| [Impacted Files](https://codecov.io/gh/postlund/pyatv/pull/1033?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl) | Coverage Δ | |
|---|---|---|
| [pyatv/dmap/\_\_init\_\_.py](https://codecov.io/gh/postlund/pyatv/pull/1033/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvZG1hcC9fX2luaXRfXy5weQ==) | `96.82% <100.00%> (ø)` | |
| [pyatv/interface.py](https://codecov.io/gh/postlund/pyatv/pull/1033/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvaW50ZXJmYWNlLnB5) | `99.07% <100.00%> (ø)` | |
| [pyatv/mrp/\_\_init\_\_.py](https://codecov.io/gh/postlund/pyatv/pull/1033/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvbXJwL19faW5pdF9fLnB5) | `95.06% <100.00%> (+0.28%)` | :arrow_up: |
| [pyatv/mrp/protocol.py](https://codecov.io/gh/postlund/pyatv/pull/1033/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl#diff-cHlhdHYvbXJwL3Byb3RvY29sLnB5) | `95.55% <0.00%> (+0.74%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/postlund/pyatv/pull/1033?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/postlund/pyatv/pull/1033?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl). Last update [5df2295...db82b9c](https://codecov.io/gh/postlund/pyatv/pull/1033?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pierre+St%C3%A5hl).


Patch:
diff --git a/docs/api/pyatv/interface.html b/docs/api/pyatv/interface.html
index f0247954..6cf36c17 100644
--- a/docs/api/pyatv/interface.html
+++ b/docs/api/pyatv/interface.html
@@ -857,7 +857,9 @@ class Metadata(ABC):
 
     @abstractmethod
     @feature(30, &#34;Artwork&#34;, &#34;Playing media artwork.&#34;)
-    async def artwork(self, width=512, height=None) -&gt; Optional[ArtworkInfo]:
+    async def artwork(
+        self, width: Optional[int] = 512, height: Optional[int] = None
+    ) -&gt; Optional[ArtworkInfo]:
         &#34;&#34;&#34;Return artwork for what is currently playing (or None).
 
         The parameters &#34;width&#34; and &#34;height&#34; makes it possible to request artwork of a
@@ -2145,7 +2147,9 @@ in one of the listed states.</p></section>
 
     @abstractmethod
     @feature(30, &#34;Artwork&#34;, &#34;Playing media artwork.&#34;)
-    async def artwork(self, width=512, height=None) -&gt; Optional[ArtworkInfo]:
+    async def artwork(
+        self, width: Optional[int] = 512, height: Optional[int] = None
+    ) -&gt; Optional[ArtworkInfo]:
         &#34;&#34;&#34;Return artwork for what is currently playing (or None).
 
         The parameters &#34;width&#34; and &#34;height&#34; makes it possible to request artwork of a
@@ -2244,7 +2248,7 @@ def device_id(self) -&gt; Optional[str]:
 <h3>Methods</h3>
 <dl>
 <dt id="pyatv.interface.Metadata.artwork"><code class="name flex">
-<span>async def <span class="ident">artwork</span></span>(<span>self, width=512, height=None) -> Optional[<a title="pyatv.interface.ArtworkInfo" href="#pyatv.interface.ArtworkInfo">ArtworkInfo</a>]</span>
+<span>async def <span class="ident">artwork</span></span>(<span>self, width: Optional[int] = 512, height: Optional[int] = None) -> Optional[<a title="pyatv.interface.ArtworkInfo" href="#pyatv.interface.ArtworkInfo">ArtworkInfo</a>]</span>
 </code></dt>
 <dd>
 <section class="desc"><p>Return artwork for what is currently playing (or None).</p>
@@ -2259,7 +2263,9 @@ aspect ratio.</p></section>
 </summary>
 <pre><code class="python">@abstractmethod
 @feature(30, &#34;Artwork&#34;, &#34;Playing media artwork.&#34;)
-async def artwork(self, width=512, height=None) -&gt; Optional[ArtworkInfo]:
+async def artwork(
+    self, width: Optional[int] = 512, height: Optional[int] = None
+) -&gt; Optional[ArtworkInfo]:
     &#34;&#34;&#34;Return artwork for what is currently playing (or None).
 
     The parameters &#34;width&#34; and &#34;height&#34; makes it possible to request artwork of a
diff --git a/pyatv/dmap/__init__.py b/pyatv/dmap/__init__.py
index ccf0e827..3d5fa0d4 100644
--- a/pyatv/dmap/__init__.py
+++ b/pyatv/dmap/__init__.py
@@ -205,7 +205,9 @@ class BaseDmapAppleTV:
         self.latest_hash = self.latest_playing.hash
         return self.latest_playing
 
-    async def artwork(self, width, height) -> Optional[ArtworkInfo]:
+    async def artwork(
+        self, width: Optional[int], height: Optional[int]
+    ) -> Optional[ArtworkInfo]:
         """Return artwork for what is currently playing (or None)."""
         url = _ARTWORK_CMD.format(width=width or 0, height=height or 0)
         art = await self.daap.get(url, daap_data=False)
@@ -405,7 +407,9 @@ class DmapMetadata(Metadata):
         self.apple_tv = apple_tv
         self.artwork_cache = Cache(limit=4)
 
-    async def artwork(self, width=512, height=None) -> Optional[ArtworkInfo]:
+    async def artwork(
+        self, width: Optional[int] = 512, height: Optional[int] = None
+    ) -> Optional[ArtworkInfo]:
         """Return artwork for what is currently playing (or None).
 
         The parameters "width" and "height" makes it possible to request artwork of a
diff --git a/pyatv/interface.py b/pyatv/interface.py
index 005d43c9..44aa7dcf 100644
--- a/pyatv/interface.py
+++ b/pyatv/interface.py
@@ -629,7 +629,9 @@ class Metadata(ABC):
 
     @abstractmethod
     @feature(30, "Artwork", "Playing media artwork.")
-    async def artwork(self, width=512, height=None) -> Optional[ArtworkInfo]:
+    async def artwork(
+        self, width: Optional[int] = 512, height: Optional[int] = None
+    ) -> Optional[ArtworkInfo]:
         """Return artwork for what is currently playing (or None).
 
         The parameters "width" and "height" makes it possible to request artwork of a
diff --git a/pyatv/mrp/__init__.py b/pyatv/mrp/__init__.py
index 30d17ebc..4dd3774f 100644
--- a/pyatv/mrp/__init__.py
+++ b/pyatv/mrp/__init__.py
@@ -420,7 +420,9 @@ class MrpMetadata(Metadata):
         self.psm = psm
         self.artwork_cache = Cache(limit=4)
 
-    async def artwork(self, width=512, height=None) -> Optional[ArtworkInfo]:
+    async def artwork(
+        self, width: Optional[int] = 512, height: Optional[int] = None
+    ) -> Optional[ArtworkInfo]:
         """Return artwork for what is currently playing (or None).
 
         The parameters "width" and "height" makes it possible to request artwork of a
@@ -438,14 +440,17 @@ class MrpMetadata(Metadata):
             _LOGGER.debug("Retrieved artwork %s from cache", identifier)
             return self.artwork_cache.get(identifier)
 
-        artwork = await self._fetch_artwork(width or 0, height or -1)
-        if artwork:
+        artwork: Optional[ArtworkInfo] = None
+        try:
+            artwork = await self._fetch_artwork(width or 0, height or -1)
+        except Exception:
+            _LOGGER.warning("Artwork not present in response")
+        else:
             self.artwork_cache.put(identifier, artwork)
-            return artwork
 
-        return None
+        return artwork
 
-    async def _fetch_artwork(self, width, height):
+    async def _fetch_artwork(self, width, height) -> Optional[ArtworkInfo]:
         playing = self.psm.playing
         resp = await self.psm.protocol.send_and_receive(
             messages.playback_queue_request(playing.location, width, height)


Test patch:
diff --git a/tests/fake_device/mrp.py b/tests/fake_device/mrp.py
index cf452939..7d2ea31f 100644
--- a/tests/fake_device/mrp.py
+++ b/tests/fake_device/mrp.py
@@ -480,12 +480,15 @@ class FakeMrpService(MrpServerAuth, asyncio.Protocol):
         setstate = messages.create(
             protobuf.SET_STATE_MESSAGE, identifier=message.identifier
         )
-        queue = setstate.inner().playbackQueue
-        queue.location = 0
-        item = queue.contentItems.add()
-        item.artworkData = self.state.states[self.state.active_player].artwork
-        item.artworkDataWidth = state.artwork_width or 456
-        item.artworkDataHeight = state.artwork_height or 789
+
+        artwork_data = self.state.states[self.state.active_player].artwork
+        if artwork_data:
+            queue = setstate.inner().playbackQueue
+            queue.location = 0
+            item = queue.contentItems.add()
+            item.artworkData = artwork_data
+            item.artworkDataWidth = state.artwork_width or 456
+            item.artworkDataHeight = state.artwork_height or 789
         self.send_to_client(setstate)
 
     def handle_wake_device(self, message, inner):
diff --git a/tests/mrp/test_mrp_functional.py b/tests/mrp/test_mrp_functional.py
index f26f1390..31baa327 100644
--- a/tests/mrp/test_mrp_functional.py
+++ b/tests/mrp/test_mrp_functional.py
@@ -168,6 +168,19 @@ class MRPFunctionalTest(common_functional_tests.CommonFunctionalTests):
         await self.playing(title="dummy")
         self.assertEqual(self.atv.metadata.artwork_id, "some_id")
 
+    @unittest_run_loop
+    async def test_metadata_artwork_erroneously_available(self):
+        self.usecase.example_video()
+
+        # Metadata suggests that artwork is available but no artwork is available
+        # when requested by client
+        self.usecase.change_artwork(None, ARTWORK_MIMETYPE, ARTWORK_ID)
+
+        await self.playing(title="dummy")
+
+        artwork = await self.atv.metadata.artwork(width=123, height=456)
+        self.assertIsNone(artwork)
+
     @unittest_run_loop
     async def test_metadata_artwork_width_and_height(self):
         self.usecase.example_video()