feat(activity): say in plain words what a failed download means and what happens next (#1617)

A failed or cancelled card showed the log line, things like `Peer
transfer failed: Errored` or a 500 with a URL in it, so a red card meant
going to the logs to find out whether anything was coming next. They now
say what happened and what happens next.

For a download that belongs to a request, the card says another copy
will be tried in about N minutes when the quick retry from #1535 is on,
and points at Retry or reopening the request when it isn't. A manual
download is never promised an automatic retry. The Soulseek cases each
get their own sentence: the peer dropped the transfer, the peer could
not be reached, slskd refused to queue it, and the peer never answered.
The "tried twice" wording leans on the retry from #1590.

The saved message is left alone because auto-download reads it to tell a
failed release from an interrupted one. Only what the activity list
shows is reworded, on a copy, and anything the new code doesn't
recognise comes back as it was.

It matches on how the message starts, so it is only as current as the
wording the rest of the code uses. A message that gets reworded
elsewhere falls back to the original line instead of breaking, which
seemed fine for something that only changes the display, but it is the
part I would look at first.

It has been running on my instance since mid-October. Ten new tests
cover each message and the next step, and the activity route tests still
pass. Ruff and basedpyright are clean on the changed files.

shelfmark/core/activity_routes.py

@@ -31,6 +31,7 @@ from shelfmark.core.request_helpers import (
     populate_request_usernames,
 )
 from shelfmark.core.request_validation import RequestStatus
+from shelfmark.core.status_messages import explain_buckets, explain_request_rows
 from shelfmark.core.viewer_redaction import (
     is_public_download_id,
     match_public_download_id,
@@ -646,6 +647,8 @@ def register_activity_routes(
             user_id=actor.owner_scope,
         )
         emit_request_updates(updated_requests)
+        # Plain wording for what the list shows, after the sync above has used the real text.
+        explain_buckets(status)
         request_rows = _list_visible_requests(
             user_db,
             is_admin=actor.is_admin,
@@ -659,6 +662,7 @@ def register_activity_routes(
             if f"request:{request_id}" in hidden_item_keys:
                 continue
             visible_request_rows.append(row)
+        visible_request_rows = explain_request_rows(visible_request_rows)
 
         if not actor.is_admin:
             # Redacted only now: the delivery-state sync above matches on real task ids.

shelfmark/core/status_messages.py

@@ -0,0 +1,115 @@
+"""Plain wording for a download that failed or stopped: what happened and what comes next.
+
+The saved message is a log line ("Peer transfer failed: Errored"). Auto-download reads it to
+tell a failed release from an interrupted one, so it is never rewritten; only what the activity
+list shows is.
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+from shelfmark.core.config import config as app_config
+from shelfmark.core.request_helpers import coerce_int
+
+FAILURE_RETRY_MINUTES_SETTING = "AUTO_DOWNLOAD_FAILURE_RETRY_MINUTES"
+
+_SHOWN_FOR = frozenset({"error", "cancelled"})
+
+
+def _next_step(*, request_linked: bool, retry_minutes: int) -> str:
+    if request_linked and retry_minutes > 0:
+        return f" Another copy will be tried in about {retry_minutes} minutes."
+    if request_linked:
+        return " Use Retry to try again, or reopen the request to try another copy."
+    return " Use Retry to try again, or pick another copy."
+
+
+def explain(
+    message: str | None,
+    *,
+    request_linked: bool,
+    retry_minutes: int | None = None,
+) -> str | None:
+    """The message in plain words with the next step, or it unchanged when nothing is known."""
+    if not isinstance(message, str) or not message.strip():
+        return message
+    text = message.strip()
+    lowered = text.lower()
+    minutes = (
+        retry_minutes
+        if retry_minutes is not None
+        else coerce_int(app_config.get(FAILURE_RETRY_MINUTES_SETTING, 0), 0)
+    )
+    nxt = _next_step(request_linked=request_linked, retry_minutes=minutes)
+
+    if lowered.startswith("superseded"):
+        return "This book was already delivered by another download."
+    if lowered.startswith("interrupted"):
+        return "Interrupted by a restart. Use Retry to run it again."
+    if lowered.startswith("peer transfer failed"):
+        detail = text.split(":", 1)[1].strip() if ":" in text else ""
+        suffix = f" ({detail})" if detail else ""
+        return f"The Soulseek peer dropped the transfer{suffix}." + nxt
+    if lowered.startswith("failed to add to slskd"):
+        if "direct or indirect" in lowered:
+            return (
+                "Could not reach the Soulseek peer, directly or through the Soulseek server "
+                "(tried twice)." + nxt
+            )
+        if '("' in text and "timed out" not in lowered:
+            # slskd gave a reason, and it is not a timeout.
+            return "slskd would not queue the download (tried twice)." + nxt
+        # A timeout, or the bare 500 an older attempt saved before slskd's reason was kept; every
+        # one of those that slskd's own log explained was a peer that did not answer in time.
+        return "The Soulseek peer did not answer slskd's queue request (tried twice)." + nxt
+    if lowered.startswith("download failed:"):
+        detail = text.split(":", 1)[1].strip()
+        return f"The download hit an internal error ({detail})." + nxt
+    if lowered.startswith("retrying after"):
+        return "Retrying automatically after a failed download."
+    if lowered.startswith("download stalled"):
+        return "Nothing was downloading, so it was stopped." + nxt
+    if lowered.startswith("no seeders"):
+        return text.rstrip(".") + "; the torrent was removed." + nxt
+    if lowered.startswith("all download sources failed"):
+        return "None of the download sources could provide this book." + nxt
+    return text
+
+
+def explain_request_rows(rows: list[dict[str, Any]]) -> list[dict[str, Any]]:
+    """The request rows with ``last_failure_reason`` in plain words (copies; the stored text stays)."""
+    minutes = coerce_int(app_config.get(FAILURE_RETRY_MINUTES_SETTING, 0), 0)
+    explained: list[dict[str, Any]] = []
+    for row in rows:
+        reason = row.get("last_failure_reason")
+        if not isinstance(reason, str) or not reason.strip():
+            explained.append(row)
+            continue
+        # A request that was reopened after a restart is searched again by the next pass.
+        text = (
+            "Interrupted by a restart; it will be tried again."
+            if reason.strip().lower().startswith("interrupted by a restart")
+            else explain(reason, request_linked=True, retry_minutes=minutes)
+        )
+        explained.append({**row, "last_failure_reason": text})
+    return explained
+
+
+def explain_buckets(status: dict[str, dict[str, Any]]) -> None:
+    """Rewrite the messages of the error and cancelled entries of a status dict, in place."""
+    minutes = coerce_int(app_config.get(FAILURE_RETRY_MINUTES_SETTING, 0), 0)
+    for bucket in _SHOWN_FOR:
+        entries = status.get(bucket, {})
+        for task_id, payload in list(entries.items()):
+            if not isinstance(payload, dict):
+                continue
+            # A copy, so a payload the queue hands out again is never explained twice.
+            entries[task_id] = {
+                **payload,
+                "status_message": explain(
+                    payload.get("status_message"),
+                    request_linked=bool(payload.get("request_id")),
+                    retry_minutes=minutes,
+                ),
+            }

tests/core/test_activity_routes_api.py

@@ -1404,7 +1404,8 @@ class TestActivityRoutes:
         assert response.status_code == 200
         assert "stale-active-task" in response.json["status"]["error"]
         assert (
-            response.json["status"]["error"]["stale-active-task"]["status_message"] == "Interrupted"
+            response.json["status"]["error"]["stale-active-task"]["status_message"]
+            == "Interrupted by a restart. Use Retry to run it again."
         )
 
     def test_snapshot_preserves_retry_for_stale_active_requested_download(
@@ -1453,7 +1454,9 @@ class TestActivityRoutes:
 
         assert response.status_code == 200
         viewer_id = public_download_id(task_id)
-        assert response.json["status"]["error"][viewer_id]["status_message"] == "Interrupted"
+        assert response.json["status"]["error"][viewer_id]["status_message"].startswith(
+            "Interrupted by a restart"
+        )
         assert response.json["status"]["error"][viewer_id]["retry_available"] is True
 
     def test_snapshot_includes_retry_available_for_live_terminal_downloads(

tests/core/test_status_messages.py

@@ -0,0 +1,144 @@
+"""What the activity list says about a download that failed, and what happens next."""
+
+from __future__ import annotations
+
+import pytest
+
+from shelfmark.core.status_messages import explain, explain_buckets
+
+
+def _explain(message, *, linked=True, minutes=20):
+    return explain(message, request_linked=linked, retry_minutes=minutes)
+
+
+@pytest.mark.parametrize(
+    ("raw", "expected"),
+    [
+        (
+            "Peer transfer failed: Errored",
+            "The Soulseek peer dropped the transfer (Errored). "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            "Failed to add to slskd: 500 Server Error: Internal Server Error for url: http://x",
+            "The Soulseek peer did not answer slskd's queue request (tried twice). "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            "Download stalled (no activity for 300s)",
+            "Nothing was downloading, so it was stopped. "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            "No seeders: 0 found, 1 needed, for 5 min (12% downloaded)",
+            "No seeders: 0 found, 1 needed, for 5 min (12% downloaded); the torrent was removed. "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            "All download sources failed",
+            "None of the download sources could provide this book. "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+    ],
+)
+def test_a_failure_says_what_happened_and_what_comes_next(raw, expected):
+    assert _explain(raw) == expected
+
+
+def test_interrupted_says_a_restart_did_it():
+    assert _explain("Interrupted") == "Interrupted by a restart. Use Retry to run it again."
+
+
+def test_a_superseded_row_says_the_book_was_delivered():
+    assert (
+        _explain("Superseded by a completed download")
+        == "This book was already delivered by another download."
+    )
+
+
+def test_without_the_quick_retry_a_request_is_pointed_at_retry_or_reopen():
+    assert _explain("Peer transfer failed: TimedOut", minutes=0).endswith(
+        "Use Retry to try again, or reopen the request to try another copy."
+    )
+
+
+def test_a_manual_download_is_never_promised_an_automatic_retry():
+    text = _explain("Peer transfer failed: Errored", linked=False)
+
+    assert "will be tried" not in text
+    assert text.endswith("Use Retry to try again, or pick another copy.")
+
+
+def test_an_unknown_message_and_an_empty_one_are_left_as_they_are():
+    assert _explain("Something new broke") == "Something new broke"
+    assert _explain(None) is None
+    assert _explain("  ") == "  "
+
+
+def test_the_buckets_are_rewritten_without_touching_the_original_payload():
+    original = {"id": "t", "request_id": 5, "status_message": "Peer transfer failed: Errored"}
+    status = {"error": {"t": original}, "cancelled": {}, "complete": {"c": {"status_message": "x"}}}
+
+    explain_buckets(status)
+
+    assert status["error"]["t"]["status_message"].startswith("The Soulseek peer dropped")
+    assert original["status_message"] == "Peer transfer failed: Errored"
+    assert status["complete"]["c"]["status_message"] == "x"
+
+
+@pytest.mark.parametrize(
+    ("raw", "expected"),
+    [
+        (
+            'Failed to add to slskd: 500 Server Error: Internal Server Error for url: http://x/batches ("Failed to connect to user u: Failed to establish a direct or indirect message connection to u (1.2.3.4:5)")',
+            "Could not reach the Soulseek peer, directly or through the Soulseek server (tried twice). "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            'Failed to add to slskd: 500 Server Error for url: http://x/batches ("The wait timed out after 5000 milliseconds")',
+            "The Soulseek peer did not answer slskd's queue request (tried twice). "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            'Failed to add to slskd: 500 Server Error for url: http://x/batches ("The peer is banned")',
+            "slskd would not queue the download (tried twice). "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            "Download failed: TypeError",
+            "The download hit an internal error (TypeError). "
+            "Another copy will be tried in about 20 minutes.",
+        ),
+        (
+            "Retrying after a 20-minute cooldown (failed; delivery was queued)",
+            "Retrying automatically after a failed download.",
+        ),
+    ],
+)
+def test_slskd_and_internal_failures_say_what_went_wrong(raw, expected):
+    assert _explain(raw) == expected
+
+
+def test_a_request_reopened_after_a_restart_says_it_will_be_tried_again():
+    from shelfmark.core.status_messages import explain_request_rows
+
+    out = explain_request_rows([{"id": 1, "last_failure_reason": "Interrupted by a restart"}])
+
+    assert out[0]["last_failure_reason"] == "Interrupted by a restart; it will be tried again."
+
+
+def test_request_failure_reasons_are_explained_on_copies():
+    from shelfmark.core.status_messages import explain_request_rows
+
+    rows = [
+        {"id": 1, "last_failure_reason": "Download failed: TypeError"},
+        {"id": 2, "last_failure_reason": None},
+        {"id": 3},
+    ]
+
+    out = explain_request_rows(rows)
+
+    assert out[0]["last_failure_reason"].startswith("The download hit an internal error")
+    assert out[1]["last_failure_reason"] is None
+    assert "last_failure_reason" not in out[2]
+    assert rows[0]["last_failure_reason"] == "Download failed: TypeError"  # untouched