snapshots: never of a save a sync is writing, never incomplete

cmd/opensave-app/frontend/src/lib/notifications.js

@@ -142,6 +142,16 @@ export function arrivalNote(ev, games) {
   return { title: game.name, body: `Got ${plural(ev.files || 1, 'file')} from ${ev.device}`, game };
 }
 
+/** A game whose automatic snapshots keep missing files it holds open
+ *  (daemon/autosnapshot.go), in the words of the catalog: `t` is the
+ *  translate function, `$t` in a component. */
+export function incompleteSnapshotsMessage(data, t) {
+  const names = data?.files ?? [];
+  const more = Math.max((data?.count ?? names.length) - names.length, 0);
+  const files = names.join(', ') + (more ? t('snapshots.incomplete.more', { n: more }) : '');
+  return t('snapshots.incomplete', { name: data?.gameName ?? '', files });
+}
+
 /** New games found, as the desktop shows them. */
 export function newGamesNote(found) {
   if (!found?.length) return null;

cmd/opensave-app/frontend/src/lib/notifications.test.js

@@ -1,5 +1,7 @@
 import { describe, expect, it } from 'vitest';
-import { arrivalMessage, badgeCount, happened, loadSeenAt, saveSeenAt, waitingOnYou, ARRIVAL_QUIET_MS } from './notifications.js';
+import { arrivalMessage, badgeCount, happened, incompleteSnapshotsMessage, loadSeenAt, saveSeenAt, waitingOnYou, ARRIVAL_QUIET_MS } from './notifications.js';
+import { translate } from './i18n.js';
+import en from '../locales/en.json';
 
 const games = {
   hades: { id: 'hades', name: 'Hades', branches: { main: { snapshots: [{ id: 's1', problem: 'damaged' }] } } },
@@ -71,3 +73,20 @@ describe('a missing save folder found on another drive', () => {
     expect(r?.go).toEqual({ view: 'game', params: { gameId: 'ark' } });
   });
 });
+
+describe('incompleteSnapshotsMessage', () => {
+  const t = (key, params) => translate(en, key, params, new Intl.PluralRules('en'));
+
+  it('names the game and the files it holds open', () => {
+    expect(incompleteSnapshotsMessage({ gameName: 'Hades', files: ['save.lock'], count: 1 }, t)).toBe(
+      '"Hades": automatic snapshots keep missing save.lock, which the game holds open. Until those can be read, usually once the game is closed, its changes are not backed up or synced.'
+    );
+  });
+
+  it('says how many more there are than it names', () => {
+    const one = incompleteSnapshotsMessage({ gameName: 'Hades', files: ['a', 'b', 'c'], count: 4 }, t);
+    expect(one).toContain('missing a, b, c and one more file, which');
+    const more = incompleteSnapshotsMessage({ gameName: 'Hades', files: ['a', 'b', 'c'], count: 9 }, t);
+    expect(more).toContain('missing a, b, c and 6 more files, which');
+  });
+});

cmd/opensave-app/frontend/src/lib/stores.js

@@ -1,8 +1,9 @@
 // Central app state, fed by the daemon's init dump + live WS updates.
 import { writable, derived, get } from 'svelte/store';
 import { notifyPrefs } from './notifyprefs.js';
-import { arrivalMessage, arrivalNote, newGamesNote } from './notifications.js';
+import { arrivalMessage, arrivalNote, incompleteSnapshotsMessage, newGamesNote } from './notifications.js';
 import { toDesktop } from './notify.js';
+import { t } from './i18n.js';
 
 // When each game's arrival was last said (lib/notifications.js).
 const arrivalShown = {};
@@ -186,6 +187,12 @@ export function applyMessage(msg) {
     case 'cloud-offers':
       cloudOffers.set(data ?? []);
       break;
+    case 'snapshots-incomplete':
+      // Sent once per run of discarded automatic snapshots, so no rate limit.
+      toast(incompleteSnapshotsMessage(data, get(t)), 'error', {
+        action: { label: get(t)('common.open'), run: () => navigate('game', { gameId: data.gameId }) }
+      });
+      break;
     case 'cloud-pulled':
       // Taken without asking, because it carried on from the save this
       // device had and this device had not changed since. Said out loud all

cmd/opensave-app/frontend/src/locales/en.json

@@ -2,6 +2,7 @@
   "settings.language": "Language",
   "settings.systemDefault": "System default",
   "common.cancel": "Cancel",
+  "common.open": "Open",
   "common.nowTracking": "Now tracking \"{name}\"",
   "home.addGame.adding": "Adding…",
   "home.addGame.changeFolder": "Change folder…",
@@ -222,5 +223,10 @@
   "restoreProgress.aboutMinutes": {
     "one": "about a minute",
     "other": "about {n} minutes"
+  },
+  "snapshots.incomplete": "\"{name}\": automatic snapshots keep missing {files}, which the game holds open. Until those can be read, usually once the game is closed, its changes are not backed up or synced.",
+  "snapshots.incomplete.more": {
+    "one": " and one more file",
+    "other": " and {n} more files"
   }
 }

cmd/opensave-app/frontend/src/locales/zh-CN.json

@@ -2,6 +2,7 @@
   "settings.language": "语言",
   "settings.systemDefault": "跟随系统",
   "common.cancel": "取消",
+  "common.open": "打开",
   "common.nowTracking": "已开始跟踪“{name}”",
   "home.addGame.adding": "添加中…",
   "home.addGame.changeFolder": "更换文件夹…",
@@ -198,5 +199,7 @@
   "restoreProgress.line": "{phase} — {percent}%（{done}/{total}）",
   "restoreProgress.lineWithTimeLeft": "{phase} — {percent}%（{done}/{total}），还剩{left}",
   "restoreProgress.underAMinute": "不到一分钟",
-  "restoreProgress.aboutMinutes": "约 {n} 分钟"
+  "restoreProgress.aboutMinutes": "约 {n} 分钟",
+  "snapshots.incomplete": "“{name}”：自动快照一直缺少游戏正在占用的 {files}。在这些文件可以读取之前（通常是游戏关闭后），它的更改不会被备份或同步。",
+  "snapshots.incomplete.more": " 等另外 {n} 个文件"
 }

internal/api/server.go

@@ -70,6 +70,9 @@ func New(d *daemon.Daemon) *Server {
 	// Newly installed games the background scan found.
 	d.OnNewGames = func(games []daemon.NewGame) { s.Hub.Broadcast("new-games", games) }
 
+	// A game whose automatic snapshots keep missing files it holds open.
+	d.OnSnapshotsIncomplete = func(p daemon.SnapshotsIncomplete) { s.Hub.Broadcast("snapshots-incomplete", p) }
+
 	// Syncing paused or resumed — from here, the CLI, the tray, or a timer.
 	d.P2P.Pause.OnChange(func(st syncpause.Status) { s.Hub.Broadcast("sync-pause", st) })
 	return s

internal/daemon/autosnapshot.go

@@ -0,0 +1,70 @@
+package daemon
+
+import (
+	"errors"
+	"path/filepath"
+	"sync"
+
+	"github.com/opensave/opensave/internal/snapshot"
+)
+
+// An automatic snapshot that could not read every file of the save is
+// discarded (snapshot.CreateAutomatic), and the watcher takes the change again
+// at its next pass. That is right once or twice: a file was being written.
+// A game that holds a file open the whole time it runs — a log, a lock file —
+// has every one discarded until it closes, and with them the sync each would
+// have started. Said only in the log, that looked like a save neither backed
+// up nor synced for no reason; so after a few passes in a row it is said on
+// screen, once, until a snapshot of the game succeeds again.
+
+// incompleteAutoSnapshotsToSay is how many passes in a row must end with the
+// snapshot discarded before it is said on screen.
+const incompleteAutoSnapshotsToSay = 3
+
+// SnapshotsIncomplete is what is said: the game, and some of the files that
+// could not be read.
+type SnapshotsIncomplete struct {
+	GameID   string   `json:"gameId"`
+	GameName string   `json:"gameName"`
+	Files    []string `json:"files"` // names only, at most three
+	Count    int      `json:"count"` // how many could not be read
+}
+
+type incompleteAutoSnapshots struct {
+	mu     sync.Mutex
+	inARow map[string]int
+}
+
+// autoSnapshotTaken ends a game's run of discarded automatic snapshots.
+func (d *Daemon) autoSnapshotTaken(gameID string) {
+	d.incomplete.mu.Lock()
+	delete(d.incomplete.inARow, gameID)
+	d.incomplete.mu.Unlock()
+}
+
+// autoSnapshotGaveUp is the watcher's pass ending without a snapshot. Only an
+// incomplete one counts: any other failure is the log's to tell.
+func (d *Daemon) autoSnapshotGaveUp(gameID string, err error) {
+	var inc *snapshot.IncompleteError
+	if !errors.As(err, &inc) {
+		return
+	}
+	d.incomplete.mu.Lock()
+	if d.incomplete.inARow == nil {
+		d.incomplete.inARow = map[string]int{}
+	}
+	d.incomplete.inARow[gameID]++
+	n := d.incomplete.inARow[gameID]
+	d.incomplete.mu.Unlock()
+	if n != incompleteAutoSnapshotsToSay || d.OnSnapshotsIncomplete == nil {
+		return
+	}
+	said := SnapshotsIncomplete{GameID: gameID, GameName: inc.Game, Count: len(inc.Skipped)}
+	for _, p := range inc.Skipped {
+		if len(said.Files) == 3 {
+			break
+		}
+		said.Files = append(said.Files, filepath.Base(p))
+	}
+	d.OnSnapshotsIncomplete(said)
+}

internal/daemon/autosnapshot_test.go

@@ -0,0 +1,45 @@
+package daemon
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/opensave/opensave/internal/snapshot"
+)
+
+// A game whose automatic snapshots keep coming out incomplete is said on
+// screen once per run of them; a snapshot that succeeds ends the run, and
+// failures of any other kind are left to the log.
+func TestIncompleteAutoSnapshotsAreSaidOncePerRun(t *testing.T) {
+	var said []SnapshotsIncomplete
+	d := &Daemon{OnSnapshotsIncomplete: func(s SnapshotsIncomplete) { said = append(said, s) }}
+	incomplete := &snapshot.IncompleteError{Game: "Hades", Skipped: []string{"/s/a.log", "/s/b.lock", "/s/c", "/s/d"}}
+
+	for i := 0; i < incompleteAutoSnapshotsToSay-1; i++ {
+		d.autoSnapshotGaveUp("hades", incomplete)
+	}
+	d.autoSnapshotGaveUp("hades", errors.New("disk full"))
+	if len(said) != 0 {
+		t.Fatalf("said after %d incomplete passes: %v", incompleteAutoSnapshotsToSay-1, said)
+	}
+	d.autoSnapshotGaveUp("hades", incomplete)
+	if len(said) != 1 {
+		t.Fatalf("said %d time(s) after %d incomplete passes, want once", len(said), incompleteAutoSnapshotsToSay)
+	}
+	got := said[0]
+	if got.GameID != "hades" || got.GameName != "Hades" || got.Count != 4 || len(got.Files) != 3 || got.Files[0] != "a.log" {
+		t.Errorf("said %+v", got)
+	}
+	d.autoSnapshotGaveUp("hades", incomplete)
+	if len(said) != 1 {
+		t.Errorf("said again within the same run")
+	}
+
+	d.autoSnapshotTaken("hades")
+	for i := 0; i < incompleteAutoSnapshotsToSay; i++ {
+		d.autoSnapshotGaveUp("hades", incomplete)
+	}
+	if len(said) != 2 {
+		t.Errorf("a new run after a good snapshot was said %d time(s), want once more", len(said)-1)
+	}
+}

internal/daemon/daemon.go

@@ -90,6 +90,11 @@ type Daemon struct {
 	// newgames.go.
 	OnNewGames func([]NewGame)
 	newGames   newGameState
+
+	// OnSnapshotsIncomplete is told when a game's automatic snapshots keep
+	// coming out incomplete, once per run of them. See autosnapshot.go.
+	OnSnapshotsIncomplete func(SnapshotsIncomplete)
+	incomplete            incompleteAutoSnapshots
 	// scanMu runs one save scan at a time. See ScanForSaves.
 	scanMu sync.Mutex
 
@@ -240,9 +245,19 @@ func New(opts Options) (*Daemon, error) {
 		},
 		SetLastManifestHash: s.SetLastManifestHash,
 		CreateSnapshot: func(gameID string) error {
-			_, err := snaps.Create(gameID, "", true)
+			_, err := snaps.CreateAutomatic(gameID)
+			if err == nil {
+				d.autoSnapshotTaken(gameID)
+			}
 			return err
 		},
+		SnapshotGaveUp: d.autoSnapshotGaveUp,
+		SyncWriting: func(gameID string) bool {
+			return d.P2P != nil && d.P2P.Sync != nil && d.P2P.Sync.BeingWritten(gameID)
+		},
+		SyncWritingNow: func(gameID string) bool {
+			return d.P2P != nil && d.P2P.Sync != nil && d.P2P.Sync.WritingNow(gameID)
+		},
 		OnChanged: func(gameID string) {
 			// An emptied save is noticed as it happens, and said on screen,
 			// even with no other device online to hold it back from.

internal/p2p/syncengine/engine.go

@@ -127,6 +127,8 @@ type Engine struct {
 	// on this device right now (settle.go).
 	settleMu sync.Mutex
 	gates    map[string]*saveGate
+	// writtenAt is when a sync last stopped writing each game (BeingWritten).
+	writtenAt map[string]time.Time
 	// servedMu guards served: the save states this device recently handed
 	// each peer, per game (served.go).
 	servedMu sync.Mutex

internal/p2p/syncengine/settle.go

@@ -131,10 +131,44 @@ func (e *Engine) Writing(gameID string) (done func()) {
 		once = true
 		g := e.gateLocked(gameID)
 		g.writers--
+		if e.writtenAt == nil {
+			e.writtenAt = map[string]time.Time{}
+		}
+		e.writtenAt[gameID] = time.Now()
 		e.wakeLocked(gameID, g)
 	}
 }
 
+// writeEcho is how long after a sync stops writing a game its file events may
+// still be arriving.
+const writeEcho = 10 * time.Second
+
+// BeingWritten reports whether a sync on this device is writing a game's save
+// now, or was a moment ago — what the watcher asks before taking a burst it
+// could not attribute (an overflow of file events) for a change of the game's.
+// A pull of a save of many files overflows the event queue again and again,
+// and each one read as the game saving: an automatic snapshot of a half-pulled
+// save every few seconds, which pushed the real ones out of retention.
+func (e *Engine) BeingWritten(gameID string) bool {
+	if e.WritingNow(gameID) {
+		return true
+	}
+	e.settleMu.Lock()
+	defer e.settleMu.Unlock()
+	return time.Since(e.writtenAt[gameID]) < writeEcho
+}
+
+// WritingNow reports whether a sync on this device is writing a game's save at
+// this moment — what the watcher waits out before looking at a burst, so it
+// never judges a save half-way between two states. Not the moments after:
+// a change the game makes then is the game's, and its snapshot is not held up.
+func (e *Engine) WritingNow(gameID string) bool {
+	e.settleMu.Lock()
+	defer e.settleMu.Unlock()
+	g := e.gates[gameID]
+	return g != nil && (g.writers > 0 || g.waitingWriters > 0)
+}
+
 // Reading holds the game's save still for a sync to read it: it waits until
 // nothing is writing it, then keeps writers out until done is called. It
 // returns ErrSettling if ctx ends first. Hold it only for the reading itself —

internal/snapshot/incomplete_test.go

@@ -0,0 +1,69 @@
+package snapshot
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// holdOpen has the named file of the save fail to open while archiving, as a
+// file the game holds open does.
+func holdOpen(t *testing.T, name string) {
+	t.Helper()
+	open := openForArchive
+	openForArchive = func(p string) (*os.File, error) {
+		if filepath.Base(p) == name {
+			return nil, errors.New("the process cannot access the file because it is being used by another process")
+		}
+		return open(p)
+	}
+	t.Cleanup(func() { openForArchive = open })
+}
+
+// An automatic snapshot that could not read every file is discarded: it would
+// stand in the history as a save that never existed.
+func TestCreateAutomatic_DiscardsAnIncompleteSnapshot(t *testing.T) {
+	env := setup(t)
+	writeSave(t, env.saveDir, "slot1.sav", "progress")
+	writeSave(t, env.saveDir, "game.log", "running")
+	holdOpen(t, "game.log")
+
+	_, err := env.mgr.CreateAutomatic("game1")
+	var inc *IncompleteError
+	if !errors.As(err, &inc) {
+		t.Fatalf("CreateAutomatic = %v, want an IncompleteError", err)
+	}
+	if len(inc.Skipped) != 1 || filepath.Base(inc.Skipped[0]) != "game.log" {
+		t.Errorf("skipped %v, want game.log", inc.Skipped)
+	}
+	if snaps, _ := env.store.ListSnapshots("game1", "main"); len(snaps) != 0 {
+		t.Errorf("%d snapshot(s) kept, want none", len(snaps))
+	}
+}
+
+// Every other snapshot is kept with what could be read, as before: above all
+// the copy kept before a sync replaces the save. Refusing it would stop the
+// sync replacing files for as long as the game holds one open.
+func TestOtherSnapshotsAreKeptWhenAFileCannotBeRead(t *testing.T) {
+	for name, create := range map[string]func(m *Manager) error{
+		"before replacing": func(m *Manager) error {
+			_, err := m.CreateBeforeReplacing("game1", "before sync replaced local files")
+			return err
+		},
+		"by hand": func(m *Manager) error { _, err := m.Create("game1", "mine", false); return err },
+	} {
+		t.Run(name, func(t *testing.T) {
+			env := setup(t)
+			writeSave(t, env.saveDir, "slot1.sav", "progress")
+			writeSave(t, env.saveDir, "game.log", "running")
+			holdOpen(t, "game.log")
+			if err := create(env.mgr); err != nil {
+				t.Fatalf("snapshot failed: %v", err)
+			}
+			if snaps, _ := env.store.ListSnapshots("game1", "main"); len(snaps) != 1 {
+				t.Errorf("%d snapshot(s) kept, want 1", len(snaps))
+			}
+		})
+	}
+}