Skip to content

fix(review): re-sign instrumental preview URLs on expiry (refresh-on-error) - #973

Merged
beveradb merged 2 commits into
mainfrom
feat/sess-20260901-1626-preview-instrumental-audio
Sep 1, 2026
Merged

fix(review): re-sign instrumental preview URLs on expiry (refresh-on-error)#973
beveradb merged 2 commits into
mainfrom
feat/sess-20260901-1626-preview-instrumental-audio

Conversation

@beveradb

@beveradb beveradb commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Problem

Completing a combined review failed to play the instrumental in the preview modal — toggling to Instrumental loaded nothing, with NS_ERROR_DOM_NETWORK_ERR in DevTools (Firefox) on the review-audio/instrumental_with_backing.ogg GCS request.

Root cause: the overlaid instrumental <audio> uses the signed GCS URL baked into the review payload at page load, which has a 120-minute expiry. A long review session outlives it, so the stem fetch returns 400 ExpiredToken and playback silently fails. Confirmed by fetching the exact URL from the report: ExpiredToken — Request signature expired at 2026-09-01T20:16:26Z (page had been open ~2h11m).

Reloading didn't help because the review UI persists the full CorrectionData — including instrumental_options[].audio_url — to localStorage (lyrics_analyzer_data) and rehydrates it when the user has unsaved edits, so the same expired URL came back.

The [SW] Service worker registered log is a red herring: frontend/public/sw.js has no fetch handler.

Fix (refresh-on-error)

  • Backend: extract a shared _build_instrumental_options helper and add GET /api/review/{job_id}/instrumental-urls returning freshly-signed stem URLs.
  • Frontend: on the stem <audio> error event, re-fetch fresh URLs via the new endpoint and swap them in. The existing video↔audio sync effect (keyed on the URL) resumes playback in place. Capped at 3 refreshes per modal open so a genuinely-broken stem (404) can't loop.

No new user-facing strings (the recovery is silent) → no i18n changes.

Testing

  • Backend: 3 new tests for the endpoint (both stems / single stem / 404) — TestInstrumentalUrlsRefresh. Full test_routes_review.py + test_combined_review_flow.py + test_audio_transcoding_service.py = 111 passed.
  • Frontend: 3 new PreviewVideoSection tests (swap on error / cap at 3 / no-op without refresh fn). Full unit suite = 1248 passed.
  • CodeRabbit local review: no findings.

@coderabbitai ignore

beveradb and others added 2 commits September 1, 2026 16:46
…error)

The combined-review preview modal's overlaid instrumental <audio> used the
signed GCS URL baked into the review payload at page load (120-min expiry).
A review session (or a reload that rehydrates cached correction data from
localStorage) can outlive that, so toggling to "Instrumental" silently fails
to load the stem (Firefox surfaces it as NS_ERROR_DOM_NETWORK_ERR).

- Backend: extract a shared `_build_instrumental_options` helper and add
  `GET /api/review/{job_id}/instrumental-urls` returning freshly-signed URLs.
- Frontend: on the stem <audio> `error`, re-fetch fresh URLs via the new
  endpoint and swap them in; the existing sync effect resumes playback in
  place. Capped at 3 refreshes per modal open so a genuinely-broken stem
  can't loop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Google's cloud-sdk apt mirror intermittently serves a Packages index whose
checksum doesn't match its Release file, and stale runner apt-lists caches
produce the same "Hash Sum mismatch" — repeatedly failing the emulator job's
setup with exit code 100 (unrelated to the code under test). Clear the apt
lists cache and retry the update+install a few times to ride out the desync.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@beveradb
beveradb merged commit 3c58143 into main Sep 1, 2026
22 checks passed
@beveradb
beveradb deleted the feat/sess-20260901-1626-preview-instrumental-audio branch September 1, 2026 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant