fix(errors): report a definitive 404 as not_found, not server_unreachable

RomM answers a request for an entity it no longer has with HTTP 404. Every
exception-type-blind service site collapsed that onto server_unreachable, so
the UI claimed "RomM offline" while the server was plainly answering.

Route those sites through classify_error, which already maps
RommNotFoundError to NOT_FOUND. Sites whose verdict is a partial-success
flag rather than a reason slug peel the 404 off with a sibling
except RommNotFoundError instead:

- get_version_list returns the local-only list with server_query_failed
  False on a 404, so the picker stops feeding the global connection store.
- get_save_status keeps server_query_failed True (the flag is what stops
  the matrix classifying local files as ready-to-upload against an empty
  server list) and gains the additive server_query_reason slug, so only an
  explicit server_unreachable drives the offline state.
- list_file_versions and rollback_to_version gain a not_found arm,
  distinct from server_unreachable (retryable) and version_deleted (one
  save missing from a ROM the server still has).

classify_error gains a narrow ConnectionError/TimeoutError/socket.gaierror
branch so a raw socket failure keeps the unreachable slug. Deliberately not
bare OSError: that also covers local disk I/O, and reporting a failed
settings write as "server unreachable" is the same lie pointing the other
way.

Messages that asserted reachability are replaced ("RomM server not
reachable.", "Cannot inspect slot - server unreachable"); messages already
true for both branches are kept, since only the routing slug was wrong.

scripts/check_404_not_unreachable.py locks this in: a catch-all handler in
services/ may not bind a verdict key (reason/status/recommended_action) to a
hardcoded SERVER_UNREACHABLE. The rule is positional rather than "does the
handler call classify_error", so it also catches a handler that consults the
funnel and then discards its verdict. It finds 14 sites against the previous
tree. steamgrid.py is exempt with a recorded reason: it talks to
SteamGridDB, whose failures classify_error has no branch for.

Author: danielcopper

py_modules/lib/errors.py

@@ -1,5 +1,6 @@
 """RomM API error types for structured error handling."""
 
+import socket
 from typing import Any
 
 from lib.list_result import ErrorCode
@@ -170,8 +171,8 @@ def classify_error(exc):
 
     ``reason`` is a canonical :class:`lib.list_result.ErrorCode` slug
     (returned as its string value). Several exception types fold onto one
-    slug \u2014 connection/timeout/SSL/5xx/generic-API all map to
-    ``server_unreachable``; 401 and 403 both map to ``auth_failed`` \u2014 but
+    slug \u2014 connection/timeout/SSL/5xx/generic-API plus raw socket errors
+    all map to ``server_unreachable``; 401/403 map to ``auth_failed`` \u2014 but
     each branch keeps a distinct human ``message``. In particular the 403
     branch stays distinguishable from the 401 branch: a Cloudflare
     bot-fight 403 at the tunnel edge is not a wrong-credentials failure, so
@@ -212,6 +213,14 @@ def classify_error(exc):
         )
     if isinstance(exc, RommApiError):
         return ErrorCode.SERVER_UNREACHABLE.value, str(exc)
+    if isinstance(exc, (ConnectionError, TimeoutError, socket.gaierror)):
+        # A socket-level failure that never passed through the adapter's
+        # ``translate_http_error`` (a caller reaching the network outside the
+        # RomM client). Same reachability verdict as ``RommConnectionError``.
+        # Deliberately NOT bare ``OSError``: that also covers local disk I/O,
+        # so a failed settings write would report the server as unreachable —
+        # the same class of lie as reporting a 404 that way.
+        return ErrorCode.SERVER_UNREACHABLE.value, f"Server unreachable — {exc}"
     return ErrorCode.UNKNOWN.value, str(exc)
 
 

py_modules/services/achievements.py

@@ -14,7 +14,7 @@
 from typing import TYPE_CHECKING, Any
 
 from domain.achievements import extract_achievements_from_rom, extract_game_progress
-from lib.list_result import ErrorCode
+from lib.errors import classify_error
 
 if TYPE_CHECKING:
     import asyncio
@@ -159,9 +159,10 @@ async def get_achievements(self, rom_id):
                     "total": len(stale["achievements"]),
                     "stale": True,
                 }
+            reason, _message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE.value,
+                "reason": reason,
                 "message": str(e),
                 "achievements": [],
                 "total": 0,
@@ -219,9 +220,10 @@ async def get_achievement_progress(self, rom_id):
             stale_progress = self._achievements_cache.get(rom_id_str, {}).get("user_progress")
             if stale_progress:
                 return {**self._progress_data_response(stale_progress), "stale": True}
+            reason, _message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE.value,
+                "reason": reason,
                 "message": str(e),
                 "earned": 0,
                 "total": 0,

py_modules/services/artwork.py

@@ -56,7 +56,7 @@
 )
 from domain.cover_refresh import cover_ts_only_change, scan_cover_refresh_candidates
 from domain.sync_stage import SyncStage
-from lib.errors import RommNotFoundError
+from lib.errors import RommNotFoundError, classify_error
 from lib.list_result import ErrorCode
 
 
@@ -823,9 +823,13 @@ async def refresh_cover(self, rom_id: int) -> dict[str, Any]:
             rom = await self._loop.run_in_executor(None, self._romm_api.get_rom, rom_id)
         except Exception as e:
             self._logger.warning(f"refresh_cover: failed to fetch rom {rom_id}: {e}")
+            reason, _message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE.value,
+                "reason": reason,
+                # Already true for both branches (the ROM is gone / the server
+                # is down), and more specific than the classifier's generic
+                # string — only the routing slug was ever wrong here.
                 "message": "Could not fetch ROM from server",
             }
         if not rom:

py_modules/services/saves/slots/deletion.py

@@ -13,7 +13,7 @@
 from typing import TYPE_CHECKING, Any
 
 from domain.save_slot import save_in_slot, slot_query_param
-from lib.list_result import ErrorCode
+from lib.errors import classify_error
 from services.saves._settings import save_sync_enabled
 
 if TYPE_CHECKING:
@@ -128,10 +128,11 @@ async def get_slot_delete_info(self, rom_id: int, slot: str) -> dict[str, Any]:
                 self._logger.warning(
                     f"get_slot_delete_info: failed to list saves for slot '{slot}': {e}",
                 )
+                reason, message = classify_error(e)
                 return {
                     "success": False,
-                    "reason": ErrorCode.SERVER_UNREACHABLE.value,
-                    "message": "Cannot inspect slot — server unreachable",
+                    "reason": reason,
+                    "message": f"Cannot inspect slot — {message}",
                 }
 
         # Local tracked files pointing to server saves in this slot
@@ -178,9 +179,10 @@ async def _delete_server_slot_saves(self, rom_id: int, slot: str) -> dict[str, A
             return {"success": True, "count": len(save_ids), "ids": set(save_ids)}
         except Exception as e:
             self._logger.warning(f"delete_slot: server delete failed for slot '{slot}': {e}")
+            reason, _message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE.value,
+                "reason": reason,
                 "message": f"Failed to delete server saves: {e}",
             }
 

py_modules/services/saves/slots/listing.py

@@ -12,7 +12,7 @@
 
 from domain.rom_save_sync_state import RomSaveSyncState
 from domain.save_slot import save_in_slot, slot_query_param
-from lib.list_result import ErrorCode
+from lib.errors import classify_error
 from services.saves._messages import SAVE_SYNC_DISABLED
 from services.saves._settings import resolve_default_slot, save_sync_enabled
 
@@ -98,9 +98,12 @@ async def get_save_slots(self, rom_id: int) -> dict[str, Any]:
             )
         except Exception as e:
             self._log_debug(f"Failed to fetch save slots for rom {rom_id}: {e}")
+            reason, _message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE,
+                "reason": reason,
+                # The raw exception text is neutral and carries the concrete
+                # detail; only the routing slug was ever wrong here.
                 "message": str(e),
                 "slots": [],
                 "active_slot": active_slot,
@@ -212,9 +215,10 @@ async def get_slot_saves(self, rom_id: int, slot: str) -> dict[str, Any]:
             ]
             return {"success": True, "slot": slot, "saves": saves}
         except Exception as e:
+            reason, _message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE,
+                "reason": reason,
                 "message": str(e),
                 "slot": slot,
                 "saves": [],

py_modules/services/saves/slots/switching.py

@@ -15,7 +15,7 @@
 from domain.rom_save_sync_state import RomSaveSyncState
 from domain.save_layout import SAVE_SYNC_CONTENT_DIR_REASON
 from domain.save_slot import save_in_slot
-from lib.list_result import ErrorCode
+from lib.errors import classify_error
 from services.saves._helpers import newest_server_saves_by_target
 from services.saves._messages import SAVE_SYNC_IN_CONTENT_DIR
 from services.saves._settings import resolve_default_slot, save_sync_enabled
@@ -237,9 +237,10 @@ async def switch_slot(self, rom_id: int, new_slot: str) -> dict[str, Any]:
                     ),
                 )
             except Exception as e:
+                reason, _message = classify_error(e)
                 return {
                     "success": False,
-                    "reason": ErrorCode.SERVER_UNREACHABLE.value,
+                    "reason": reason,
                     "message": str(e),
                 }
 

py_modules/services/saves/status/service.py

@@ -15,6 +15,7 @@
     status_from_action,
 )
 from domain.sync_action import Conflict, Skip
+from lib.errors import classify_error
 from services.saves._settings import save_sync_enabled
 
 if TYPE_CHECKING:
@@ -170,6 +171,7 @@ def _get_save_status_io(
         server_saves: list[dict[str, Any]],
         *,
         server_query_failed: bool = False,
+        server_query_reason: str | None = None,
     ) -> dict[str, Any]:
         """Sync helper for get_save_status — runs in executor.
 
@@ -309,6 +311,15 @@ def _get_save_status_io(
             "savefiles_in_content_dir": savefiles_in_content_dir,
             "save_sync_display": save_sync_display,
             "server_query_failed": server_query_failed,
+            # Why the query failed, as a :func:`classify_error` slug (None when
+            # it didn't). Additive alongside the flag, per CLAUDE.md's
+            # partial-success carve-out — do NOT collapse this into the
+            # canonical ``{success, reason, message}``: the response is a full
+            # payload the UI renders, and the binary ``success`` would erase
+            # that. The flag stays the "we lack the server's view" signal that
+            # drives ``status="unknown"``; this slug only says why, so a
+            # definitive 404 stops being reported as an offline server (#1570).
+            "server_query_reason": server_query_reason,
             # Interim #908 guard: a slot whose current save spans >1 distinct
             # file (e.g. Saturn .bkr/.bcr/.smpc) is an N-file *set*, not a
             # single file with a version history. Per-version rollback would
@@ -337,6 +348,13 @@ async def get_save_status(self, rom_id: int) -> dict[str, Any]:
         the matrix-derived "ready to upload" label that an empty server
         list would otherwise produce — see ``_get_save_status_io``.
 
+        ``server_query_reason`` carries the :func:`classify_error` slug for
+        that failure (``None`` on success). The flag and the slug answer
+        different questions: the flag says the server's view is unknown —
+        true for a 404 as much as for an outage, and what keeps the matrix
+        from acting on an empty list — while the slug says *why*, so only an
+        explicit ``server_unreachable`` drives the UI's offline state (#1570).
+
         The additive ``savefiles_in_content_dir: bool`` flag is ``True``
         when RetroArch writes saves next to the ROM (the unsupported case):
         local probing is skipped and the display reads "Save sync off",
@@ -353,6 +371,7 @@ async def get_save_status(self, rom_id: int) -> dict[str, Any]:
 
         server_saves: list[dict[str, Any]] = []
         server_query_failed = False
+        server_query_reason: str | None = None
         try:
             device_id = await self._loop.run_in_executor(None, self._device_registry.get_device_id)
             server_saves = await self._loop.run_in_executor(
@@ -362,6 +381,7 @@ async def get_save_status(self, rom_id: int) -> dict[str, Any]:
         except Exception as e:
             self._log_debug(f"Failed to fetch saves for rom {rom_id}: {e}")
             server_query_failed = True
+            server_query_reason, _message = classify_error(e)
 
         async with self._sync_engine.rom_lock(rom_id):
             return await self._loop.run_in_executor(
@@ -370,6 +390,7 @@ async def get_save_status(self, rom_id: int) -> dict[str, Any]:
                     rom_id,
                     server_saves,
                     server_query_failed=server_query_failed,
+                    server_query_reason=server_query_reason,
                 ),
             )
 

py_modules/services/saves/sync_engine/devices.py

@@ -321,9 +321,12 @@ async def list_devices(self, *, loop: asyncio.AbstractEventLoop) -> dict[str, An
             return {"success": True, "devices": enriched}
         except Exception as e:
             self._log_debug(f"list_devices failed: {e}")
+            reason, _message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE.value,
+                "reason": reason,
+                # Neutral and true whichever way the call failed — only the
+                # routing slug was ever wrong here.
                 "message": "Could not load devices",
                 "devices": [],
             }

py_modules/services/saves/sync_engine/rollback.py

@@ -142,11 +142,11 @@ async def resolve(
                 ),
             )
         except Exception as e:
-            _code, _msg = classify_error(e)
+            reason, message = classify_error(e)
             return {
                 "success": False,
-                "reason": ErrorCode.SERVER_UNREACHABLE.value,
-                "message": f"Failed to fetch saves: {_msg}",
+                "reason": reason,
+                "message": f"Failed to fetch saves: {message}",
             }
 
         active_slot = save_state.active_slot

py_modules/services/saves/versions.py

@@ -20,6 +20,7 @@
 from domain.save_layout import SAVE_SYNC_CONTENT_DIR_REASON
 from domain.save_slot import save_in_slot, slot_query_param
 from domain.save_status import compute_multi_file_slot
+from lib.errors import RommNotFoundError
 from services.saves._helpers import local_save_target
 from services.saves._settings import resolve_default_slot
 
@@ -143,6 +144,11 @@ async def list_file_versions(self, rom_id: int, slot: str, filename: str) -> dic
           ``list_saves`` call failed (network, server, auth, etc.). The
           frontend distinguishes this from an empty list so it can show a
           retry affordance instead of "no versions available".
+        - ``{"status": "not_found", "message": ...}`` if the ``list_saves``
+          call drew a definitive 404 — RomM no longer has this ROM or the
+          registered device id. Distinct from ``server_unreachable``: the
+          server answered, so retrying is pointless and the panel says so
+          instead of blaming the connection (#1570).
         """
         rom_id = int(rom_id)
         save_state, device_id = await self._loop.run_in_executor(None, self._read_inputs, rom_id)
@@ -159,6 +165,11 @@ async def list_file_versions(self, rom_id: int, slot: str, filename: str) -> dic
                     lambda: self._romm_api.list_saves(rom_id, device_id=device_id, slot=slot_query_param(slot))
                 ),
             )
+        except RommNotFoundError as e:
+            # The server answered: it has no such ROM (or no such device id). A
+            # retry cannot change that, so the panel must not offer one (#1570).
+            self._log_debug(f"list_file_versions: server has no such entity: {e}")
+            return {"status": "not_found", "message": str(e)}
         except Exception as e:
             self._log_debug(f"list_file_versions: failed to list saves: {e}")
             return {"status": "server_unreachable", "message": str(e)}
@@ -318,6 +329,11 @@ async def rollback_to_version(self, rom_id: int, slot: str, save_id: int) -> dic
           auth, etc.). The frontend distinguishes this from
           ``version_deleted`` so it can show a retry affordance instead
           of "version no longer on the server".
+        - ``{"status": "not_found", "message": ...}`` if that same call drew
+          a definitive 404 — RomM no longer has this ROM or the registered
+          device id. Distinct from both siblings: ``version_deleted`` is one
+          missing save inside a ROM the server still has, and
+          ``server_unreachable`` is a connection the user can retry (#1570).
         - ``{"status": "conflict_blocked", "conflicts": [...]}`` if the
           pre-flight surfaced a conflict on the currently-tracked save.
           The frontend resolves it via the standard conflict modal.
@@ -387,6 +403,13 @@ async def rollback_to_version(self, rom_id: int, slot: str, save_id: int) -> dic
                         lambda: self._romm_api.list_saves(rom_id, device_id=device_id, slot=slot_query_param(slot))
                     ),
                 )
+            except RommNotFoundError as e:
+                # The server answered: it has no such ROM (or no such device id).
+                # Not a connectivity verdict, so the toast must not blame the
+                # connection or invite a retry that cannot succeed (#1570).
+                self._log_debug(f"rollback_to_version: server has no such entity: {e}")
+                await self._loop.run_in_executor(None, self._write_save_state, rom_id, save_state)
+                return {"status": "not_found", "message": str(e)}
             except Exception as e:
                 self._log_debug(f"rollback_to_version: failed to list saves: {e}")
                 # Persist whatever the pre-flight mutated before bailing.