fix(saves): re-register the device when RomM no longer has its id#1573
Merged
Conversation
When RomM's database is wiped or restored, the device id the plugin cached no longer exists server-side, so every device-scoped save call 404s. The best-effort update_device touch in ensure_device_registered already makes that 404 the natural, zero-extra-request healing point, but it swallowed the error and trusted the dead id forever. Peel a definitive RommNotFoundError off that touch: on a 404, forget the dead id (clearing the kv_config row) and fall through to the existing registration path so a fresh id is minted and persisted. Every other touch failure stays swallowed best-effort — a transport blip must not throw away a valid id and churn a re-registration. If the re-registration itself fails mid-heal, the method returns the classified failure with an empty id and the dead id left cleared, a clean state the next call retries. Healing fires on every plugin load, so the fix applies itself the moment the release is installed — no user action required. Closes #1560.
…on absence The four sync callers gated the device-registration heal on presence, not liveness: a dead-but-present device id (kv_config still holds it, but the server 404s it after a database wipe/restore) is truthy, so the `if not get_device_id()` guard skipped ensure_device_registered and the run went straight into list_saves with the dead id — "Downloaded 0 saves, 1 error". That is the same presence-not-liveness bug as the update_device touch itself, now one level up in the callers. Extract a shared _ensure_device_live_or_fail helper and call it from all four syncs (pre_launch, post_exit, sync_rom_saves, sync_all_saves). It runs ensure_device_registered UNCONDITIONALLY, so the update_device liveness touch validates and — via the 404 heal — re-registers a dead id BEFORE the sync reads it. The sync then uses the fresh id: _run_rom_sync reads it through _read_sync_inputs, and sync_all_saves re-reads it for the bulk negotiate, both AFTER the heal, so neither carries a stale value. A live id costs one extra 200 touch per sync, the accepted price of correctness. Docs: the troubleshooting note now says a device the server no longer has is re-registered both on plugin load and before every save-sync.
…tration The four sync entry points call _ensure_device_live_or_fail and return its failure verbatim, but nothing pinned what happens when no live registration can be established. Each entry point now has a test that drives the failure through the real DeviceRegistry and asserts both halves: the canonical device_not_registered dict comes back, and the sync itself never reaches the server (no list_saves, negotiate_sync, upload_save or download_save_content). The absence is the point — a sync that ran anyway would attribute every save it moved to a device the server does not have. The registration failure is modelled as a 200 whose body carries no device id, via a new one-shot arm on FakeSaveApi. It leaves the device unregistered without a transport error, so no reachability guard upstream of the device gate can account for the abort. Also covers resolve_core's install gate: an uninstalled ROM resolves no core and never consults the active-core seam.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
PR 2 of #1570. When RomM's database is wiped or restored, the device id the plugin cached no longer
exists on the server. Every device-scoped save call then gets a definitive 404, and the plugin was stuck
reporting the server as offline forever. PR 1 made that message honest (
not_found); this PR makes itrecover:
ensure_device_registerednow detects that its cached device id is dead and re-registers afresh one instead of trusting the stale id blindly.
Closes #1560.
The verified facts this rests on
"Device with ID <uuid> not found"), surfaced asRommNotFoundError. A dead ROM id does not 404 the saves endpoint — it returns200 []. So a device-scoped 404 is unambiguously a dead device, never a dead ROM.ensure_device_registeredalready callsupdate_device(cached_id)as a best-effort touch on every runand swallowed any exception there. That swallowed call is exactly where a dead cached id produces
its 404 — the natural healing point, at zero extra requests.
src/index.tsx, when save-sync is enabled) and before everysave-sync. So installing the release — which reloads the plugin — applies the fix on its own. No manual
step for the user.
The change
ensure_device_registered(py_modules/services/saves/sync_engine/devices.py): the best-effortupdate_devicetouch now peels a definitiveRommNotFoundErrorintoforget_device()+ fall-through tothe existing registration path, minting and persisting a fresh id and returning it (not the stale one).
The discipline that matters: only a
RommNotFoundErrorheals. Every other exception — transport,timeout, 403, generic — stays a best-effort swallow that keeps the cached id, so a server blip can never
throw away a valid registration and churn a re-register. The
except RommNotFoundErroris ordered beforethe broad clause, and the broad clause binds no verdict slug, so the
check_404_not_unreachablegate(from PR 1) stays green. A re-register that itself fails after a forget returns a clean
{success: false, reason, message}with the id left cleared — a retriable state, no half-state.Scope is one service method, its tests, and one docs note. The read callables (already honest via PR 1),
forget_device/register_device, the origin-change trigger, and the frontend are untouched.Docs
docs/user-guide/troubleshooting.md— a device the server no longer has is re-registered automatically onthe next plugin load; the deleted/re-added-ROM re-check (a new rom_id — separate work) stays described as
the user's action.
Verification
mise run gategreen: 6300 backend + 2238 frontend tests, ruff/basedpyright clean,check_404_not_unreachableOK, 31/31 intest_devices.py. Every test asserts persisted kv_config state(id changed on heal / kept on a transport failure / cleared on a failed re-register), not call counts.
On-device (backend path, can't be unit-tested): stage a device id the server no longer has (register,
then remove it server-side), load the plugin or launch a game, and confirm a fresh device appears in
RomM's device list with no offline claim; a genuine transient outage during the touch must NOT
re-register.
Closes part of #1570.
Extension — the heal also runs before every sync
On-device testing found the fix incomplete: the four sync callers gated the heal on presence, not
liveness (
if not self.get_device_id():). A dead-but-present id is truthy, so the heal was skipped and thesync ran straight into the 404 — the same presence-not-liveness bug this PR fixes one level down.
All four sync entry points (pre-launch, post-exit, per-game sync, full sync) now call one shared
_ensure_device_live_or_fail()unconditionally, so the liveness touch validates and heals the id beforethe sync reads it. Verified that no path captures the id before the ensure — every one re-reads it after,
so the sync uses the healed id. A transient (non-404) touch failure on a live id is still swallowed, so the
unconditional touch cannot fail a sync that would previously have succeeded.
Coverage, stated plainly
Heals: plugin load, game launch, game exit, per-game sync, full sync.
Does not heal (reports honestly instead, via the
not_foundslug from PR 1): the save-tab reads andthe slot mutations (switch/delete slot, copy save, version rollback, conflict resolve, setup wizard). Those
run often enough that giving each one an extra device round-trip is not worth it; they surface an honest
message and heal on the next load or sync. Library sync, BIOS downloads and artwork refresh never carry a
device id and are unaffected.