Fix invalid feeds persisting after failed initial update

discord_rss_bot/feeds.py

@@ -82,6 +82,17 @@ type UpdateCallback = Callable[[], UpdatedFeed | None]
 class FeedUpdateError(HTTPException):
     """Raised when the initial update for a newly added feed fails."""
 
+    def __init__(
+        self,
+        *,
+        status_code: int,
+        detail: str,
+        autodiscover_links: object | None = None,
+    ) -> None:
+        """Initialize an initial-update error and preserve advertised feed links."""
+        super().__init__(status_code=status_code, detail=detail)
+        self.autodiscover_links = autodiscover_links
+
 
 class JsonResponseLike(Protocol):
     """Response interface needed for Discord webhook JSON parsing."""
@@ -1894,6 +1905,22 @@ def truncate_webhook_message(webhook_message: str) -> str:
     return webhook_message
 
 
+def get_raw_autodiscover_links(reader: Reader, feed_url: str) -> object | None:
+    """Return advertised feed links stored after a failed initial update."""
+    try:
+        return reader.get_tag(feed_url, ".reader.autodiscover", None)
+    except ReaderError:
+        return None
+
+
+def remove_invalid_new_feed(reader: Reader, feed_url: str) -> None:
+    """Remove a feed inserted before its initial update failed."""
+    try:
+        reader.delete_feed(feed_url)
+    except ReaderError:
+        logger.exception("Failed to remove invalid feed after initial update: %s", feed_url)
+
+
 def create_feed(reader: Reader, feed_url: str, webhook_dropdown: str) -> None:  # noqa: C901, PLR0912
     """Add a new feed, update it and mark every entry as read.
 
@@ -1908,6 +1935,7 @@ def create_feed(reader: Reader, feed_url: str, webhook_dropdown: str) -> None:
     """
     clean_feed_url: str = feed_url.strip()
     webhook_url: str = ""
+    feed_was_added: bool = False
     if hooks := reader.get_tag((), "webhooks", []):
         # Get the webhook URL from the dropdown.
         for hook in hooks:
@@ -1924,6 +1952,7 @@ def create_feed(reader: Reader, feed_url: str, webhook_dropdown: str) -> None:
 
     try:
         reader.add_feed(clean_feed_url)
+        feed_was_added = True
     except FeedExistsError:
         # Add the webhook to an already added feed if it doesn't have a webhook instead of trying to create a new.
         if not reader.get_tag(clean_feed_url, "webhook", ""):
@@ -1934,7 +1963,16 @@ def create_feed(reader: Reader, feed_url: str, webhook_dropdown: str) -> None:
     try:
         reader.update_feed(clean_feed_url)
     except ReaderError as e:
-        raise FeedUpdateError(status_code=404, detail=f"Error updating feed: {e}") from e
+        autodiscover_links = get_raw_autodiscover_links(reader, clean_feed_url)
+
+        if feed_was_added:
+            remove_invalid_new_feed(reader, clean_feed_url)
+
+        raise FeedUpdateError(
+            status_code=404,
+            detail=f"Error updating feed: {e}",
+            autodiscover_links=autodiscover_links,
+        ) from e
 
     # Mark every entry as read, so we don't send all the old entries to Discord.
     entries: Iterable[Entry] = reader.get_entries(feed=clean_feed_url, read=False)

discord_rss_bot/main.py

@@ -282,20 +282,26 @@ def get_global_delivery_mode(reader: Reader) -> str:
     return global_delivery_mode if global_delivery_mode in {"embed", "text"} else "embed"
 
 
-def get_autodiscover_links(reader: Reader, feed_url: str) -> list[AutodiscoverLink]:
+def get_autodiscover_links(
+    reader: Reader,
+    feed_url: str,
+    stored_links: object | None = None,
+) -> list[AutodiscoverLink]:
     """Return valid autodiscovered links stored for a failed feed update.
 
     Args:
         reader: The Reader instance.
         feed_url: The URL that failed to parse as a feed.
+        stored_links: Optional advertised links preserved before deleting an invalid feed.
 
     Returns:
         Valid discovered feed link dictionaries.
     """
-    try:
-        stored_links = reader.get_tag(feed_url, ".reader.autodiscover", [])
-    except ReaderError:
-        return []
+    if stored_links is None:
+        try:
+            stored_links = reader.get_tag(feed_url, ".reader.autodiscover", [])
+        except ReaderError:
+            return []
 
     if not isinstance(stored_links, list):
         return []
@@ -440,7 +446,7 @@ async def post_create_feed(
     try:
         create_feed(reader, feed_url, webhook_dropdown)
     except FeedUpdateError as exception:
-        autodiscover_links = get_autodiscover_links(reader, clean_feed_url)
+        autodiscover_links = get_autodiscover_links(reader, clean_feed_url, exception.autodiscover_links)
         if not autodiscover_links:
             raise
 

tests/test_feeds.py

@@ -15,6 +15,7 @@ from unittest.mock import patch
 import pytest
 from reader import EntryNotFoundError
 from reader import Feed
+from reader import FeedExistsError
 from reader import FeedNotFoundError
 from reader import Reader
 from reader import StorageError
@@ -448,6 +449,38 @@ def test_create_feed_falls_back_to_embed_when_global_delivery_mode_is_invalid()
     reader.set_tag.assert_any_call("https://example.com/feed.xml", "should_send_embed", True)
 
 
+def test_create_feed_removes_new_feed_when_initial_update_fails() -> None:
+    feed_url = "https://example.com/not-a-feed"
+    autodiscover_links = [{"href": "https://example.com/feed.xml", "type": "application/rss+xml"}]
+    reader = MagicMock()
+    reader.get_tag.side_effect = lambda resource, key, default=None: {  # noqa: ARG005
+        "webhooks": [{"name": "Main", "url": "https://discord.com/api/webhooks/123/abc"}],
+        ".reader.autodiscover": autodiscover_links,
+    }.get(key, default)
+    reader.update_feed.side_effect = StorageError("invalid feed")
+
+    with pytest.raises(feeds.FeedUpdateError) as exc_info:
+        create_feed(reader, feed_url, "Main")
+
+    reader.delete_feed.assert_called_once_with(feed_url)
+    assert exc_info.value.autodiscover_links == autodiscover_links
+
+
+def test_create_feed_does_not_remove_existing_feed_when_update_fails() -> None:
+    feed_url = "https://example.com/existing-feed.xml"
+    reader = MagicMock()
+    reader.get_tag.side_effect = lambda resource, key, default=None: {  # noqa: ARG005
+        "webhooks": [{"name": "Main", "url": "https://discord.com/api/webhooks/123/abc"}],
+    }.get(key, default)
+    reader.add_feed.side_effect = FeedExistsError(feed_url)
+    reader.update_feed.side_effect = StorageError("temporary failure")
+
+    with pytest.raises(feeds.FeedUpdateError):
+        create_feed(reader, feed_url, "Main")
+
+    reader.delete_feed.assert_not_called()
+
+
 @patch("discord_rss_bot.feeds.capture_full_page_screenshot")
 @patch("discord_rss_bot.feeds.DiscordWebhook")
 def test_create_screenshot_webhook_adds_image_file(

tests/test_main.py

@@ -199,8 +199,6 @@ def test_create_feed_suggests_autodiscovered_links() -> None:
             return [{"name": webhook_name, "url": webhook_url}]
         if resource == () and key == "delivery_mode":
             return "embed"
-        if resource == submitted_url and key == ".reader.autodiscover":
-            return [{"href": discovered_url, "title": "Example feed", "type": "application/rss+xml"}]
         return default
 
     stub_reader.get_tag.side_effect = get_tag
@@ -210,7 +208,17 @@ def test_create_feed_suggests_autodiscovered_links() -> None:
         with patch.object(
             main_module,
             "create_feed",
-            side_effect=feeds.FeedUpdateError(status_code=404, detail="Error updating feed"),
+            side_effect=feeds.FeedUpdateError(
+                status_code=404,
+                detail="Error updating feed",
+                autodiscover_links=[
+                    {
+                        "href": discovered_url,
+                        "title": "Example feed",
+                        "type": "application/rss+xml",
+                    }
+                ],
+            ),
         ):
             response: Response = client.post(
                 url="/add",
@@ -1034,31 +1042,23 @@ def test_change_feed_url_nonexistent_old_url_returns_404() -> None:
 
 def test_change_feed_url_new_url_already_exists_returns_409() -> None:
     """Changing to a URL that is already tracked should return HTTP 409."""
-    second_feed_url = "https://lovinator.space/rss_test_small.xml"
+    second_feed_url = "https://example.com/existing.xml"
 
-    # Ensure both feeds are absent.
-    client.post(url="/remove", data={"feed_url": feed_url})
-    client.post(url="/remove", data={"feed_url": second_feed_url})
+    class StubReader:
+        def change_feed_url(self, _old_url: str, new_url: str) -> None:
+            raise feeds.FeedExistsError(new_url)
 
-    # Ensure webhook exists.
-    client.post(url="/delete_webhook", data={"webhook_url": webhook_url})
-    client.post(url="/add_webhook", data={"webhook_name": webhook_name, "webhook_url": webhook_url})
+    app.dependency_overrides[get_reader_dependency] = StubReader
+    try:
+        response: Response = client.post(
+            url="/change_feed_url",
+            data={"old_feed_url": feed_url, "new_feed_url": second_feed_url},
+        )
+    finally:
+        app.dependency_overrides = {}
 
-    # Add both feeds.
-    client.post(url="/add", data={"feed_url": feed_url, "webhook_dropdown": webhook_name})
-    client.post(url="/add", data={"feed_url": second_feed_url, "webhook_dropdown": webhook_name})
-
-    # Try to rename one to the other.
-    response: Response = client.post(
-        url="/change_feed_url",
-        data={"old_feed_url": feed_url, "new_feed_url": second_feed_url},
-    )
     assert response.status_code == 409, f"Expected 409 when new URL already exists, got {response.status_code}"
 
-    # Cleanup.
-    client.post(url="/remove", data={"feed_url": feed_url})
-    client.post(url="/remove", data={"feed_url": second_feed_url})
-
 
 def test_change_feed_url_same_url_redirects_without_error() -> None:
     """Changing a feed's URL to itself should redirect cleanly without any error."""