Skip to content

Commit 59c6bdb

Browse files
committed
fix(saves): classify a 404 as not_found in copy_save_to_slot
copy_save_to_slot landed on main during PR 1's review; its post-preflight list_saves collapsed every failure onto server_unreachable — the same conflation this branch removes, which its own new gate now flags. The call is rom- AND device-scoped, so a definitive 404 is the #1560 family (dead ROM or dropped device registration): the server answered. Add a sibling except RommNotFoundError before the catch-all, returning the discriminated not_found status and persisting the pre-flight-mutated save_state first, exactly as the server_unreachable branch does — otherwise that work is lost. The catch-all keeps server_unreachable for genuine transport failures. Frontend: add the not_found member to CopySaveToSlotStatus and a case in useCopyToSlot that neither reports the server offline nor claims the game's saves are gone (the 404 can be the device registration). Tests pin the backend branch (incl. that the pre-flight timestamp was persisted) and the copy rule for the new string.
1 parent 3d56e72 commit 59c6bdb

5 files changed

Lines changed: 80 additions & 3 deletions

File tree

py_modules/services/saves/copies.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from domain.save_layout import SAVE_SYNC_CONTENT_DIR_REASON
2121
from domain.save_slot import save_in_slot
2222
from domain.save_status import compute_multi_file_slot
23-
from lib.errors import RommConflictError
23+
from lib.errors import RommConflictError, RommNotFoundError
2424
from services.saves._helpers import local_save_target
2525
from services.saves._settings import resolve_default_slot
2626

@@ -256,7 +256,10 @@ async def copy_save_to_slot(self, rom_id: int, save_id: int, target_slot: str) -
256256
unconditionally, to protect its dirty local before the copy overwrites
257257
it — ``conflict_blocked`` / ``preflight_failed`` on a bad pre-flight.
258258
7. ``list_saves`` with no slot filter (the source may be in any slot);
259-
failure is ``server_unreachable``.
259+
a transport failure is ``server_unreachable``, a definitive 404 (the
260+
server answered that it has no such ROM or device id) is ``not_found``
261+
— distinct so the copy surface never reports an answering server
262+
offline (#1570).
260263
8. The copy proper runs in :meth:`_copy_save_to_slot_io`.
261264
262265
A dedup pre-check between the ``list_saves`` and the copy short-circuits
@@ -267,7 +270,7 @@ async def copy_save_to_slot(self, rom_id: int, save_id: int, target_slot: str) -
267270
Returns a discriminated-status dict (mirrors ``RollbackStatus``):
268271
``ok | already_present{existing_id} | not_configured | invalid_slot_name |
269272
rom_not_installed | version_deleted | unsupported{reason?} |
270-
server_unreachable{message} | conflict_blocked{conflicts} |
273+
server_unreachable{message} | not_found{message} | conflict_blocked{conflicts} |
271274
preflight_failed{errors} | target_slot_busy{message} | copy_failed{message}``.
272275
"""
273276
rom_id = int(rom_id)
@@ -332,6 +335,15 @@ async def copy_save_to_slot(self, rom_id: int, save_id: int, target_slot: str) -
332335
None,
333336
lambda: self._retry.with_retry(lambda: self._romm_api.list_saves(rom_id, device_id=device_id)),
334337
)
338+
except RommNotFoundError as e:
339+
# The server ANSWERED: it has no such ROM (or no such device id).
340+
# ``list_saves`` is rom- AND device-scoped, so this is the #1560
341+
# family. Not a connectivity verdict — the copy surface must not
342+
# report the server offline (#1570).
343+
self._log_debug(f"copy_save_to_slot: server has no such entity: {e}")
344+
# Persist whatever the pre-flight mutated before bailing.
345+
await self._loop.run_in_executor(None, self._write_save_state, rom_id, save_state)
346+
return {"status": "not_found", "message": str(e)}
335347
except Exception as e:
336348
self._log_debug(f"copy_save_to_slot: failed to list saves: {e}")
337349
# Persist whatever the pre-flight mutated before bailing.

src/components/saves/useCopyToSlot.test.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ describe("useCopyToSlot", () => {
136136
{ result: { status: "not_configured" }, needle: "Set up save slots" },
137137
{ result: { status: "invalid_slot_name" }, needle: "valid slot name" },
138138
{ result: { status: "server_unreachable", message: "boom" }, needle: "Couldn't reach RomM" },
139+
{ result: { status: "not_found", message: "HTTP 404" }, needle: "couldn't find this game's save data" },
139140
{ result: { status: "preflight_failed", errors: ["net"] }, needle: "Sync failed before copy" },
140141
{ result: { status: "copy_failed", message: "oops" }, needle: "Couldn't copy the save" },
141142
{ result: { status: "unsupported", reason: "savefiles_in_content_dir" }, needle: "writes saves next to the ROM" },
@@ -151,6 +152,20 @@ describe("useCopyToSlot", () => {
151152
expect(dataChanged).not.toHaveBeenCalled();
152153
});
153154

155+
it("not_found: names what RomM couldn't find, never a reachability or no-saves claim", async () => {
156+
// The 404 means the server ANSWERED (#1560 family). Same rule as every
157+
// other 404 surface in #1570: no offline blame, and no claim the game's
158+
// saves are gone (the 404 can be the device registration).
159+
await runCopy({ status: "not_found", message: "HTTP 404: Not Found" });
160+
161+
const toastCalls = vi.mocked(toaster.toast).mock.calls;
162+
const body = String(toastCalls[toastCalls.length - 1]![0].body ?? "");
163+
expect(body).not.toMatch(/couldn't reach|check your connection|unreachable|not reachable|offline/i);
164+
expect(body).not.toMatch(/no saves|has no save|without saves/i);
165+
// A refusal never refreshes the views.
166+
expect(dataChanged).not.toHaveBeenCalled();
167+
});
168+
154169
it("swallows a rejected copy call and surfaces the generic failure toast", async () => {
155170
vi.mocked(backend.copySaveToSlot).mockRejectedValue(new Error("network down"));
156171
const hook = renderHook(() => useCopyToSlot(42, SLOTS));

src/components/saves/useCopyToSlot.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ async function handleResult(result: CopySaveToSlotStatus, target: string, romId:
6060
case "server_unreachable":
6161
toaster.toast({ title: TITLE, body: "Couldn't reach RomM. Check your connection and try again." });
6262
return;
63+
case "not_found":
64+
// The server answered — it has no such ROM or device id (#1560 family).
65+
// A retry can't help, so the copy must not send the user to check their
66+
// connection, and must not claim the saves are gone: the 404 can be the
67+
// device registration rather than the ROM (#1570).
68+
toaster.toast({ title: TITLE, body: "RomM couldn't find this game's save data — nothing was copied." });
69+
return;
6370
case "version_deleted":
6471
toaster.toast({ title: TITLE, body: "This save no longer exists on the server." });
6572
return;

src/types/saves.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,8 @@ export type CopySaveToSlotStatus =
260260
| { status: "version_deleted" }
261261
| { status: "unsupported"; reason?: string }
262262
| { status: "server_unreachable"; message: string }
263+
// See RollbackStatus — the server answered, it just has no such entry.
264+
| { status: "not_found"; message: string }
263265
| { status: "conflict_blocked"; conflicts: SyncConflict[] }
264266
| { status: "preflight_failed"; errors: string[] }
265267
| { status: "target_slot_busy"; message: string }

tests/services/saves/test_copies.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import pytest
1313

1414
from domain.save_layout import ContentDir
15+
from lib.errors import RommNotFoundError
1516
from tests.services.saves._helpers import (
1617
_create_save,
1718
_enable_sync_with_device,
@@ -387,6 +388,46 @@ def fail_second_list(*args, **kwargs):
387388
assert "unreachable" in result.get("message", "").lower()
388389
assert "error" not in result
389390

391+
@pytest.mark.asyncio
392+
async def test_definitive_404_on_post_preflight_list_is_not_found(self, tmp_path):
393+
"""A 404 on the post-preflight ``list_saves`` → not_found, not offline (#1570).
394+
395+
``list_saves`` is rom- AND device-scoped, so a definitive 404 is the
396+
#1560 family (dead ROM or dropped device registration) — the server
397+
answered. It must read as ``not_found`` rather than ``server_unreachable``,
398+
and — like the transport branch — it must still persist whatever the
399+
pre-flight mutated before bailing, or that work is lost.
400+
"""
401+
svc, fake = make_service(tmp_path)
402+
_create_save(tmp_path)
403+
local_hash = _file_md5(str(tmp_path / "saves" / "gba" / "pokemon.srm"))
404+
_setup_configured(svc, tmp_path, active_slot="autosave", tracked_id=100, last_sync_hash=local_hash)
405+
fake.saves[100] = _tracked_save(100, slot="autosave")
406+
# Seeded state has no sync-check timestamp; the pre-flight sets one.
407+
assert _require_save_state(svc, 42).last_sync_check_at is None
408+
409+
original_list = fake.list_saves
410+
call_count = {"n": 0}
411+
412+
def fail_second_list(*args, **kwargs):
413+
call_count["n"] += 1
414+
if call_count["n"] >= 2:
415+
raise RommNotFoundError("HTTP 404: Not Found")
416+
return original_list(*args, **kwargs)
417+
418+
fake.list_saves = fail_second_list # type: ignore[method-assign]
419+
try:
420+
result = await svc.copy_save_to_slot(42, 100, TARGET)
421+
finally:
422+
fake.list_saves = original_list # type: ignore[method-assign]
423+
424+
assert result["status"] == "not_found"
425+
assert result["status"] != "server_unreachable"
426+
assert "message" in result
427+
# The pre-flight mutation was persisted before bailing (mark_sync_evaluated
428+
# ran, so the seeded-None timestamp is now set on disk).
429+
assert _require_save_state(svc, 42).last_sync_check_at is not None
430+
390431
@pytest.mark.asyncio
391432
async def test_target_slot_busy_on_409(self, tmp_path):
392433
"""The target slot has a newer foreign save this device hasn't synced → 409 → target_slot_busy."""

0 commit comments

Comments
 (0)