From 8921ae24fb240c16940c62eb67aa93d6b22828ed Mon Sep 17 00:00:00 2001
From: Shaowen Yin <cosven@users.noreply.github.com>
Date: Mon, 15 Jul 2024 12:25:10 +0800
Subject: [PATCH] player: add timeout when fetching song metadata (#853)

---
 feeluown/library/library.py   |  9 ++++++++-
 feeluown/player/playlist.py   | 20 +++++++++++++-------
 feeluown/utils/aio.py         |  3 +++
 tests/player/test_playlist.py | 10 ++++++----
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/feeluown/library/library.py b/feeluown/library/library.py
index 7542cb7bfb..9ffa03bab7 100644
--- a/feeluown/library/library.py
+++ b/feeluown/library/library.py
@@ -292,6 +292,9 @@ def song_prepare_media(self, song: BriefSongModel, policy) -> Media:
                 song_web_url = provider.song_get_web_url(song)
                 logger.info(f'use ytdl to get media for {song_web_url}')
                 media = self.ytdl.select_audio(song_web_url, policy, source=song.source)
+                found = media is not None
+                logger.debug(f'ytdl select audio for {song_web_url} finished, '
+                             f'found: {found}')
             if not media:
                 raise MediaNotFound('provider returns empty media')
         return media
@@ -425,7 +428,6 @@ def video_prepare_media(self, video: BriefVideoModel, policy) -> Media:
         :param video: either a v1 MvModel or a v2 (Brief)VideoModel.
         """
         provider = self.get(video.source)
-
         try:
             if isinstance(provider, SupportsVideoMultiQuality):
                 media, _ = provider.video_select_media(video, policy)
@@ -440,6 +442,11 @@ def video_prepare_media(self, video: BriefVideoModel, policy) -> Media:
                 media = self.ytdl.select_video(video_web_url,
                                                policy,
                                                source=video.source)
+                found = media is not None
+                logger.debug(f'ytdl select video for {video_web_url} finished, '
+                             f'found: {found}')
+            else:
+                media = None
             if not media:
                 raise
         return media
diff --git a/feeluown/player/playlist.py b/feeluown/player/playlist.py
index 41c5dc37c4..d5beaeaeee 100644
--- a/feeluown/player/playlist.py
+++ b/feeluown/player/playlist.py
@@ -600,26 +600,32 @@ async def _prepare_metadata_for_song(self, song):
             MetadataFields.album: song.album_name_display or '',
         })
         try:
-            song = await aio.run_fn(self._app.library.song_upgrade, song)
+            song: SongModel = await aio.wait_for(
+                aio.run_fn(self._app.library.song_upgrade, song),
+                timeout=1,
+            )
         except ResourceNotFound:
             return metadata
         except:  # noqa
             logger.exception(f"fetching song's meta failed, song:'{song.title_display}'")
             return metadata
 
-        artwork = ''
-        released = ''
-        if song.album is not None:
+        artwork = song.pic_url
+        released = song.date
+        if not (artwork and released) and song.album is not None:
             try:
-                album = await aio.run_fn(self._app.library.album_upgrade, song.album)
+                album = await aio.wait_for(
+                    aio.run_fn(self._app.library.album_upgrade, song.album),
+                    timeout=1
+                )
             except ResourceNotFound:
                 pass
             except:  # noqa
                 logger.warning(
                     f"fetching song's album meta failed, song:{song.title_display}")
             else:
-                artwork = album.cover
-                released = album.released
+                artwork = album.cover or artwork
+                released = album.released or released
                 # For model v1, the cover can be a Media object.
                 # For example, in fuo_local plugin, the album.cover is a Media
                 # object with url set to fuo://local/songs/{identifier}/data/cover.
diff --git a/feeluown/utils/aio.py b/feeluown/utils/aio.py
index 6616a92196..0dd5ff7dd3 100644
--- a/feeluown/utils/aio.py
+++ b/feeluown/utils/aio.py
@@ -36,6 +36,9 @@
 #: gather is an alias of `asyncio.gather`.
 gather = asyncio.gather
 
+#: wait_for is an alias of `asyncio.wait_for`
+wait_for = asyncio.wait_for
+
 
 def run_in_executor(executor, func, *args):
     """alias for loop.run_in_executor"""
diff --git a/tests/player/test_playlist.py b/tests/player/test_playlist.py
index 6cc04a8166..ae8dfa9453 100644
--- a/tests/player/test_playlist.py
+++ b/tests/player/test_playlist.py
@@ -355,13 +355,15 @@ def test_playlist_next_should_call_set_current_song(app_mock, mocker, song):
 
 
 @pytest.mark.asyncio
-async def test_playlist_prepare_metadata_for_song(app_mock, pl, ekaf_brief_song0):
+async def test_playlist_prepare_metadata_for_song(
+        app_mock, library, pl, ekaf_brief_song0, mocker):
     class Album:
         cover = Media('fuo://')
         released = '2018-01-01'
+
+    app_mock.library = library
     album = Album()
-    f = asyncio.Future()
-    f.set_result(album)
-    app_mock.library.album_upgrade.return_value = album
+    mocker.patch.object(library, 'album_upgrade', return_value=album)
+    # app_mock.library.album_upgrade.return_value = album
     # When cover is a media object, prepare_metadata should also succeed.
     await pl._prepare_metadata_for_song(ekaf_brief_song0)