Continue with the other RSS feed URIs when one fails

interfaces/Config/templates/config_rss.tmpl

@@ -106,10 +106,11 @@
                 <a href="$url('config/rss')">$T('cmenu-rss')</a> &raquo;
                 $active_feed
             </h2>
-            <!--#if $error#-->
+            <!--#if $errors#-->
             <div class="alert alert-danger">
-                <span class="glyphicon glyphicon-exclamation-sign"></span>
-                $error
+                <!--#for $error in $errors#-->
+                <div><span class="glyphicon glyphicon-exclamation-sign"></span> $error</div>
+                <!--#end for#-->
             </div>
             <!--#end if#-->
             <form action="$url('config/rss/upd_rss_feed')" method="post">

sabnzbd/interface.py

@@ -1366,12 +1366,12 @@ def _rss_redirect(feed: str = "") -> RedirectResponse:
     return base_redirect_response(_RSS_ROOT)
 
 
-def _rss_flash_redirect(request: Request, feed: str, msg: str = "") -> RedirectResponse:
+def _rss_flash_redirect(request: Request, feed: str, errors: Optional[list[str]] = None) -> RedirectResponse:
     """Store a feed read-out result as a one-shot flash in the client session and
     redirect back to the RSS page. The flash lives in the per-client signed
     session cookie rather than shared module state, so concurrent requests (other
     tabs, the API path) can't clobber each other's result."""
-    request.session["rss_flash"] = {"feed": feed, "msg": msg}
+    request.session["rss_flash"] = {"feed": feed, "errors": errors or []}
     return _rss_redirect(feed)
 
 
@@ -1412,7 +1412,7 @@ def config_rss_index(request: Request):
         # re-evaluation is performed by the POST action handler that redirected
         # us, which leaves its result message as a one-shot flash in the session.
         flash = request.session.pop("rss_flash", None)
-        conf["error"] = flash["msg"] if flash and flash.get("feed") == active_feed else ""
+        conf["errors"] = flash["errors"] if flash and flash.get("feed") == active_feed else []
         conf["downloaded"], conf["matched"], conf["unmatched"] = GetRssLog(active_feed)
 
     # Find a unique new Feed name
@@ -1527,8 +1527,8 @@ def config_rss_add_rss_feed(request: Request):
             config.save_config()
             # Read out the new feed now (this handler runs in the threadpool) and
             # carry the result message to the redirected page via the session flash.
-            msg = sabnzbd.RSSReader.process_feed(feed, readout=True, ignore_first=True)
-            return _rss_flash_redirect(request, feed, msg)
+            errors = sabnzbd.RSSReader.process_feed(feed, readout=True, ignore_first=True)
+            return _rss_flash_redirect(request, feed, errors)
         else:
             return base_redirect_response(_RSS_ROOT)
     else:
@@ -1567,8 +1567,8 @@ def config_rss_download_rss_feed(request: Request):
     if not feed:
         return _rss_redirect()
     # Network read-out with forced download; this handler runs in the threadpool.
-    msg = sabnzbd.RSSReader.process_feed(feed, readout=True, download=True, force=True)
-    return _rss_flash_redirect(request, feed, msg)
+    errors = sabnzbd.RSSReader.process_feed(feed, readout=True, download=True, force=True)
+    return _rss_flash_redirect(request, feed, errors)
 
 
 @secured_expose(route="/config/rss/clean_rss_jobs", check_configlock=True, methods=["POST"])
@@ -1590,14 +1590,14 @@ def config_rss_test_rss_feed(request: Request):
     if not feed:
         return _rss_redirect()
     # Network read-out; this handler runs in the threadpool.
-    msg = sabnzbd.RSSReader.process_feed(feed, readout=True, ignore_first=True)
+    errors = sabnzbd.RSSReader.process_feed(feed, readout=True, ignore_first=True)
     # This endpoint is only called via AJAX; the client navigates to the feed
     # page itself once we return. Returning a redirect here would make the XHR
     # follow it transparently and consume the one-shot session flash before the
     # browser navigation can read it, so store the flash and return a plain
     # response instead.
-    request.session["rss_flash"] = {"feed": feed, "msg": msg}
-    return PlainTextResponse(msg)
+    request.session["rss_flash"] = {"feed": feed, "errors": errors}
+    return PlainTextResponse("\n".join(errors))
 
 
 @secured_expose(route="/config/rss/eval_rss_feed", check_configlock=True, methods=["POST"])

sabnzbd/rss.py

@@ -960,15 +960,19 @@ class RSSReader:
         ignore_first: bool = False,
         force: bool = False,
         readout: bool = True,
-    ) -> str:
-        """Run the query for one URI and apply filters"""
+    ) -> list[str]:
+        """Run the query for one URI and apply filters
+
+        Returns the problems that were encountered, if any.
+        """
         self.shutdown = False
 
         if not feed:
-            return "No such feed"
+            return ["No such feed"]
 
         new_links: set[str] = set()
         new_downloads: list[str] = []
+        errors: list[str] = []
 
         # Configuration
         try:
@@ -976,7 +980,7 @@ class RSSReader:
         except KeyError:
             logging.error(T('Incorrect RSS feed description "%s"'), feed)
             logging.info("Traceback: ", exc_info=True)
-            return T('Incorrect RSS feed description "%s"') % feed
+            return [T('Incorrect RSS feed description "%s"') % feed]
 
         uris = feeds.uri()
         filters = FeedConfig.from_config(feeds)
@@ -987,45 +991,43 @@ class RSSReader:
 
             # Fetch & parse RSS
             if readout:
-                gen = self.fetch_rss(feed, uris)
+                gen = self.fetch_rss(feed, uris, errors)
             else:
                 gen = repo.get_feed_jobs(feed=feed)
 
             # Evaluate rules and apply side effects
-            try:
-                for entry in gen:
-                    if self.shutdown:
-                        return ""
+            for entry in gen:
+                if self.shutdown:
+                    return []
 
-                    # Skip duplicates across multiple feeds
-                    if entry.link in new_links or (len(uris) > 1 and repo.is_duplicate(entry)):
-                        logging.info("Ignoring job %s from other feed", entry.title)
-                        continue
+                # Skip duplicates across multiple feeds
+                if entry.link in new_links or (len(uris) > 1 and repo.is_duplicate(entry)):
+                    logging.info("Ignoring job %s from other feed", entry.title)
+                    continue
 
-                    # Track all valid links so obsolete ones can be cleaned up later
-                    new_links.add(entry.link)
+                # Track all valid links so obsolete ones can be cleaned up later
+                new_links.add(entry.link)
 
-                    downloaded = self._process_entry(
-                        feed_entry=entry,
-                        filters=filters,
-                        first=first,
-                        download=download,
-                        force=force,
-                        readout=readout,
-                    )
-                    if downloaded:
-                        new_downloads.append(entry.title)
-            except RuntimeError as e:
-                return str(e)
+                downloaded = self._process_entry(
+                    feed_entry=entry,
+                    filters=filters,
+                    first=first,
+                    download=download,
+                    force=force,
+                    readout=readout,
+                )
+                if downloaded:
+                    new_downloads.append(entry.title)
 
             # Send email if wanted and not "forced"
             if new_downloads and cfg.email_rss() and not force:
                 emailer.rss_mail(feed, new_downloads)
 
-            if readout:
+            if readout and not errors:
                 repo.remove_obsolete(feed, new_links, purge_downloaded=True)
 
-        return ""
+        # Report every problem, not just the last one, but never the same one twice
+        return list(dict.fromkeys(errors))
 
     @staticmethod
     def patch_feedparser():
@@ -1075,8 +1077,11 @@ class RSSReader:
         feedparser_mixin._start_nzedb_attr = _start_newznab_attr
         feedparser_mixin._start_nntmux_attr = _start_newznab_attr
 
-    def fetch_rss(self, feed: str, uris: list[str]) -> Generator[ResolvedEntry, Any, None]:
-        """Fetch and parse RSS feeds for the given URIs."""
+    def fetch_rss(self, feed: str, uris: list[str], errors: list[str]) -> Generator[ResolvedEntry, Any, None]:
+        """Fetch and parse RSS feeds for the given URIs
+
+        Failures are collected in errors so the remaining URIs are still read out.
+        """
 
         with sabnzbd.rss.rss_repository() as repo:
             for uri in uris:
@@ -1094,21 +1099,14 @@ class RSSReader:
                     logging.debug("Finished parsing %s", uri)
 
                     status = feed_parsed.get("status", 999)
-                    if status in (401, 402, 403):
-                        raise RuntimeError(T("Do not have valid authentication for feed %s") % uri)
-                    elif 500 <= status <= 599:
-                        raise RuntimeError(
-                            T("Server side error (server code %s); could not get %s on %s") % (status, feed, uri)
-                        )
-
                     entries = feed_parsed.get("entries", [])
-                    if not entries and "feed" in feed_parsed and "error" in feed_parsed["feed"]:
-                        raise RuntimeError(
-                            T("Failed to retrieve RSS from %s: %s") % (uri, feed_parsed["feed"]["error"])
-                        )
-
-                    # Exception was thrown
-                    if "bozo_exception" in feed_parsed and not entries:
+                    if status in (401, 402, 403):
+                        msg = T("Do not have valid authentication for feed %s") % uri
+                    elif 500 <= status <= 599:
+                        msg = T("Server side error (server code %s); could not get %s on %s") % (status, feed, uri)
+                    elif not entries and "feed" in feed_parsed and "error" in feed_parsed["feed"]:
+                        msg = T("Failed to retrieve RSS from %s: %s") % (uri, feed_parsed["feed"]["error"])
+                    elif "bozo_exception" in feed_parsed and not entries:
                         msg = str(feed_parsed["bozo_exception"])
                         if "CERTIFICATE_VERIFY_FAILED" in msg:
                             msg = T("Server %s uses an untrusted HTTPS certificate") % get_base_url(uri)
@@ -1122,11 +1120,13 @@ class RSSReader:
                     if msg:
                         # We need to escape any "%20" that could be in the warning due to the URL's
                         helpful_warning(urllib.parse.unquote(msg))
-                        raise RuntimeError(msg)
+                        errors.append(msg)
+                        continue
                     elif not entries:
                         msg = T("RSS Feed %s was empty") % uri
                         logging.info(msg)
-                        raise RuntimeError(msg)
+                        errors.append(msg)
+                        continue
 
                     for entry in entries:
                         normalised = ResolvedEntry.from_feed_entry(feed, entry)
@@ -1140,7 +1140,7 @@ class RSSReader:
                 except (AttributeError, IndexError):
                     logging.info(T("Incompatible feed") + " " + uri)
                     logging.info("Traceback: ", exc_info=True)
-                    raise RuntimeError(T("Incompatible feed"))
+                    errors.append(T("Incompatible feed"))
 
     def _process_entry(
         self,
@@ -1231,23 +1231,25 @@ class RSSReader:
             if self.next_run < time.time():
                 self.next_run = time.time() + cfg.rss_rate() * 60
             feeds = config.get_rss()
-            try:
-                for feed in feeds:
-                    if feeds[feed].enable():
-                        logging.info('Starting scheduled RSS read-out for "%s"', feed)
-                        active = True
-                        self.process_feed(feed, download=True, ignore_first=True)
-                        # Wait 15 seconds, else sites may get irritated
-                        for _ in range(15):
-                            if self.shutdown:
-                                return
-                            else:
-                                time.sleep(1.0)
-            except (KeyError, RuntimeError):
-                # Feed must have been deleted
-                logging.info("RSS read-out crashed, feed must have been deleted or edited")
-                logging.debug("Traceback: ", exc_info=True)
-                pass
+            for feed in list(feeds):
+                try:
+                    if not feeds[feed].enable():
+                        continue
+                    logging.info('Starting scheduled RSS read-out for "%s"', feed)
+                    active = True
+                    self.process_feed(feed, download=True, ignore_first=True)
+                except Exception:
+                    # Feed must have been deleted, continue with the other feeds
+                    logging.info("RSS read-out crashed, feed must have been deleted or edited")
+                    logging.debug("Traceback: ", exc_info=True)
+                    continue
+
+                # Wait 15 seconds, else sites may get irritated
+                for _ in range(15):
+                    if self.shutdown:
+                        return
+                    else:
+                        time.sleep(1.0)
             if active:
                 logging.info("Finished scheduled RSS read-outs")
 

tests/test_rss.py

@@ -958,7 +958,7 @@ class TestRSS:
         # First run: ignore_first=True, download=True (scheduled-like behaviour)
         # This should mark the entry as GOOD+initial_scan, but not download it
         msg_first = reader.process_feed(feed_name, download=True, ignore_first=True)
-        assert msg_first == ""
+        assert msg_first == []
 
         job_first = repo.find_job_by_url(feed_name, "http://example.test/starred-episode")
         assert job_first is not None
@@ -968,7 +968,7 @@ class TestRSS:
 
         # Simulate a later run: readout only, no download
         msg_second = reader.process_feed(feed_name, download=True, ignore_first=False)
-        assert msg_second == ""
+        assert msg_second == []
 
         job_second = repo.find_job_by_url(feed_name, "http://example.test/starred-episode")
         assert job_second is not None
@@ -981,7 +981,7 @@ class TestRSS:
         # Third phase: force download; this should clear the starred status
         add_url_mock = mocker.patch("sabnzbd.urlgrabber.add_url")
         msg_third = reader.process_feed(feed_name, download=True, ignore_first=False, force=True)
-        assert msg_third == ""
+        assert msg_third == []
         assert add_url_mock.call_count == 1
 
         job_third = repo.find_job_by_url(feed_name, "http://example.test/starred-episode")
@@ -1048,6 +1048,52 @@ class TestRSS:
         # Shared link must only appear once
         assert links == {shared_link, a_only_link, b_only_link}
 
+    def test_rssreader_failing_uri_does_not_block_other_uris(self, httpserver: HTTPServer, tmp_rss):
+        """An empty or failing URI must not stop the remaining URIs of the same feed."""
+        repo, reader = tmp_rss
+        good_link = "http://example.test/still-read"
+
+        empty_xml = """<?xml version="1.0" encoding="utf-8"?>
+        <rss version="2.0">
+        <channel>
+            <title>Empty</title>
+        </channel>
+        </rss>
+        """
+        good_xml = f"""<?xml version="1.0" encoding="utf-8"?>
+        <rss version="2.0">
+        <channel>
+            <title>Good</title>
+            <item>
+                <title>Some.Show.S01E01.720p</title>
+                <link>{good_link}</link>
+                <guid>http://example.test/info/still-read</guid>
+                <category>tv</category>
+                <pubDate>Wed, 01 Jan 2025 00:00:00 GMT</pubDate>
+            </item>
+        </channel>
+        </rss>
+        """
+
+        httpserver.expect_request("/rss_empty.xml").respond_with_data(empty_xml, content_type="application/rss+xml")
+        httpserver.expect_request("/rss_error.xml").respond_with_data("", status=500)
+        httpserver.expect_request("/rss_good.xml").respond_with_data(good_xml, content_type="application/rss+xml")
+
+        feed_name = "PartlyFailingFeed"
+        uri_empty = httpserver.url_for("/rss_empty.xml")
+        uri_error = httpserver.url_for("/rss_error.xml")
+        uri_good = httpserver.url_for("/rss_good.xml")
+        self.setup_rss(feed_name, f"{uri_empty} {uri_error} {uri_good}")
+
+        msg = reader.process_feed(feed_name)
+
+        # Both problems are reported, but the working URI was read out
+        assert msg == [
+            "RSS Feed %s was empty" % uri_empty,
+            "Server side error (server code 500); could not get %s on %s" % (feed_name, uri_error),
+        ]
+        assert repo.find_job_by_url(feed_name, good_link) is not None
+
     def test_purge_removed_feeds_only_drops_unconfigured_feeds(self, tmp_rss):
         """Records should only be dropped for feeds that are no longer configured."""
         repo, _reader = tmp_rss
@@ -1118,11 +1164,11 @@ class TestRSS:
             )
         )
 
-        assert reader.process_feed(feed_name, readout=False) == ""
+        assert reader.process_feed(feed_name, readout=False) == []
         assert repo.find_job_by_url(feed_name, old_url) is not None
 
         # A real readout does not find the link anymore, so it gets purged
-        assert reader.process_feed(feed_name, readout=True) == ""
+        assert reader.process_feed(feed_name, readout=True) == []
         assert repo.find_job_by_url(feed_name, old_url) is None
 
     def test_downloaded_item_still_in_feed_is_not_redownloaded(self, httpserver: HTTPServer, tmp_rss, mocker):
@@ -1148,7 +1194,7 @@ class TestRSS:
         self.setup_rss(feed_name, httpserver.url_for("/rss_long_lived.xml"))
 
         add_url = mocker.patch("sabnzbd.urlgrabber.add_url")
-        assert reader.process_feed(feed_name, download=True, force=True) == ""
+        assert reader.process_feed(feed_name, download=True, force=True) == []
         assert add_url.call_count == 1
         assert repo.find_job_by_url(feed_name, link).state is RSSState.DOWNLOADED
 
@@ -1158,12 +1204,12 @@ class TestRSS:
         repo.db.execute("UPDATE rss SET seen_at = ? WHERE feed = ?", (stale, feed_name))
 
         # Still being listed should refresh seen_at instead of purging the job
-        assert reader.process_feed(feed_name, download=True) == ""
+        assert reader.process_feed(feed_name, download=True) == []
         job = repo.find_job_by_url(feed_name, link)
         assert job is not None
         assert job.state is RSSState.DOWNLOADED
         assert job.seen_at.timestamp() > stale
 
         # And it must not be picked up as a new job on the next scans
-        assert reader.process_feed(feed_name, download=True) == ""
+        assert reader.process_feed(feed_name, download=True) == []
         assert add_url.call_count == 1