mirror of
https://github.com/nathom/streamrip.git
synced 2025-05-24 12:07:14 -04:00
Fix: Prevent individual track failures from aborting entire playlist/album downloads (#814)
* add tests (failing) * Improve error handling and logging in media download processes - Add exception handling and logging for track and media item downloads - Capture and log errors during album, playlist, and media ripping - Provide summary of failed items when download process completes
This commit is contained in:
parent
dc7ca02529
commit
7697ae7b6e
4 changed files with 137 additions and 11 deletions
|
@ -32,12 +32,19 @@ class Album(Media):
|
||||||
|
|
||||||
async def download(self):
|
async def download(self):
|
||||||
async def _resolve_and_download(pending: Pending):
|
async def _resolve_and_download(pending: Pending):
|
||||||
|
try:
|
||||||
track = await pending.resolve()
|
track = await pending.resolve()
|
||||||
if track is None:
|
if track is None:
|
||||||
return
|
return
|
||||||
await track.rip()
|
await track.rip()
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(f"Error downloading track: {e}")
|
||||||
|
|
||||||
await asyncio.gather(*[_resolve_and_download(p) for p in self.tracks])
|
results = await asyncio.gather(*[_resolve_and_download(p) for p in self.tracks], return_exceptions=True)
|
||||||
|
|
||||||
|
for result in results:
|
||||||
|
if isinstance(result, Exception):
|
||||||
|
logger.error(f"Album track processing error: {result}")
|
||||||
|
|
||||||
async def postprocess(self):
|
async def postprocess(self):
|
||||||
progress.remove_title(self.meta.album)
|
progress.remove_title(self.meta.album)
|
||||||
|
|
|
@ -121,17 +121,25 @@ class Playlist(Media):
|
||||||
track_resolve_chunk_size = 20
|
track_resolve_chunk_size = 20
|
||||||
|
|
||||||
async def _resolve_download(item: PendingPlaylistTrack):
|
async def _resolve_download(item: PendingPlaylistTrack):
|
||||||
|
try:
|
||||||
track = await item.resolve()
|
track = await item.resolve()
|
||||||
if track is None:
|
if track is None:
|
||||||
return
|
return
|
||||||
await track.rip()
|
await track.rip()
|
||||||
|
except Exception as e:
|
||||||
|
logger.error(f"Error downloading track: {e}")
|
||||||
|
|
||||||
batches = self.batch(
|
batches = self.batch(
|
||||||
[_resolve_download(track) for track in self.tracks],
|
[_resolve_download(track) for track in self.tracks],
|
||||||
track_resolve_chunk_size,
|
track_resolve_chunk_size,
|
||||||
)
|
)
|
||||||
|
|
||||||
for batch in batches:
|
for batch in batches:
|
||||||
await asyncio.gather(*batch)
|
results = await asyncio.gather(*batch, return_exceptions=True)
|
||||||
|
|
||||||
|
for result in results:
|
||||||
|
if isinstance(result, Exception):
|
||||||
|
logger.error(f"Batch processing error: {result}")
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def batch(iterable, n=1):
|
def batch(iterable, n=1):
|
||||||
|
|
|
@ -164,7 +164,17 @@ class Main:
|
||||||
|
|
||||||
async def rip(self):
|
async def rip(self):
|
||||||
"""Download all resolved items."""
|
"""Download all resolved items."""
|
||||||
await asyncio.gather(*[item.rip() for item in self.media])
|
results = await asyncio.gather(*[item.rip() for item in self.media], return_exceptions=True)
|
||||||
|
|
||||||
|
failed_items = 0
|
||||||
|
for result in results:
|
||||||
|
if isinstance(result, Exception):
|
||||||
|
logger.error(f"Error processing media item: {result}")
|
||||||
|
failed_items += 1
|
||||||
|
|
||||||
|
if failed_items > 0:
|
||||||
|
total_items = len(self.media)
|
||||||
|
logger.info(f"Download completed with {failed_items} failed items out of {total_items} total items.")
|
||||||
|
|
||||||
async def search_interactive(self, source: str, media_type: str, query: str):
|
async def search_interactive(self, source: str, media_type: str, query: str):
|
||||||
client = await self.get_logged_in_client(source)
|
client = await self.get_logged_in_client(source)
|
||||||
|
|
101
tests/test_error_handling.py
Normal file
101
tests/test_error_handling.py
Normal file
|
@ -0,0 +1,101 @@
|
||||||
|
import asyncio
|
||||||
|
import json
|
||||||
|
import pytest
|
||||||
|
from unittest.mock import AsyncMock, MagicMock, patch
|
||||||
|
|
||||||
|
from streamrip.media.playlist import Playlist, PendingPlaylistTrack
|
||||||
|
from streamrip.media.album import Album, PendingTrack
|
||||||
|
from streamrip.exceptions import NonStreamableError
|
||||||
|
|
||||||
|
|
||||||
|
class TestErrorHandling:
|
||||||
|
"""Test error handling in playlist and album downloads."""
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_playlist_handles_failed_track(self):
|
||||||
|
"""Test that a playlist download continues even if one track fails."""
|
||||||
|
mock_config = MagicMock()
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_db = MagicMock()
|
||||||
|
|
||||||
|
mock_track_success = MagicMock()
|
||||||
|
mock_track_success.resolve = AsyncMock(return_value=MagicMock())
|
||||||
|
mock_track_success.resolve.return_value.rip = AsyncMock()
|
||||||
|
|
||||||
|
mock_track_failure = MagicMock()
|
||||||
|
mock_track_failure.resolve = AsyncMock(side_effect=json.JSONDecodeError("Expecting value", "", 0))
|
||||||
|
|
||||||
|
playlist = Playlist(
|
||||||
|
name="Test Playlist",
|
||||||
|
config=mock_config,
|
||||||
|
client=mock_client,
|
||||||
|
tracks=[mock_track_success, mock_track_failure]
|
||||||
|
)
|
||||||
|
|
||||||
|
await playlist.download()
|
||||||
|
|
||||||
|
mock_track_success.resolve.assert_called_once()
|
||||||
|
mock_track_success.resolve.return_value.rip.assert_called_once()
|
||||||
|
mock_track_failure.resolve.assert_called_once()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_album_handles_failed_track(self):
|
||||||
|
"""Test that an album download continues even if one track fails."""
|
||||||
|
mock_config = MagicMock()
|
||||||
|
mock_client = MagicMock()
|
||||||
|
mock_db = MagicMock()
|
||||||
|
mock_meta = MagicMock()
|
||||||
|
|
||||||
|
# Create a list of mock tracks - one will succeed, one will fail
|
||||||
|
mock_track_success = MagicMock()
|
||||||
|
mock_track_success.resolve = AsyncMock(return_value=MagicMock())
|
||||||
|
mock_track_success.resolve.return_value.rip = AsyncMock()
|
||||||
|
|
||||||
|
# This track will raise a JSONDecodeError when resolved
|
||||||
|
mock_track_failure = MagicMock()
|
||||||
|
mock_track_failure.resolve = AsyncMock(side_effect=json.JSONDecodeError("Expecting value", "", 0))
|
||||||
|
|
||||||
|
album = Album(
|
||||||
|
meta=mock_meta,
|
||||||
|
config=mock_config,
|
||||||
|
tracks=[mock_track_success, mock_track_failure],
|
||||||
|
folder="/test/folder",
|
||||||
|
db=mock_db
|
||||||
|
)
|
||||||
|
|
||||||
|
await album.download()
|
||||||
|
|
||||||
|
mock_track_success.resolve.assert_called_once()
|
||||||
|
mock_track_success.resolve.return_value.rip.assert_called_once()
|
||||||
|
mock_track_failure.resolve.assert_called_once()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_main_rip_handles_failed_media(self):
|
||||||
|
"""Test that the Main.rip method handles failed media items."""
|
||||||
|
from streamrip.rip.main import Main
|
||||||
|
|
||||||
|
mock_config = MagicMock()
|
||||||
|
|
||||||
|
mock_config.session.downloads.requests_per_minute = 0
|
||||||
|
mock_config.session.database.downloads_enabled = False
|
||||||
|
mock_config.session.database.failed_downloads_enabled = False
|
||||||
|
|
||||||
|
with patch('streamrip.rip.main.QobuzClient'), \
|
||||||
|
patch('streamrip.rip.main.TidalClient'), \
|
||||||
|
patch('streamrip.rip.main.DeezerClient'), \
|
||||||
|
patch('streamrip.rip.main.SoundcloudClient'):
|
||||||
|
|
||||||
|
main = Main(mock_config)
|
||||||
|
|
||||||
|
mock_media_success = MagicMock()
|
||||||
|
mock_media_success.rip = AsyncMock()
|
||||||
|
|
||||||
|
mock_media_failure = MagicMock()
|
||||||
|
mock_media_failure.rip = AsyncMock(side_effect=Exception("Media download failed"))
|
||||||
|
|
||||||
|
main.media = [mock_media_success, mock_media_failure]
|
||||||
|
|
||||||
|
await main.rip()
|
||||||
|
|
||||||
|
mock_media_success.rip.assert_called_once()
|
||||||
|
mock_media_failure.rip.assert_called_once()
|
Loading…
Add table
Add a link
Reference in a new issue