fix(reader): stop web audiobook progress retrying rejected writes

Every non-OK playback-state response marked the write dirty and retried it every five seconds, so a stale manifest (412) or a position past the rounded track duration (400) looped for the rest of the session. Each attempt also minted a new operationId, which defeated the server's replay protection, and capturedAt was stamped at flush time rather than when the position was taken. Capture the timestamp and one operationId with each position and reuse them across retries. Retry only network failures, 5xx and 429, and drop other rejected writes. On 409, reload the server state and keep the local write only if it is newer. On 412, hold the write and have the player reload the manifest, resending once the revision changes. Clamp positions to the track duration from the manifest.
2026-11-03 00:11:37 -06:00 · 2026-09-28 22:23:06 +00:00
parent d8f68a0d38
commit c76d9c01f9
3 changed files with 225 additions and 45 deletions
@@ -77,7 +77,23 @@ const audioFiles = computed<AudiobookManifestAsset[]>(() => manifest.value?.asse
 // ── Progress ──────────────────────────────────────────────────────────────────

 const manifestRevision = computed(() => manifest.value?.revision ?? '')
-const progress = useAudioProgress(props.bookId, { trackingEnabled, manifestRevision })
+const progress = useAudioProgress(props.bookId, { trackingEnabled, manifestRevision, assets: audioFiles, onManifestStale: reloadStaleManifest })
+
+async function reloadStaleManifest() {
+  try {
+    const res = await api(`/api/v1/audiobooks/${props.bookId}/manifest`)
+    if (!res.ok || !mounted) return
+    const next = (await res.json()) as AudiobookManifest
+    const current = audioFiles.value
+    // The queue was built from the old track list, so only a manifest with the same tracks can replace it in place.
+    const sameTracks = next.assets.length === current.length && next.assets.every((asset, index) => asset.assetId === current[index]?.assetId)
+    if (!sameTracks) return
+    manifest.value = next
+    progress.flush()
+  } catch {
+    // Progress stays on hold until the manifest can be reloaded.
+  }
+}

 // ── Queue (created lazily after files load) ───────────────────────────────────

@@ -0,0 +1,137 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { ref } from 'vue'
+import { useAudioProgress } from '../useAudioProgress'
+
+const apiMock = vi.hoisted(() => vi.fn<(url: string, init?: RequestInit) => Promise<unknown>>())
+vi.mock('@/lib/api', () => ({ api: apiMock }))
+
+const ASSET_ID = 'aud_00000000-0000-4000-8000-000000000001'
+const REVISION_A = 'a'.repeat(64)
+const REVISION_B = 'b'.repeat(64)
+
+function response(status: number, body: unknown = null) {
+  return { ok: status >= 200 && status < 300, status, json: () => Promise.resolve(body) }
+}
+
+function serverState(overrides: Record<string, unknown> = {}) {
+  return {
+    assetId: ASSET_ID,
+    positionMs: 1_000,
+    percentage: 1,
+    completed: false,
+    capturedAt: new Date().toISOString(),
+    revision: 1,
+    manifestRevision: REVISION_A,
+    ...overrides,
+  }
+}
+
+function puts() {
+  return apiMock.mock.calls
+    .filter(([, init]) => init?.method === 'PUT')
+    .map(([, init]) => JSON.parse(init!.body as string) as Record<string, unknown>)
+}
+
+function setup() {
+  const manifestRevision = ref(REVISION_A)
+  const onManifestStale = vi.fn<() => void>()
+  const progress = useAudioProgress(1, {
+    manifestRevision,
+    assets: [{ assetId: ASSET_ID, durationMs: 20_000 }],
+    onManifestStale,
+  })
+  return { progress, manifestRevision, onManifestStale }
+}
+
+describe('useAudioProgress', () => {
+  beforeEach(() => {
+    vi.useFakeTimers()
+    vi.setSystemTime(new Date('2026-09-01T10:00:00.000Z'))
+    apiMock.mockReset()
+  })
+
+  afterEach(() => {
+    vi.useRealTimers()
+  })
+
+  it('stamps capturedAt when the position is captured and clamps it to the asset duration', async () => {
+    apiMock.mockResolvedValue(response(200, serverState({ revision: 1 })))
+    const { progress } = setup()
+
+    progress.update(ASSET_ID, 20.3)
+    await vi.advanceTimersByTimeAsync(5_000)
+
+    expect(puts()).toEqual([expect.objectContaining({ positionMs: 20_000, capturedAt: '2026-09-01T10:00:00.000Z', baseRevision: 0 })])
+    expect(progress.revision.value).toBe(1)
+  })
+
+  it('retries a network failure with the same operationId and capturedAt', async () => {
+    apiMock.mockRejectedValueOnce(new TypeError('Failed to fetch')).mockResolvedValue(response(200, serverState()))
+    const { progress } = setup()
+
+    progress.update(ASSET_ID, 5)
+    await vi.advanceTimersByTimeAsync(10_000)
+
+    const [first, second] = puts()
+    expect(puts()).toHaveLength(2)
+    expect(second!.operationId).toBe(first!.operationId)
+    expect(second!.capturedAt).toBe(first!.capturedAt)
+  })
+
+  it('retries a server error but drops a write the server rejects with 400', async () => {
+    apiMock.mockResolvedValueOnce(response(503)).mockResolvedValue(response(400))
+    const { progress } = setup()
+
+    progress.update(ASSET_ID, 5)
+    await vi.advanceTimersByTimeAsync(60_000)
+
+    expect(puts()).toHaveLength(2)
+  })
+
+  it('holds the write on 412 until the manifest is reloaded', async () => {
+    apiMock.mockResolvedValueOnce(response(412)).mockResolvedValue(response(200, serverState()))
+    const { progress, manifestRevision, onManifestStale } = setup()
+
+    progress.update(ASSET_ID, 5)
+    await vi.advanceTimersByTimeAsync(60_000)
+
+    expect(puts()).toHaveLength(1)
+    expect(onManifestStale).toHaveBeenCalledTimes(1)
+
+    manifestRevision.value = REVISION_B
+    progress.flush()
+    await vi.advanceTimersByTimeAsync(0)
+
+    const [stale, fresh] = puts()
+    expect(fresh).toMatchObject({ manifestRevision: REVISION_B, operationId: stale!.operationId, capturedAt: stale!.capturedAt })
+  })
+
+  it('drops the write on 409 when the server state is at least as new', async () => {
+    apiMock.mockImplementation((_url, init) =>
+      Promise.resolve(init?.method === 'PUT' ? response(409) : response(200, serverState({ revision: 4, capturedAt: '2026-09-01T10:00:02.000Z' }))),
+    )
+    const { progress } = setup()
+
+    progress.update(ASSET_ID, 5)
+    await vi.advanceTimersByTimeAsync(60_000)
+
+    expect(puts()).toHaveLength(1)
+    expect(progress.revision.value).toBe(4)
+  })
+
+  it('retries on 409 with the reloaded revision when the local write is newer', async () => {
+    apiMock
+      .mockResolvedValueOnce(response(409))
+      .mockResolvedValueOnce(response(200, serverState({ revision: 4, capturedAt: '2026-09-01T09:59:00.000Z' })))
+      .mockResolvedValue(response(200, serverState({ revision: 5 })))
+    const { progress } = setup()
+
+    progress.update(ASSET_ID, 5)
+    await vi.advanceTimersByTimeAsync(10_000)
+
+    const [first, retry] = puts()
+    expect(puts()).toHaveLength(2)
+    expect(retry).toMatchObject({ baseRevision: 4, operationId: first!.operationId })
+    expect(progress.revision.value).toBe(5)
+  })
+})
@@ -1,4 +1,5 @@
 import { onUnmounted, ref, unref, type MaybeRef } from 'vue'
+import type { AudiobookManifestAsset, AudiobookPlaybackState } from '@bookorbit/types'
 import { api } from '@/lib/api'

 const SAVE_THROTTLE_MS = 5_000
@@ -6,6 +7,15 @@ const SAVE_THROTTLE_MS = 5_000
 export interface AudioProgressOptions {
  trackingEnabled?: MaybeRef<boolean>
  manifestRevision: MaybeRef<string>
+  assets?: MaybeRef<Pick<AudiobookManifestAsset, 'assetId' | 'durationMs'>[]>
+  onManifestStale?: () => void
+}
+
+interface PendingWrite {
+  assetId: string
+  positionMs: number
+  capturedAt: string
+  operationId: string
 }

 export function useAudioProgress(bookId: number, options: AudioProgressOptions) {
@@ -14,85 +24,102 @@ export function useAudioProgress(bookId: number, options: AudioProgressOptions)
  const revision = ref(0)
  const loaded = ref(false)
  const trackingEnabled = options.trackingEnabled ?? true
+  const url = `/api/v1/audiobooks/${bookId}/playback-state`

-  let pendingAssetId: string | null = null
-  let pendingPosition = 0
+  let pending: PendingWrite | null = null
  let saveTimer: ReturnType<typeof setTimeout> | null = null
-  let dirty = false
  let saving = false
+  let staleManifestRevision: string | null = null
+
+  function applyState(state: AudiobookPlaybackState | null) {
+    revision.value = state?.revision ?? 0
+    resumeAssetId.value = state?.assetId ?? null
+    resumePosition.value = (state?.positionMs ?? 0) / 1000
+  }

  async function load() {
    if (!unref(trackingEnabled)) {
      loaded.value = true
      return
    }
-    const res = await api(`/api/v1/audiobooks/${bookId}/playback-state`)
+    const res = await api(url)
    // Mark loaded regardless of response so callers can distinguish
    // "load attempted" from "load not yet called".
    loaded.value = true
    if (!res.ok) return
-    const data = await res.json()
-    if (data) {
-      resumeAssetId.value = data.assetId ?? null
-      resumePosition.value = (data.positionMs ?? 0) / 1000
-      revision.value = data.revision ?? 0
-    }
+    applyState(await res.json())
+  }
+
+  function scheduleFlush() {
+    if (saveTimer) return
+    saveTimer = setTimeout(() => {
+      saveTimer = null
+      void flushIfDirty()
+    }, SAVE_THROTTLE_MS)
  }

  function update(assetId: string, positionSeconds: number) {
    if (!unref(trackingEnabled)) return
-    pendingAssetId = assetId
-    pendingPosition = positionSeconds
-    dirty = true
-
-    if (!saveTimer) {
-      saveTimer = setTimeout(() => {
-        saveTimer = null
-        flushIfDirty()
-      }, SAVE_THROTTLE_MS)
+    pending = {
+      assetId,
+      positionMs: Math.max(0, Math.round(positionSeconds * 1000)),
+      capturedAt: new Date().toISOString(),
+      operationId: crypto.randomUUID(),
    }
+    scheduleFlush()
+  }
+
+  function settle(write: PendingWrite) {
+    if (pending === write) pending = null
+  }
+
+  function clampToAsset(write: PendingWrite): number {
+    const durationMs = unref(options.assets)?.find((asset) => asset.assetId === write.assetId)?.durationMs
+    return typeof durationMs === 'number' ? Math.min(write.positionMs, durationMs) : write.positionMs
  }

  async function flushIfDirty() {
    if (!unref(trackingEnabled)) return
-    if (!dirty || pendingAssetId === null || saving) return
-    const assetId = pendingAssetId
-    const positionMs = Math.max(0, Math.round(pendingPosition * 1000))
+    const write = pending
+    if (write === null || saving) return
+    const manifestRevision = unref(options.manifestRevision)
+    // A 412 means this manifest is gone; hold the write until the view loads the new one.
+    if (manifestRevision === staleManifestRevision) return
    const body = JSON.stringify({
-      assetId,
-      positionMs,
-      capturedAt: new Date().toISOString(),
-      operationId: crypto.randomUUID(),
+      assetId: write.assetId,
+      positionMs: clampToAsset(write),
+      capturedAt: write.capturedAt,
+      operationId: write.operationId,
      baseRevision: revision.value,
-      manifestRevision: unref(options.manifestRevision),
+      manifestRevision,
    })
-    dirty = false
    saving = true
    try {
-      const res = await api(`/api/v1/audiobooks/${bookId}/playback-state`, {
+      const res = await api(url, {
        method: 'PUT',
        headers: { 'Content-Type': 'application/json' },
        body,
      })
-      if (!res.ok) {
-        dirty = true
-        if (res.status === 409) await load()
-        return
+      if (res.ok) {
+        applyState(await res.json())
+        settle(write)
+      } else if (res.status === 409) {
+        const current = await api(url)
+        if (!current.ok) return
+        const server = (await current.json()) as AudiobookPlaybackState | null
+        applyState(server)
+        if (server && Date.parse(server.capturedAt) >= Date.parse(write.capturedAt)) settle(write)
+      } else if (res.status === 412) {
+        staleManifestRevision = manifestRevision
+        options.onManifestStale?.()
+      } else if (res.status < 500 && res.status !== 429) {
+        settle(write)
      }
-      const state = await res.json()
-      revision.value = state.revision
-      resumeAssetId.value = state.assetId
-      resumePosition.value = state.positionMs / 1000
    } catch {
-      dirty = true
+      // Network failure: keep the write and retry it with the same operationId.
    } finally {
      saving = false
-      if (dirty && !saveTimer) {
-        saveTimer = setTimeout(() => {
-          saveTimer = null
-          void flushIfDirty()
-        }, SAVE_THROTTLE_MS)
-      }
+      if (pending !== null && unref(options.manifestRevision) !== staleManifestRevision) scheduleFlush()
    }
  }