From e286a62b2e0ce6fae65f5759a5a1c81171fbd96d Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 14 Oct 2026 14:11:33 +0200 Subject: [PATCH] fix(server-storage): a superseded page sends and saves nothing; re-reads keep other tabs' newer saves A suspended tab's retry timer could run before it was shown again: touch() recreated the record another tab had removed when it adopted the page's writes, so the page sent them again over newer edits. The facade now treats a page whose record is gone as superseded: the queue sends nothing (maySend hook), its journal and record are not written again, and the retry timer reloads it like a page shown again. A re-read answered after another tab saved an entry no longer writes the older answer over the shared cache: entries changed while the read travelled are kept, with an unknown tag, so the next write here conflicts and replays instead of removing what the other tab saved. Co-Authored-By: Claude Opus 5.5 --- docs/architecture/pwa-self-hosted.md | 6 +- .../server-storage-session.spec.ts | 4 + .../server-storage/server-storage-session.ts | 5 + .../server-user-data-adopt.spec.ts | 108 ++++++++++++++++++ .../server-storage/server-user-data-cache.ts | 29 +++++ .../server-storage/server-user-data-queue.ts | 5 + .../server-user-data-storage.ts | 48 +++++--- 7 files changed, 184 insertions(+), 21 deletions(-) diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index 67c5ac4b1..d0869dc5a 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -237,7 +237,8 @@ settings and guards read them in every build. its record closed on `pagehide` (not into the back-forward cache, where its record goes stale, as in a background tab the browser suspends; restored or shown again after another tab adopted it, the page reloads - rather than send those writes again), and + rather than send those writes again, and until then it neither sends nor + saves them, also when its retry timer runs first), and only a closed, stale or missing record is taken over: a duplicated tab, which copies `sessionStorage`, leaves the running tab its writes. A page load that still looked live (or whose close flag storage refused) is @@ -266,7 +267,8 @@ settings and guards read them in every build. stays parked). Replayed edits keep recently viewed lists newest first, as the app cuts them at their limit from the end. Also, re-reads run one at a time and keep entries edited while they were in - flight or not yet saved. A response body cut off after its headers counts + flight or not yet saved, and entries another tab saved meanwhile (the + answer predates them; their next write here conflicts and replays). A response body cut off after its headers counts as offline. Each write is sent on the tag of the entry it was made against (`UserDataWriteQueue`); after a `412` the edit (before → after) is replayed on the server's current value, so another device's change to the diff --git a/libs/services/src/lib/server-storage/server-storage-session.spec.ts b/libs/services/src/lib/server-storage/server-storage-session.spec.ts index 7f6ae4b8c..dc9df4a16 100644 --- a/libs/services/src/lib/server-storage/server-storage-session.spec.ts +++ b/libs/services/src/lib/server-storage/server-storage-session.spec.ts @@ -185,6 +185,10 @@ describe('ServerStorageSession', () => { document.dispatchEvent(new Event('visibilitychange')); expect(reload).toHaveBeenCalledTimes(2); delete (document as { visibilityState?: unknown }).visibilityState; + + // And its retry timer, which may run before it is shown again. + jest.advanceTimersByTime(30_000); + expect(reload).toHaveBeenCalledTimes(3); }); it('goes to sign-in on every sign-out, also when already signed out', async () => { diff --git a/libs/services/src/lib/server-storage/server-storage-session.ts b/libs/services/src/lib/server-storage/server-storage-session.ts index b48829460..4bd8a273b 100644 --- a/libs/services/src/lib/server-storage/server-storage-session.ts +++ b/libs/services/src/lib/server-storage/server-storage-session.ts @@ -201,6 +201,11 @@ export class ServerStorageSession { } }); setInterval(() => { + // A suspended tab's timers can run before it is shown again. + if (!this.userData.resumed()) { + this.reloadPage(); + return; + } this.userData.touch(); // Also online: a write the server refused (a 5xx) or a first // read that failed is not retried by anything else. diff --git a/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts b/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts index d711040f5..dd644989e 100644 --- a/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts +++ b/libs/services/src/lib/server-storage/server-user-data-adopt.spec.ts @@ -209,4 +209,112 @@ describe('ServerUserDataStorage taking over other page loads', () => { expect(positions()).toEqual([at(1, 30), at(2, 6)]); }); + + it("keeps what another tab saved while this tab's read travelled", async () => { + const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); + const shared = memoryStorage(); + const a = new ServerUserDataStorage( + server.api as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await a.hydrate(); + let answer = (): void => undefined; + const b = new ServerUserDataStorage( + { + ...server.api, + // Answered with the state of when it was asked. + getJson: jest.fn(() => { + const old = server.api.getJson(); + return new Promise((resolve) => { + answer = () => resolve(old); + }); + }), + } as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + const reading = b.hydrate(); + for (let turn = 0; turn < 10; turn += 1) { + await Promise.resolve(); + } + a.setItem('xtream-favorites', JSON.stringify({ p1: ['S', 'X'] })); + await a.flush(); + answer(); + await reading; + + // Tab A edits the list as it reads it now. + const read = JSON.parse(a.getItem('xtream-favorites') ?? '{}'); + a.setItem( + 'xtream-favorites', + JSON.stringify({ p1: [...read.p1, 'Y'] }) + ); + await a.flush(); + await a.flush(); + + expect(server.state['xtream-favorites']['p1']).toEqual(['S', 'X', 'Y']); + }); + + it('sends nothing more once another tab took its writes over', async () => { + const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); + const shared = memoryStorage(); + const clock = jest.spyOn(Date, 'now').mockReturnValue(1_000); + let online = false; + const send = server.api.sendJson.getMockImplementation(); + if (!send) { + throw new Error('The fake server answers sendJson'); + } + const suspended = new ServerUserDataStorage( + { + ...server.api, + sendJson: jest.fn((...args: Parameters) => + online + ? send(...args) + : Promise.reject( + new ServerApiError(0, 'network', 'offline') + ) + ), + } as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await suspended.hydrate(); + suspended.setItem( + 'xtream-favorites', + JSON.stringify({ p1: ['S', 'X'] }) + ); + await suspended.flush(); + // Suspended past the record lifetime: another tab takes it over, + // sends it, and the user removes X there afterwards. + clock.mockReturnValue(1_000 + UNCONFIRMED_TTL_MS + 1); + const other = new ServerUserDataStorage( + server.api as never, + shared, + jest.fn(), + 'default', + memoryStorage() + ); + await other.hydrate(); + await other.flush(); + await other.flush(); + other.setItem('xtream-favorites', JSON.stringify({ p1: ['S'] })); + await other.flush(); + expect(server.state['xtream-favorites']['p1']).toEqual(['S']); + + // Its retry timer runs before the tab is shown again. + online = true; + suspended.touch(); + await suspended.flush(); + await suspended.flush(); + clock.mockRestore(); + + expect(server.state['xtream-favorites']['p1']).toEqual(['S']); + expect(suspended.resumed()).toBe(false); + }); }); diff --git a/libs/services/src/lib/server-storage/server-user-data-cache.ts b/libs/services/src/lib/server-storage/server-user-data-cache.ts index b0693473f..b63710a9e 100644 --- a/libs/services/src/lib/server-storage/server-user-data-cache.ts +++ b/libs/services/src/lib/server-storage/server-user-data-cache.ts @@ -3,6 +3,7 @@ import { type ServerUserStateNamespace, } from '@iptvnator/shared/interfaces'; import type { UserDataStorage } from '../user-data-storage'; +import { changedEntries } from './server-user-data-edit'; import { parseMap } from './server-user-data-entries'; /** Server-mode cache keys; the legacy unprefixed keys are never touched. */ @@ -59,6 +60,34 @@ export class UserDataCache { } } + /** What every namespace holds now, to compare with later. */ + snapshot(): Map { + return new Map( + SERVER_USER_STATE_NAMESPACES.map((namespace) => [ + namespace, + this.read(namespace), + ]) + ); + } + + /** Entries (`namespace|playlistId`) changed since `snapshot`. */ + changedSince( + snapshot: ReadonlyMap + ): string[] { + const keys: string[] = []; + for (const [namespace, before] of snapshot) { + if (this.read(namespace) !== before) { + for (const [playlistId] of changedEntries( + parseMap(before ?? '{}'), + this.map(namespace) + )) { + keys.push(`${namespace}|${playlistId}`); + } + } + } + return keys; + } + map(namespace: ServerUserStateNamespace): Record { return parseMap(this.read(namespace) ?? '{}'); } diff --git a/libs/services/src/lib/server-storage/server-user-data-queue.ts b/libs/services/src/lib/server-storage/server-user-data-queue.ts index c2ff73528..4f702aff1 100644 --- a/libs/services/src/lib/server-storage/server-user-data-queue.ts +++ b/libs/services/src/lib/server-storage/server-user-data-queue.ts @@ -41,6 +41,8 @@ export interface UserDataQueueHooks { value: unknown ) => void; readonly writeFailed: (error: unknown) => void; + /** False while nothing may go out (another tab took the writes over). */ + readonly maySend?: () => boolean; } /** @@ -291,6 +293,9 @@ export class UserDataWriteQueue { clearTimeout(this.flushTimer); this.flushTimer = null; } + if (this.hooks.maySend?.() === false) { + return []; + } const batch: PendingEntry[] = []; for (const [key, entry] of this.pending) { if (!this.held.some((matches) => matches(key))) { diff --git a/libs/services/src/lib/server-storage/server-user-data-storage.ts b/libs/services/src/lib/server-storage/server-user-data-storage.ts index ee72b0ac4..9db37e0d3 100644 --- a/libs/services/src/lib/server-storage/server-user-data-storage.ts +++ b/libs/services/src/lib/server-storage/server-user-data-storage.ts @@ -104,6 +104,7 @@ export class ServerUserDataStorage implements UserDataStorage { show: (namespace, playlistId, value) => this.cache.setEntry(namespace, playlistId, value), writeFailed: onWriteFailure, + maySend: () => !this.superseded(), }); claimCache(local, instanceId); const previous = swapPage(tabStore, instanceId, this.tabId); @@ -209,14 +210,18 @@ export class ServerUserDataStorage implements UserDataStorage { * instead). */ resumed(): boolean { - if ( - this.recorded && - !hasRecord(this.local, this.instanceId, this.tabId) - ) { - return false; - } this.publishUnconfirmed(); - return true; + return !this.superseded(); + } + + /** + * Its record is gone: another tab took this page's writes over, so + * nothing more of them goes out or is saved here. + */ + private superseded(): boolean { + return ( + this.recorded && !hasRecord(this.local, this.instanceId, this.tabId) + ); } /** Not read yet, or holding writes the server has not taken. */ @@ -342,17 +347,18 @@ export class ServerUserDataStorage implements UserDataStorage { const editsBefore = this.writes.lastEdit; const deletesBefore = this.deleted.length; await this.writes.flush(); + const sent = this.cache.snapshot(); const { entries, tags } = withoutDeleted( await this.api.getJson('/user-state'), this.deleted.slice(deletesBefore) ); const unsaved = this.writes.unsavedSince(editsBefore); - // Another tab's unconfirmed writes are in the shared cache too. - for (const key of othersUnconfirmed( - this.local, - this.instanceId, - this.tabId - )) { + // In the shared cache: other tabs' unconfirmed writes, and what a + // tab saved while this read travelled (newer than its answer). + for (const key of [ + ...othersUnconfirmed(this.local, this.instanceId, this.tabId), + ...this.cache.changedSince(sent), + ]) { unsaved.add(key); } const deferred = [...this.deferred.values()]; @@ -469,6 +475,9 @@ export class ServerUserDataStorage implements UserDataStorage { /** Persists what the server has not confirmed; skips idle rewrites. */ /** False when browser storage refused the copy. */ private saveUnsent(): boolean { + if (this.superseded()) { + return false; + } // A newer edit can be queued while an older one is still travelling. const writes = this.writes.unsentWrites(); if (writes.length === 0 && !this.unsentSaved) { @@ -508,11 +517,12 @@ export class ServerUserDataStorage implements UserDataStorage { } private publishUnconfirmed(): void { - this.recorded = markUnconfirmed( - this.local, - this.instanceId, - this.tabId, - [...this.writes.unconfirmedKeys(), ...this.deferred.keys()] - ); + // Superseded, it stays so: the record is not written again. + this.recorded = + this.superseded() || + markUnconfirmed(this.local, this.instanceId, this.tabId, [ + ...this.writes.unconfirmedKeys(), + ...this.deferred.keys(), + ]); } }