From 29898b16c037e7b0ff7e35ee88c9534fd761a134 Mon Sep 17 00:00:00 2001 From: Rodrigo Braz Date: Fri, 12 Jun 2026 22:06:04 +0100 Subject: [PATCH 1/5] fix(exclusions): fail safe when JustWatch or Trakt are unavailable --- app/deleterr.py | 15 +++++++++++ app/media_cleaner.py | 41 +++++++++++++++++----------- app/modules/justwatch.py | 32 +++++++++++++++------- app/modules/trakt.py | 13 +++++++++ tests/modules/test_justwatch.py | 43 ++++++++++++++++++++++++++---- tests/modules/test_trakt.py | 37 +++++++++++++++++++++++++- tests/test_deleterr.py | 27 +++++++++++++++++++ tests/test_media_cleaner.py | 47 +++++++++++++++++++++++++++++++++ 8 files changed, 225 insertions(+), 30 deletions(-) diff --git a/app/deleterr.py b/app/deleterr.py index 00de7d0..d2f852d 100644 --- a/app/deleterr.py +++ b/app/deleterr.py @@ -15,6 +15,7 @@ from app import logger from app.config import hang_on_error, load_config from app.media_cleaner import ConfigurationError, MediaCleaner, parse_leaving_soon_duration +from app.modules.trakt import TraktError from app.modules.notifications import NotificationManager, RunResult, DeletedItem, LibraryStats from app.state import StateManager from app.utils import print_readable_freed_space @@ -790,6 +791,13 @@ def process_radarr(self): except ConfigurationError as e: logger.error(str(e)) self.libraries_failed += 1 + except TraktError as e: + logger.error( + f"Skipping library '{library_name}': {e}. " + "Trakt exclusions could not be verified, so no items were " + "deleted from this library this run." + ) + self.libraries_failed += 1 logger.log_freed_space(saved_space, "movie", self.config.settings.get("dry_run", True)) @@ -860,6 +868,13 @@ def process_sonarr(self): except ConfigurationError as e: logger.error(str(e)) self.libraries_failed += 1 + except TraktError as e: + logger.error( + f"Skipping library '{library_name}': {e}. " + "Trakt exclusions could not be verified, so no items were " + "deleted from this library this run." + ) + self.libraries_failed += 1 logger.log_freed_space(saved_space, "show", self.config.settings.get("dry_run", True)) diff --git a/app/media_cleaner.py b/app/media_cleaner.py index a165eac..21094eb 100644 --- a/app/media_cleaner.py +++ b/app/media_cleaner.py @@ -8,7 +8,7 @@ from pyarr.exceptions import PyarrResourceNotFound, PyarrServerError from app import logger -from app.modules.justwatch import JustWatch +from app.modules.justwatch import JustWatch, JustWatchError from app.modules.seerr import Seerr from app.modules.watch_provider import create_watch_provider from app.modules.mdblist import Mdblist @@ -1779,6 +1779,10 @@ def check_excluded_justwatch(media_data, plex_media_item, exclude, justwatch_ins Returns: True if media should NOT be excluded (i.e., is actionable) False if media should be excluded (i.e., skip this media) + + Fails safe: if the JustWatch API cannot be queried (timeout, rate limit, + network error), the item is excluded from deletion this run rather than + treating the outage as "not available on streaming". """ jw_config = exclude.get("justwatch", {}) @@ -1790,21 +1794,28 @@ def check_excluded_justwatch(media_data, plex_media_item, exclude, justwatch_ins # Determine media type based on data structure media_type = "movie" if "tmdbId" in media_data else "show" - # Check available_on mode (exclude if available on specified providers) - if providers := jw_config.get("available_on"): - if justwatch_instance.available_on(title, year, media_type, providers): - logger.debug( - f"{title} is available on streaming service(s) {providers}, skipping" - ) - return False + try: + # Check available_on mode (exclude if available on specified providers) + if providers := jw_config.get("available_on"): + if justwatch_instance.available_on(title, year, media_type, providers): + logger.debug( + f"{title} is available on streaming service(s) {providers}, skipping" + ) + return False - # Check not_available_on mode (exclude if NOT available on specified providers) - if providers := jw_config.get("not_available_on"): - if justwatch_instance.is_not_available_on(title, year, media_type, providers): - logger.debug( - f"{title} is not available on streaming service(s) {providers}, skipping" - ) - return False + # Check not_available_on mode (exclude if NOT available on specified providers) + if providers := jw_config.get("not_available_on"): + if justwatch_instance.is_not_available_on(title, year, media_type, providers): + logger.debug( + f"{title} is not available on streaming service(s) {providers}, skipping" + ) + return False + except JustWatchError as e: + logger.warning( + f"Could not check JustWatch availability for '{title}' ({year}): {e}. " + "Failing safe and skipping this item for this run." + ) + return False return True diff --git a/app/modules/justwatch.py b/app/modules/justwatch.py index c224d15..5fd98d8 100644 --- a/app/modules/justwatch.py +++ b/app/modules/justwatch.py @@ -17,6 +17,15 @@ _USER_AGENT = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" +class JustWatchError(Exception): + """Raised when the JustWatch API cannot be reached or returns an error. + + Distinguishes a service failure (timeout, rate limit, network error) from a + genuine "title not found" result, so callers can fail safe instead of + treating an outage as "not available on streaming". + """ + + def _get_graphql_url() -> str: """Get the JustWatch API URL, reading from environment at call time. @@ -55,10 +64,15 @@ def __init__(self, country, language): def _search(self, title, max_results=5, detailed=False): """ - Search for a title on JustWatch API with caching and error handling. + Search for a title on JustWatch API with caching. Returns: - List of MediaEntry objects, or empty list on error + List of MediaEntry objects (empty when the title is not found) + + Raises: + JustWatchError: when the JustWatch API cannot be reached or + returns an error, so callers can tell a service failure + apart from a "title not found" result. """ global _rate_limit_warned cache_key = f"{title}:{max_results}:{detailed}" @@ -72,15 +86,15 @@ def _search(self, title, max_results=5, detailed=False): self._search_cache[cache_key] = results _rate_limit_warned = False # Reset on successful request return results - except httpx.TimeoutException: + except httpx.TimeoutException as e: logger.warning(f"JustWatch API timeout while searching for '{title}' - the service may be slow") - return [] + raise JustWatchError(f"JustWatch API timeout for '{title}'") from e except httpx.HTTPStatusError as e: status_code = e.response.status_code if status_code == 429: if not _rate_limit_warned: logger.warning( - "JustWatch API rate limit hit. Streaming availability checks will be skipped. " + "JustWatch API rate limit hit. Streaming availability checks cannot be performed. " "Consider reducing the number of items processed per run." ) _rate_limit_warned = True @@ -90,13 +104,13 @@ def _search(self, title, max_results=5, detailed=False): logger.warning(f"JustWatch API server error (HTTP {status_code}) - service may be unavailable") else: logger.warning(f"JustWatch API error (HTTP {status_code}) for '{title}'") - return [] - except httpx.ConnectError: + raise JustWatchError(f"JustWatch API error (HTTP {status_code}) for '{title}'") from e + except httpx.ConnectError as e: logger.warning("Cannot reach JustWatch API - check internet connection") - return [] + raise JustWatchError(f"Cannot reach JustWatch API while searching for '{title}'") from e except Exception as e: logger.warning(f"JustWatch search failed for '{title}': {type(e).__name__}: {e}") - return [] + raise JustWatchError(f"JustWatch search failed for '{title}': {type(e).__name__}: {e}") from e def search_by_title_and_year(self, title, year, media_type): """ diff --git a/app/modules/trakt.py b/app/modules/trakt.py index dd5ecef..6cd2521 100644 --- a/app/modules/trakt.py +++ b/app/modules/trakt.py @@ -5,6 +5,15 @@ from app import logger +class TraktError(Exception): + """Raised when a configured Trakt list cannot be fetched. + + Treating a fetch failure as an empty list would silently disable the + exclusion and allow deletion of items the user wanted protected, so + callers must handle this error and fail safe instead. + """ + + class Trakt: def __init__(self, trakt_id, trakt_secret): self._configure_trakt(trakt_id, trakt_secret) @@ -53,6 +62,10 @@ def _fetch_list_items( except Exception as e: logger.error(f"Failed to fetch list items for {media_type} {listname}") logger.debug(f"Error: {e}") + raise TraktError( + f"Failed to fetch Trakt list '{listname}' for {media_type}s: " + f"{type(e).__name__}: {e}" + ) from e return [] def _fetch_user_list_items( diff --git a/tests/modules/test_justwatch.py b/tests/modules/test_justwatch.py index 7b7e5f9..01a35aa 100644 --- a/tests/modules/test_justwatch.py +++ b/tests/modules/test_justwatch.py @@ -1,8 +1,9 @@ from unittest.mock import MagicMock, patch +import httpx import pytest -from app.modules.justwatch import JustWatch +from app.modules.justwatch import JustWatch, JustWatchError @pytest.mark.unit @@ -40,11 +41,43 @@ def test_search_error_handling(self, mock_search): mock_search.side_effect = Exception("API Error") justwatch_instance = JustWatch("US", "en") - # Act - result = justwatch_instance._search("test_title") + # Act / Assert - errors must not look like "title not found" + with pytest.raises(JustWatchError): + justwatch_instance._search("test_title") + + @patch("app.modules.justwatch._search_justwatch") + def test_search_timeout_raises(self, mock_search): + # Arrange + mock_search.side_effect = httpx.TimeoutException("timed out") + justwatch_instance = JustWatch("US", "en") + + # Act / Assert + with pytest.raises(JustWatchError): + justwatch_instance._search("test_title") + + @patch("app.modules.justwatch._search_justwatch") + def test_search_rate_limit_raises(self, mock_search): + # Arrange + request = httpx.Request("POST", "https://apis.justwatch.com/graphql") + response = httpx.Response(429, request=request) + mock_search.side_effect = httpx.HTTPStatusError( + "429", request=request, response=response + ) + justwatch_instance = JustWatch("US", "en") + + # Act / Assert + with pytest.raises(JustWatchError): + justwatch_instance._search("test_title") + + @patch("app.modules.justwatch._search_justwatch") + def test_search_by_title_and_year_propagates_errors(self, mock_search): + # Arrange + mock_search.side_effect = httpx.ConnectError("no network") + justwatch_instance = JustWatch("US", "en") - # Assert - should return empty list on error - assert result == [] + # Act / Assert - callers must be able to tell error from not-found + with pytest.raises(JustWatchError): + justwatch_instance.search_by_title_and_year("title1", 2001, "movie") @patch("app.modules.justwatch._search_justwatch") def test_clear_cache(self, mock_search): diff --git a/tests/modules/test_trakt.py b/tests/modules/test_trakt.py index c4fec43..768037d 100644 --- a/tests/modules/test_trakt.py +++ b/tests/modules/test_trakt.py @@ -3,7 +3,12 @@ import pytest import trakt -from app.modules.trakt import Trakt, _process_trakt_item_list, extract_info_from_url +from app.modules.trakt import ( + Trakt, + TraktError, + _process_trakt_item_list, + extract_info_from_url, +) @pytest.fixture @@ -133,6 +138,36 @@ def test_fetch_list_items( assert result == [] +@patch.object( + Trakt, "_fetch_user_list_items", side_effect=Exception("Trakt API unavailable") +) +def test_fetch_list_items_failure_raises_trakt_error( + mock_user, trakt_instance_and_mock +): + """A fetch failure must not be silently treated as an empty exclusion list.""" + trakt_instance, _ = trakt_instance_and_mock + + with pytest.raises(TraktError): + trakt_instance._fetch_list_items("movie", "username", "listname", None, 100) + + +@patch.object( + Trakt, "_fetch_general_list_items", side_effect=Exception("Trakt API unavailable") +) +def test_get_all_items_for_url_propagates_fetch_failure( + mock_general, trakt_instance_and_mock +): + """Errors fetching a configured list must propagate to the caller.""" + trakt_instance, _ = trakt_instance_and_mock + trakt_config = { + "max_items_per_list": 100, + "lists": ["https://trakt.tv/movies/trending"], + } + + with pytest.raises(TraktError): + trakt_instance.get_all_items_for_url("movie", trakt_config) + + def test_process_trakt_item_list(): # Arrange items = {} diff --git a/tests/test_deleterr.py b/tests/test_deleterr.py index b8e7716..d847d8b 100644 --- a/tests/test_deleterr.py +++ b/tests/test_deleterr.py @@ -40,6 +40,33 @@ def test_process_radarr(radarr_mock, sonarr_mock, deleterr): ) +@patch("app.deleterr.DSonarr") +@patch("app.modules.radarr.DRadarr") +def test_process_radarr_trakt_failure_skips_library(radarr_mock, sonarr_mock, deleterr): + """A Trakt outage must skip the library (no deletions) instead of crashing.""" + from app.modules.trakt import TraktError + + # Arrange + deleterr.radarr = {"Radarr1": MagicMock()} + deleterr.config.settings = { + "libraries": [{"radarr": "Radarr1"}], + } + deleterr.media_cleaner.process_library_movies = MagicMock( + side_effect=TraktError("Failed to fetch Trakt list 'watchlist' for movies") + ) + deleterr.run_result = MagicMock() + failed_before = deleterr.libraries_failed + processed_before = deleterr.libraries_processed + + # Act - must not raise + deleterr.process_radarr() + + # Assert - library counted as failed, nothing deleted + assert deleterr.libraries_failed == failed_before + 1 + assert deleterr.libraries_processed == processed_before + deleterr.run_result.add_deleted.assert_not_called() + + @patch("app.deleterr.DSonarr") @patch("app.modules.radarr.DRadarr") def test_process_sonarr(radarr_mock, sonarr_mock, deleterr): diff --git a/tests/test_media_cleaner.py b/tests/test_media_cleaner.py index 2917377..9efc8f1 100644 --- a/tests/test_media_cleaner.py +++ b/tests/test_media_cleaner.py @@ -1852,6 +1852,53 @@ def test_not_available_on_does_not_exclude_when_available(self, mocker): assert result is True # Not excluded + def test_available_on_api_error_fails_safe(self, mocker): + """When the JustWatch API errors, the item must be excluded from deletion.""" + from app.media_cleaner import check_excluded_justwatch + from app.modules.justwatch import JustWatchError + + mock_logger = mocker.patch("app.media_cleaner.logger") + media_data = {"title": "Test Movie", "tmdbId": 123, "year": 2022} + plex_media_item = MagicMock() + plex_media_item.title = "Test Movie" + plex_media_item.year = 2022 + exclude = {"justwatch": {"available_on": ["netflix"]}} + + mock_justwatch = MagicMock() + mock_justwatch.available_on.side_effect = JustWatchError("API timeout") + + result = check_excluded_justwatch( + media_data, plex_media_item, exclude, mock_justwatch + ) + + # Fail safe: the check could not be performed, so skip the item + assert result is False + mock_logger.warning.assert_called_once() + assert "Test Movie" in mock_logger.warning.call_args[0][0] + + def test_not_available_on_api_error_fails_safe(self, mocker): + """When the JustWatch API errors, not_available_on must also fail safe.""" + from app.media_cleaner import check_excluded_justwatch + from app.modules.justwatch import JustWatchError + + mock_logger = mocker.patch("app.media_cleaner.logger") + media_data = {"title": "Test Movie", "tmdbId": 123, "year": 2022} + plex_media_item = MagicMock() + plex_media_item.title = "Test Movie" + plex_media_item.year = 2022 + exclude = {"justwatch": {"not_available_on": ["netflix"]}} + + mock_justwatch = MagicMock() + mock_justwatch.is_not_available_on.side_effect = JustWatchError("API down") + + result = check_excluded_justwatch( + media_data, plex_media_item, exclude, mock_justwatch + ) + + # Fail safe: the check could not be performed, so skip the item + assert result is False + mock_logger.warning.assert_called_once() + def test_detects_movie_type_from_tmdb_id(self, mocker): """Should detect movie type from tmdbId in media_data.""" from app.media_cleaner import check_excluded_justwatch From c7ab0baac82f7e0aac7886eb0aec92ca082cbbe7 Mon Sep 17 00:00:00 2001 From: Rodrigo Braz Date: Sun, 5 Jul 2026 09:56:13 +0100 Subject: [PATCH 2/5] refactor(deleterr): deduplicate library failure handling --- app/deleterr.py | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/app/deleterr.py b/app/deleterr.py index d2f852d..ea85c48 100644 --- a/app/deleterr.py +++ b/app/deleterr.py @@ -728,6 +728,22 @@ def _is_universe_member(self, plex_item, universe_ids): or (guids.get("imdb_id") and guids["imdb_id"] in universe_ids["imdb"]) ) + def _handle_library_failure(self, library_name, error): + """Log a per-library failure and count it, without aborting the run. + + TraktError means exclusions could not be verified, so the library is + skipped entirely (fail safe). ConfigurationError keeps its own message. + """ + if isinstance(error, TraktError): + logger.error( + f"Skipping library '{library_name}': {error}. " + "Trakt exclusions could not be verified, so no items were " + "deleted from this library this run." + ) + else: + logger.error(str(error)) + self.libraries_failed += 1 + def process_radarr(self): for name, radarr in self.radarr.items(): logger.info(f"Processing radarr instance: '{name}'") @@ -788,16 +804,8 @@ def process_radarr(self): # Log library completion time library_duration = time.time() - library_start logger.info(f"Library '{library_name}' completed in {logger.format_duration(library_duration)}") - except ConfigurationError as e: - logger.error(str(e)) - self.libraries_failed += 1 - except TraktError as e: - logger.error( - f"Skipping library '{library_name}': {e}. " - "Trakt exclusions could not be verified, so no items were " - "deleted from this library this run." - ) - self.libraries_failed += 1 + except (ConfigurationError, TraktError) as e: + self._handle_library_failure(library_name, e) logger.log_freed_space(saved_space, "movie", self.config.settings.get("dry_run", True)) @@ -865,16 +873,8 @@ def process_sonarr(self): # Log library completion time library_duration = time.time() - library_start logger.info(f"Library '{library_name}' completed in {logger.format_duration(library_duration)}") - except ConfigurationError as e: - logger.error(str(e)) - self.libraries_failed += 1 - except TraktError as e: - logger.error( - f"Skipping library '{library_name}': {e}. " - "Trakt exclusions could not be verified, so no items were " - "deleted from this library this run." - ) - self.libraries_failed += 1 + except (ConfigurationError, TraktError) as e: + self._handle_library_failure(library_name, e) logger.log_freed_space(saved_space, "show", self.config.settings.get("dry_run", True)) From 0770bb62bc78c06bb42d5647a5d885fdfe432876 Mon Sep 17 00:00:00 2001 From: Rodrigo Braz Date: Sun, 5 Jul 2026 10:14:08 +0100 Subject: [PATCH 3/5] fix(exclusions): fail safe when Mdblist is unavailable --- app/deleterr.py | 14 ++++++++------ app/modules/mdblist.py | 23 +++++++++++++++++++++-- tests/modules/test_mdblist.py | 29 +++++++++++++---------------- tests/test_deleterr.py | 27 +++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 24 deletions(-) diff --git a/app/deleterr.py b/app/deleterr.py index ea85c48..8871d4e 100644 --- a/app/deleterr.py +++ b/app/deleterr.py @@ -15,6 +15,7 @@ from app import logger from app.config import hang_on_error, load_config from app.media_cleaner import ConfigurationError, MediaCleaner, parse_leaving_soon_duration +from app.modules.mdblist import MdblistError from app.modules.trakt import TraktError from app.modules.notifications import NotificationManager, RunResult, DeletedItem, LibraryStats from app.state import StateManager @@ -731,13 +732,14 @@ def _is_universe_member(self, plex_item, universe_ids): def _handle_library_failure(self, library_name, error): """Log a per-library failure and count it, without aborting the run. - TraktError means exclusions could not be verified, so the library is - skipped entirely (fail safe). ConfigurationError keeps its own message. + TraktError/MdblistError mean exclusions could not be verified, so the + library is skipped entirely (fail safe). ConfigurationError keeps its + own message. """ - if isinstance(error, TraktError): + if isinstance(error, (TraktError, MdblistError)): logger.error( f"Skipping library '{library_name}': {error}. " - "Trakt exclusions could not be verified, so no items were " + "Exclusions could not be verified, so no items were " "deleted from this library this run." ) else: @@ -804,7 +806,7 @@ def process_radarr(self): # Log library completion time library_duration = time.time() - library_start logger.info(f"Library '{library_name}' completed in {logger.format_duration(library_duration)}") - except (ConfigurationError, TraktError) as e: + except (ConfigurationError, TraktError, MdblistError) as e: self._handle_library_failure(library_name, e) logger.log_freed_space(saved_space, "movie", self.config.settings.get("dry_run", True)) @@ -873,7 +875,7 @@ def process_sonarr(self): # Log library completion time library_duration = time.time() - library_start logger.info(f"Library '{library_name}' completed in {logger.format_duration(library_duration)}") - except (ConfigurationError, TraktError) as e: + except (ConfigurationError, TraktError, MdblistError) as e: self._handle_library_failure(library_name, e) logger.log_freed_space(saved_space, "show", self.config.settings.get("dry_run", True)) diff --git a/app/modules/mdblist.py b/app/modules/mdblist.py index 75bc0bc..667e51e 100644 --- a/app/modules/mdblist.py +++ b/app/modules/mdblist.py @@ -6,6 +6,15 @@ from app import logger +class MdblistError(Exception): + """Raised when a configured Mdblist list cannot be fetched. + + Treating a fetch failure as an empty list would silently disable the + exclusion and allow deletion of items the user wanted protected, so + callers must handle this error and fail safe instead. + """ + + class Mdblist: def __init__(self, api_key, ssl_verify=True): self.api_key = api_key @@ -26,8 +35,10 @@ def get_all_items_for_url(self, media_type, mdblist_config): def _fetch_list_items(self, list_url, media_type, max_items_per_list): list_path = extract_list_path(list_url) if not list_path: - logger.error(f"Could not extract list path from URL: {list_url}") - return [] + # A misparsed URL must not silently disable the exclusion + raise MdblistError( + f"Could not extract list path from Mdblist URL: {list_url}" + ) # API returns {"movies": [...], "shows": [...]} response_key = "movies" if media_type == "movie" else "shows" @@ -67,9 +78,17 @@ def _fetch_list_items(self, list_url, media_type, max_items_per_list): if not has_more: break + except MdblistError: + raise except Exception as e: logger.error(f"Failed to fetch Mdblist items from {list_url}") logger.debug(f"Error: {e}") + # Never return a partial page set as if it were the full list - + # the caller would treat it as complete and delete unprotected items + raise MdblistError( + f"Failed to fetch Mdblist list '{list_url}': " + f"{type(e).__name__}: {e}" + ) from e return all_items[:max_items_per_list] diff --git a/tests/modules/test_mdblist.py b/tests/modules/test_mdblist.py index 0a4fee3..da01a92 100644 --- a/tests/modules/test_mdblist.py +++ b/tests/modules/test_mdblist.py @@ -3,7 +3,7 @@ import pytest import requests -from app.modules.mdblist import Mdblist, _process_mdblist_item_list, extract_list_path +from app.modules.mdblist import MdblistError, Mdblist, _process_mdblist_item_list, extract_list_path @pytest.mark.parametrize( @@ -176,38 +176,35 @@ def test_process_mdblist_item_list_show_missing_tvdbid(): @patch("app.modules.mdblist.requests.get") -def test_fetch_list_items_error_handling(mock_get): - """Test that network errors are handled gracefully.""" +def test_fetch_list_items_error_raises(mock_get): + """A network error must not look like an empty exclusion list.""" mdblist = Mdblist("test_api_key") mock_get.side_effect = requests.exceptions.ConnectionError("Connection failed") - result = mdblist._fetch_list_items("https://mdblist.com/lists/user/list", "movie", 1000) - - assert result == [] + with pytest.raises(MdblistError): + mdblist._fetch_list_items("https://mdblist.com/lists/user/list", "movie", 1000) @patch("app.modules.mdblist.requests.get") -def test_fetch_list_items_http_error(mock_get): - """Test that HTTP errors (e.g., rate limiting) are handled gracefully.""" +def test_fetch_list_items_http_error_raises(mock_get): + """HTTP errors (e.g. rate limiting) must fail safe, not fail open.""" mdblist = Mdblist("test_api_key") response = MagicMock() response.raise_for_status.side_effect = requests.exceptions.HTTPError("429 Too Many Requests") mock_get.return_value = response - result = mdblist._fetch_list_items("https://mdblist.com/lists/user/list", "movie", 1000) - - assert result == [] + with pytest.raises(MdblistError): + mdblist._fetch_list_items("https://mdblist.com/lists/user/list", "movie", 1000) -def test_fetch_list_items_invalid_url(): - """Test that invalid URLs return empty list.""" +def test_fetch_list_items_invalid_url_raises(): + """A misparsed URL is a config problem, not an empty list.""" mdblist = Mdblist("test_api_key") - result = mdblist._fetch_list_items("https://example.com/not-mdblist", "movie", 1000) - - assert result == [] + with pytest.raises(MdblistError): + mdblist._fetch_list_items("https://example.com/not-mdblist", "movie", 1000) @patch("app.modules.mdblist.requests.get") diff --git a/tests/test_deleterr.py b/tests/test_deleterr.py index d847d8b..9d7b53a 100644 --- a/tests/test_deleterr.py +++ b/tests/test_deleterr.py @@ -67,6 +67,33 @@ def test_process_radarr_trakt_failure_skips_library(radarr_mock, sonarr_mock, de deleterr.run_result.add_deleted.assert_not_called() +@patch("app.deleterr.DSonarr") +@patch("app.modules.radarr.DRadarr") +def test_process_radarr_mdblist_failure_skips_library(radarr_mock, sonarr_mock, deleterr): + """An mdblist outage must skip the library (no deletions) instead of failing open.""" + from app.modules.mdblist import MdblistError + + # Arrange + deleterr.radarr = {"Radarr1": MagicMock()} + deleterr.config.settings = { + "libraries": [{"radarr": "Radarr1"}], + } + deleterr.media_cleaner.process_library_movies = MagicMock( + side_effect=MdblistError("Failed to fetch Mdblist list 'protected'") + ) + deleterr.run_result = MagicMock() + failed_before = deleterr.libraries_failed + processed_before = deleterr.libraries_processed + + # Act - must not raise + deleterr.process_radarr() + + # Assert - library counted as failed, nothing deleted + assert deleterr.libraries_failed == failed_before + 1 + assert deleterr.libraries_processed == processed_before + deleterr.run_result.add_deleted.assert_not_called() + + @patch("app.deleterr.DSonarr") @patch("app.modules.radarr.DRadarr") def test_process_sonarr(radarr_mock, sonarr_mock, deleterr): From d6f9a343a13f0e92e89e5edfed7768cb4bac2866 Mon Sep 17 00:00:00 2001 From: Rodrigo Braz Date: Sun, 5 Jul 2026 10:17:18 +0100 Subject: [PATCH 4/5] fix(safety): skip libraries with empty watch history instead of treating all items as unwatched --- app/deleterr.py | 10 ++++--- app/media_cleaner.py | 56 +++++++++++++++++++++++++++++++++++++ app/schema.py | 4 +++ tests/test_deleterr.py | 27 ++++++++++++++++++ tests/test_media_cleaner.py | 46 ++++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 4 deletions(-) diff --git a/app/deleterr.py b/app/deleterr.py index 8871d4e..7e3a31a 100644 --- a/app/deleterr.py +++ b/app/deleterr.py @@ -14,7 +14,7 @@ from app import logger from app.config import hang_on_error, load_config -from app.media_cleaner import ConfigurationError, MediaCleaner, parse_leaving_soon_duration +from app.media_cleaner import ConfigurationError, MediaCleaner, WatchDataError, parse_leaving_soon_duration from app.modules.mdblist import MdblistError from app.modules.trakt import TraktError from app.modules.notifications import NotificationManager, RunResult, DeletedItem, LibraryStats @@ -736,7 +736,9 @@ def _handle_library_failure(self, library_name, error): library is skipped entirely (fail safe). ConfigurationError keeps its own message. """ - if isinstance(error, (TraktError, MdblistError)): + if isinstance(error, WatchDataError): + logger.error(f"Skipping library '{library_name}': {error}") + elif isinstance(error, (TraktError, MdblistError)): logger.error( f"Skipping library '{library_name}': {error}. " "Exclusions could not be verified, so no items were " @@ -806,7 +808,7 @@ def process_radarr(self): # Log library completion time library_duration = time.time() - library_start logger.info(f"Library '{library_name}' completed in {logger.format_duration(library_duration)}") - except (ConfigurationError, TraktError, MdblistError) as e: + except (ConfigurationError, TraktError, MdblistError, WatchDataError) as e: self._handle_library_failure(library_name, e) logger.log_freed_space(saved_space, "movie", self.config.settings.get("dry_run", True)) @@ -875,7 +877,7 @@ def process_sonarr(self): # Log library completion time library_duration = time.time() - library_start logger.info(f"Library '{library_name}' completed in {logger.format_duration(library_duration)}") - except (ConfigurationError, TraktError, MdblistError) as e: + except (ConfigurationError, TraktError, MdblistError, WatchDataError) as e: self._handle_library_failure(library_name, e) logger.log_freed_space(saved_space, "show", self.config.settings.get("dry_run", True)) diff --git a/app/media_cleaner.py b/app/media_cleaner.py index 21094eb..0bad133 100644 --- a/app/media_cleaner.py +++ b/app/media_cleaner.py @@ -476,6 +476,50 @@ def get_show_activity(self, library, plex_library): def get_movie_activity(self, library, movies_library): return self.watch_provider.get_activity(movies_library.key) + @staticmethod + def _library_uses_watch_rules(library): + """True when deletion decisions for this library depend on watch data + in the dangerous direction (empty data => everything actionable). + + watch_status: watched already fails safe on empty data (nothing + matches), so it does not need the guard. + """ + return ( + library.get("watch_status") == "unwatched" + or library.get("last_watched_threshold") is not None + or bool(library.get("apply_last_watch_threshold_to_collections")) + ) + + def guard_empty_watch_activity(self, library, activity, library_size): + """Refuse to act on a non-empty library with zero watch activity. + + An empty activity set makes find_watched_data return None for every + item, bypassing last_watched_threshold and the unwatched check - one + degraded Tautulli response (DB rebuild, section mismatch, empty auth + result) would mass-delete recently watched content. Fail safe instead. + + Genuinely never-watched libraries can opt out with + allow_empty_watch_history: true. + """ + if activity or not library_size: + return + if not self._library_uses_watch_rules(library): + return + if library.get("allow_empty_watch_history", False): + logger.warning( + f"Library '{library.get('name')}' has no watch history but " + f"allow_empty_watch_history is set - treating all " + f"{library_size} items as unwatched." + ) + return + raise WatchDataError( + f"No watch history returned for library '{library.get('name')}' " + f"({library_size} items). Treating every item as unwatched could " + f"mass-delete watched content, so this library is skipped. If the " + f"library really has never been watched, set " + f"allow_empty_watch_history: true for it." + ) + def filter_shows(self, library, unfiltered_all_show_data): return [ show @@ -529,6 +573,7 @@ def process_library(self, library, sonarr_instance, unfiltered_all_show_data): show_activity = self.get_show_activity(library, plex_library) logger.info(f"Got {len(show_activity)} items in tautulli activity") + self.guard_empty_watch_activity(library, show_activity, plex_library.totalSize) return self.process_shows( library, @@ -677,6 +722,7 @@ def process_library_movies(self, library, radarr_instance): logger.info(f"Got {len(mdblist_movies)} mdblist items to exclude") movie_activity = self.get_movie_activity(library, movies_library) + self.guard_empty_watch_activity(library, movie_activity, movies_library.totalSize) return self.process_movies( library, @@ -2244,6 +2290,16 @@ class ConfigurationError(Exception): pass +class WatchDataError(Exception): + """Raised when watch activity data looks unusable for deletion decisions. + + An empty activity set makes every item look never-watched, which with + unwatched/threshold rules turns the entire library into deletion + candidates. Callers must handle this error and skip the library instead. + """ + pass + + def library_meets_disk_space_threshold(library, dpyarr_instance): for item in library.get("disk_size_threshold", []): path = item.get("path") diff --git a/app/schema.py b/app/schema.py index fca2fc5..7f26f18 100644 --- a/app/schema.py +++ b/app/schema.py @@ -572,6 +572,10 @@ class LibraryConfig(BaseModel): description="Days since added to Plex. Media added within this period is protected", json_schema_extra={"example": 180}, ) + allow_empty_watch_history: bool = Field( + default=False, + description="Allow watch-based rules to run when the library has zero watch history. Off by default: empty history makes everything look unwatched, so the library is skipped to avoid mass deletion", + ) apply_last_watch_threshold_to_collections: bool = Field( default=False, description="Apply last watched threshold to all items in the same collection", diff --git a/tests/test_deleterr.py b/tests/test_deleterr.py index 9d7b53a..d5041ad 100644 --- a/tests/test_deleterr.py +++ b/tests/test_deleterr.py @@ -94,6 +94,33 @@ def test_process_radarr_mdblist_failure_skips_library(radarr_mock, sonarr_mock, deleterr.run_result.add_deleted.assert_not_called() +@patch("app.deleterr.DSonarr") +@patch("app.modules.radarr.DRadarr") +def test_process_radarr_empty_watch_data_skips_library(radarr_mock, sonarr_mock, deleterr): + """Unusable watch data must skip the library (no deletions) instead of crashing.""" + from app.media_cleaner import WatchDataError + + # Arrange + deleterr.radarr = {"Radarr1": MagicMock()} + deleterr.config.settings = { + "libraries": [{"radarr": "Radarr1"}], + } + deleterr.media_cleaner.process_library_movies = MagicMock( + side_effect=WatchDataError("No watch history returned for library 'Movies'") + ) + deleterr.run_result = MagicMock() + failed_before = deleterr.libraries_failed + processed_before = deleterr.libraries_processed + + # Act - must not raise + deleterr.process_radarr() + + # Assert - library counted as failed, nothing deleted + assert deleterr.libraries_failed == failed_before + 1 + assert deleterr.libraries_processed == processed_before + deleterr.run_result.add_deleted.assert_not_called() + + @patch("app.deleterr.DSonarr") @patch("app.modules.radarr.DRadarr") def test_process_sonarr(radarr_mock, sonarr_mock, deleterr): diff --git a/tests/test_media_cleaner.py b/tests/test_media_cleaner.py index 9efc8f1..9ea7082 100644 --- a/tests/test_media_cleaner.py +++ b/tests/test_media_cleaner.py @@ -2896,3 +2896,49 @@ def test_no_filename_match_returns_none(self): # Search for non-existent file result = index.find_by_filename("Nonexistent.Movie.2020.mkv") assert result is None + + +class TestGuardEmptyWatchActivity: + """Empty watch activity must not turn a library into deletion candidates (#287).""" + + def test_raises_for_unwatched_rule_with_empty_activity(self, media_cleaner): + from app.media_cleaner import WatchDataError + + library = {"name": "Movies", "watch_status": "unwatched"} + with pytest.raises(WatchDataError): + media_cleaner.guard_empty_watch_activity(library, {}, 500) + + def test_raises_for_last_watched_threshold_with_empty_activity(self, media_cleaner): + from app.media_cleaner import WatchDataError + + library = {"name": "Movies", "last_watched_threshold": 90} + with pytest.raises(WatchDataError): + media_cleaner.guard_empty_watch_activity(library, {}, 500) + + def test_passes_with_activity(self, media_cleaner): + library = {"name": "Movies", "watch_status": "unwatched"} + media_cleaner.guard_empty_watch_activity(library, {"guid": {}}, 500) + + def test_passes_for_empty_library(self, media_cleaner): + library = {"name": "Movies", "watch_status": "unwatched"} + media_cleaner.guard_empty_watch_activity(library, {}, 0) + + def test_passes_without_watch_rules(self, media_cleaner): + # No watch-based rules: empty activity is irrelevant to decisions + library = {"name": "Movies", "added_at_threshold": 180} + media_cleaner.guard_empty_watch_activity(library, {}, 500) + + def test_passes_for_watched_rule(self, media_cleaner): + # watch_status: watched fails safe on empty data (nothing matches) + library = {"name": "Movies", "watch_status": "watched"} + media_cleaner.guard_empty_watch_activity(library, {}, 500) + + def test_opt_out_allows_empty_history(self, media_cleaner, mocker): + mock_logger = mocker.patch("app.media_cleaner.logger") + library = { + "name": "Movies", + "watch_status": "unwatched", + "allow_empty_watch_history": True, + } + media_cleaner.guard_empty_watch_activity(library, {}, 500) + mock_logger.warning.assert_called_once() From 5d266134b284b7eeb3c44ff36f3f01627e5ec573 Mon Sep 17 00:00:00 2001 From: Rodrigo Braz Date: Sun, 5 Jul 2026 10:18:06 +0100 Subject: [PATCH 5/5] docs(schema): include allow_empty_watch_history in generated configuration reference --- scripts/generate_docs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/generate_docs.py b/scripts/generate_docs.py index 65650d1..2bb5c9c 100644 --- a/scripts/generate_docs.py +++ b/scripts/generate_docs.py @@ -964,6 +964,7 @@ def main(): # Generate library table without nested objects library_fields = ["name", "radarr", "sonarr", "series_type", "action_mode", "watch_status", "last_watched_threshold", "added_at_threshold", "apply_last_watch_threshold_to_collections", + "allow_empty_watch_history", "add_list_exclusion_on_delete", "max_actions_per_run", "preview_next", "disk_size_threshold", "sort", "leaving_soon"] library_lines = ["| Property | Type | Required | Default | Description |"]