diff --git a/docs/architecture/pwa-self-hosted.md b/docs/architecture/pwa-self-hosted.md index c7073bcf6..b8d08ee9e 100644 --- a/docs/architecture/pwa-self-hosted.md +++ b/docs/architecture/pwa-self-hosted.md @@ -228,7 +228,8 @@ settings and guards read them in every build. (the page-hide flush waits for a batch still travelling), a failed write, including a refused `keepalive` one or one the server took and never answered (it ends after the request timeout), stays queued for the next - flush, and + flush (a write saved whose answer was cut off counts as saved on an + unknown version: only later edits replay on the next conflict), and every unconfirmed write (queued or travelling) is saved in `localStorage` per page load (`iptvnator-server:unsent-writes::`). The tab remembers its page load in `sessionStorage`, so a reload takes its @@ -343,7 +344,8 @@ settings and guards read them in every build. replaced volume at the same address is another database), and it checks the instance as soon as it goes offline, so a blip costs no retry interval (only the instance, and once per retry interval: a read or a - check that keeps failing waits for the retry). Sign-out always navigates to sign-in, also when a guard kept the + check that keeps failing waits for the retry; the retry also runs while a + late answer set an unchecked page online). Sign-out always navigates to sign-in, also when a guard kept the page the first time. Both turn the guard's 401 (`isServerAccessChallenge`) into `reportAccessDenied()`, as the storage API client does with any 401, so a token changed on 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 79d6d219c..206dc490d 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 @@ -294,6 +294,34 @@ describe('ServerStorageSession', () => { expect(fetchMock).toHaveBeenCalledTimes(1); }); + it('keeps checking an unverified page a late answer set online', async () => { + writeServerAccessToken('t', 'i-1'); + const { session, status } = setup('token'); + await session.start(); + fetchMock.mockRejectedValue(new TypeError('offline')); + status.reportOffline(); + TestBed.tick(); + for (let turn = 0; turn < 10; turn += 1) { + await Promise.resolve(); + } + // A request sent before the outage is answered late. + status.reportSuccess(); + expect(status.state()).toBe('online'); + expect(status.isVerified()).toBe(false); + fetchMock.mockReset(); + fetchMock.mockResolvedValue( + answer({ instanceId: 'i-1', entries: {} }) + ); + + jest.advanceTimersByTime(30_000); + for (let turn = 0; turn < 10; turn += 1) { + await Promise.resolve(); + } + + expect(fetchMock.mock.calls[0][0]).toBe('/api/v1/instance'); + expect(status.isVerified()).toBe(true); + }); + it('reloads when another instance answers at the address', async () => { const { session, status } = setup('none'); const reload = jest 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 70eaa62d8..3230daca4 100644 --- a/libs/services/src/lib/server-storage/server-storage-session.ts +++ b/libs/services/src/lib/server-storage/server-storage-session.ts @@ -220,10 +220,13 @@ export class ServerStorageSession { this.userData.touch(); // Also online: a write the server refused (a 5xx) or a first // read that failed is not retried by anything else. + // Online but unverified too: a late answer to a request sent + // before the outage may have set it online again. const state = this.status.state(); if ( state === 'offline' || - (state === 'online' && this.userData.needsSync()) + (state === 'online' && + (this.userData.needsSync() || !this.status.isVerified())) ) { void this.refresh(); } 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 4f702aff1..a1a036ecf 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 @@ -357,10 +357,7 @@ export class UserDataWriteQueue { )); this.retireInFlight(entry); if (outcome.status === 'saved') { - // Without a tag a newer edit keeps this one's steps. - if (outcome.tag !== undefined) { - this.confirmed(entry, outcome.tag); - } + this.confirmed(entry, outcome.tag); } else if (outcome.status === 'conflict') { this.replayOnConflict(entry, outcome.current); } else if (outcome.status === 'gone') { @@ -390,9 +387,7 @@ export class UserDataWriteQueue { .readJson>(response) .catch(() => ({ tag: undefined })); this.retireInFlight(entry); - if (tag !== undefined) { - this.confirmed(entry, tag); - } + this.confirmed(entry, tag); } catch (error) { this.retireInFlight(entry); if ( @@ -424,8 +419,21 @@ export class UserDataWriteQueue { } /** The server took `entry`: the edit queued behind it builds on it. */ - private confirmed(entry: PendingEntry, tag: string | null): void { - this.tags.update(entry.namespace, entry.playlistId, tag); + /** + * `entry` landed. Without a readable tag (a body cut off after its + * headers) a conditional write leaves the version unknown: a newer edit + * drops the landed steps all the same and conflicts, so only the edits + * made after it replay. An unconditional one stays unconditional. + */ + private confirmed( + entry: PendingEntry, + read: string | null | undefined + ): void { + const tag = + read !== undefined || entry.tag === undefined ? read : UNKNOWN_TAG; + if (tag !== undefined) { + this.tags.update(entry.namespace, entry.playlistId, tag); + } const key = entryKey(entry); const next = this.pending.get(key); if (next?.steps.some((step) => step.sent === entry.edit)) { @@ -433,7 +441,10 @@ export class UserDataWriteQueue { this.pending.set(key, { ...next, steps: next.steps.filter((step) => step.sent !== entry.edit), - tag: next.tag === UNKNOWN_TAG ? UNKNOWN_TAG : tag, + tag: + next.tag === UNKNOWN_TAG || tag === undefined + ? next.tag + : tag, }); } } diff --git a/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts b/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts index cce264177..c2af32173 100644 --- a/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts +++ b/libs/services/src/lib/server-storage/server-user-data-sync.spec.ts @@ -681,6 +681,75 @@ describe('ServerUserDataStorage with other devices', () => { expect(server.state['xtream-playback-positions']['p1']).toEqual(at(30)); }); + it('does not replay a saved edit whose answer was cut off', async () => { + const at = (id: number, seconds: number) => ({ + contentType: 'vod', + contentXtreamId: id, + positionSeconds: seconds, + }); + const server = fakeServer({ + 'xtream-playback-positions': { p1: [at(1, 10), at(2, 5)] }, + }); + const send = server.api.sendJson.getMockImplementation(); + if (!send) { + throw new Error('The fake server answers sendJson'); + } + let cutOff = true; + let answer = (): void => undefined; + const tab = new ServerUserDataStorage( + { + ...server.api, + // Saved, but the body with the new tag never arrives. + sendJson: jest.fn(async (...args: Parameters) => { + if (!cutOff) { + return send(...args); + } + cutOff = false; + await new Promise((resolve) => (answer = resolve)); + await send(...args); + return { + json: async () => { + throw new TypeError('body cut off'); + }, + } as unknown as Response; + }), + } as never, + memoryStorage(), + jest.fn(), + 'default', + memoryStorage() + ); + await tab.hydrate(); + tab.setItem( + 'xtream-playback-positions', + JSON.stringify({ p1: [at(1, 20), at(2, 5)] }) + ); + const saving = tab.flush(); + for (let turn = 0; turn < 10; turn += 1) { + await Promise.resolve(); + } + // Movie 2 changes here while that write travels. + tab.setItem( + 'xtream-playback-positions', + JSON.stringify({ p1: [at(1, 20), at(2, 6)] }) + ); + answer(); + await saving; + expect(server.state['xtream-playback-positions']['p1']).toEqual([ + at(1, 20), + at(2, 5), + ]); + // Another device saves movie 1 at 30 s. + server.state['xtream-playback-positions']['p1'] = [at(1, 30), at(2, 5)]; + await tab.flush(); + await tab.flush(); + + expect(server.state['xtream-playback-positions']['p1']).toEqual([ + at(1, 30), + at(2, 6), + ]); + }); + it("keeps the last page load's saved writes when storage refuses the copy", async () => { const server = fakeServer({ 'xtream-favorites': { p1: ['S'] } }); const local = memoryStorage();