diff --git a/AGENTS.md b/AGENTS.md index f6df23c..a27dfe5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,8 +54,8 @@ params** — an early version leaked a user's Last.fm session key into analytics `step_viewed` → `auth_success` / `auth_failed` / `auth_token_invalid` → `upload_parse_started` / `upload_parse_completed` / `upload_no_matching_files` → `tracks_selected` → `scrobble_started` / `scrobble_resumed` / `scrobble_paused` / -`scrobble_completed`, plus `session_saved`, `session_resumed`, and -`user_logged_out`. +`scrobble_stopped` / `scrobble_completed`, plus `session_saved`, +`session_resumed`, and `user_logged_out`. Rate limiting has its own events: `scrobble_rate_limited`, `scrobble_rate_limit_cooldown_complete`, `scrobble_rate_limit_recovered`, @@ -65,12 +65,56 @@ from the adaptive limit in `RateLimitTracker`. Network failures emit `scrobble_network_error`. `scrobble_ignored` fires when Last.fm accepts the request but discards the play (see below). -`scrobble_paused` carries a `reason` of `burst_limit`, `daily_limit`, -`rate_limit`, `rate_limit_exhausted`, `network_error`, `lastfm_daily_limit`, or -`manual` — only `manual` is a user action; the rest are Last.fm throttling, not -bugs. These dominate by volume (`scrobble_paused` and `scrobble_rate_limited` -are the highest-count events after `step_viewed`), so treat them as normal -operation, not signal. +**`scrobble_paused` and `scrobble_stopped` are deliberately separate events**, so +"how often does a run end early, and why" is answerable without knowing which +reasons happen to be terminal. Every terminal case used to be a `scrobble_paused` +reason too, and two of them (`repeated_rejections`, `repeated_failures`) emitted +nothing at all. + +- `scrobble_paused` — transient; the loop resumes by itself. Reasons: + `burst_limit`, `rate_limit`, `network_error`. This is Last.fm throttling, not + a bug: treat it as normal operation rather than signal. +- `scrobble_stopped` — terminal; the run is over until the user comes back. + Reasons: `daily_limit`, `lastfm_daily_limit`, `rate_limit_exhausted`, + `repeated_rejections`, `repeated_failures`, `manual`. Only `manual` is a user + action. Every one carries `auto_saved`, which is the difference between an + interruption and lost work — all six now save, so `auto_saved: false` in the + data means the save itself failed and is worth investigating. + +All terminal paths go through the `trackStopped()` helper rather than emitting +inline, so a new one cannot silently skip the event. + +Every terminal path must also leave the user a way back in. The paused panel's +resume button is gated on the `canResume` computed (`stopped || manuallyPaused`), +not on `stopped` alone: a manual pause is terminal but is *not* an error, so it +sets `manuallyPaused` and gets `info` styling via `pauseAlertType` instead of a +red `error` banner. Setting only `paused` renders a **disabled** "Wait Here" +button waiting on an auto-resume the loop has already returned from — a dead end +that stranded manual pauses, `repeated_rejections` and `repeated_failures`. + +`manualPause()` deliberately does **not** save. It only raises the flags; the +save and the `scrobble_stopped` event happen in the scrobble loop's pause check, +which runs *between* tracks. Saving on the click would snapshot a +`scrobbledTracks` that omits the in-flight track, so the resume would re-send it +— and for a re-tagged play that means a freshly allocated timestamp and a +phantom duplicate scrobble. + +`burst_limit` is **preventive pacing, not a stoppage**, and it is emitted once +per *stretch* of throttled sends — paired with a `scrobble_pacing_ended` event +carrying `paced_tracks`, `paced_wait_ms` and `pacing_duration_ms`. Do not read +it as one event per pause-and-resume of a track. + +That pairing exists because `msUntilBurstSafe()` frees exactly one slot at a +time, so once the rolling window is saturated *every* remaining track waits a +fraction of a second. Emitting per track made `scrobble_paused` a per-scrobble +heartbeat: it went from ~15/day to 734/day and briefly became the +highest-count event after `step_viewed`. **Events from 2026-07-27 to 2026-07-29 +are inflated this way and are not comparable with later data**, and before +2026-07-29 `scrobble_paused` also carried the terminal reasons. + +Pacing waits under `PACING_COUNTDOWN_THRESHOLD_MS` (10s) are a plain sleep that +leaves the scrobbling UI up; only longer waits show the paused panel and +countdown. ### Measuring completion @@ -95,6 +139,14 @@ Use the helpers in `src/services/Analytics.ts` (`trackEvent`, `trackError`, `identifyUser`) rather than calling `posthog` directly. Analytics must never break the app: every call is wrapped in try/catch and ignored on failure. +That invariant covers *deriving* the payload, not just sending it. `trackError` +receives arbitrary values — the global handlers hand it whatever a third party +threw — so coercion goes through `toError()`, which guards `String(value)` +(that throws for Symbols and for objects with a throwing `toString`), and +`normalizeErrorForTracking` is called inside the try. Doing either before the +guard loses the report *and* throws a fresh error out of a `catch` block or a +global handler, which is exactly where it does the most damage. + ## Scrobble timestamps Last.fm keys a scrobble on **(user, artist, track, timestamp)** and silently diff --git a/src/components/ScrobbleStep.vue b/src/components/ScrobbleStep.vue index 1e30fc5..4c02f28 100644 --- a/src/components/ScrobbleStep.vue +++ b/src/components/ScrobbleStep.vue @@ -23,6 +23,15 @@

Scrobbling... {{ currentTrackName }}

+ + + {{ pacingNotice }} + +
Overall: {{ totalSucceeded }} of {{ originalTotalTracks }} scrobbled @@ -40,7 +49,7 @@
- + {{ pauseReason }} @@ -51,7 +60,7 @@
You can save progress and leave now, then resume later at any time.
-
+
Your progress has been saved automatically — just come back to this page later and choose "Resume".
@@ -60,7 +69,14 @@
Save Progress & Leave - Try Again Now + + + {{ manuallyPaused ? 'Resume Now' : 'Try Again Now' }} + Wait Here
@@ -133,6 +149,18 @@ const MAX_RATE_LIMIT_RETRIES = RATE_LIMIT_BACKOFF_MS.length; const NETWORK_ERROR_COOLDOWN_MS = 30 * MS_PER_SECOND; const NETWORK_ERROR_COOLDOWN_SECONDS = Math.ceil(NETWORK_ERROR_COOLDOWN_MS / MS_PER_SECOND); +// Preventive pacing waits shorter than this are ordinary throughput control, +// not something to interrupt the user with. +// +// `msUntilBurstSafe()` frees exactly one slot at a time, so once the rolling +// window is saturated *every* remaining track waits a fraction of a second +// (observed median: 630ms). Treating each of those as a pause flipped the whole +// view into the paused panel and emitted a `scrobble_paused` event once per +// track — roughly one analytics event per scrobble. Above this threshold the +// wait is long enough to be worth an explicit countdown, which happens when a +// window saturated by an earlier session has to drain before we can start. +const PACING_COUNTDOWN_THRESHOLD_MS = 10 * MS_PER_SECOND; + const MAX_CONSECUTIVE_FAILURES = 10; // Re-tagged plays (see Scrobble.reTagged) are stamped at send time, starting @@ -181,10 +209,22 @@ export default Vue.extend({ // A pause the loop will not resume from on its own. Distinguishes "wait a // moment" from "we've given up for now, come back later". stopped: false, + // A terminal pause the *user* asked for. Terminal like `stopped`, but not + // an error, so it gets its own flag rather than colouring a deliberate + // action as a failure. + manuallyPaused: false, autoSaved: false, pauseReason: '', countdown: 0, countdownTimer: null as number | null, + // Preventive pacing is a *stretch* of throttled sends, not a single + // pause: it is entered once when the rolling window fills and left once + // the window has room again. Telemetry and UI both describe the stretch, + // so neither fires per track. + pacing: false, + pacingStartedAtMs: 0, + pacedTracks: 0, + pacedWaitMs: 0, // Mirrors of RateLimitTracker state. The tracker itself is deliberately // non-reactive (see created()), so these are refreshed explicitly. burstCount: 0, @@ -235,6 +275,27 @@ export default Vue.extend({ const seconds = this.countdown % 60; return `${minutes}:${seconds.toString().padStart(2, '0')}`; }, + pacingNotice(): string { + return `Pacing to stay under Last.fm's rate limit — ${this.burstCount} scrobbles in the` + + ' last 10 minutes. Scrobbling continues automatically.'; + }, + + /* + Whether the loop has returned for good and the user must restart it. Both + giving up on an error and pausing on purpose qualify; only the transient + waits (which clear `paused` themselves) do not. + */ + canResume(): boolean { + return this.stopped || this.manuallyPaused; + }, + + // A deliberate pause is not a failure, so it must not be styled as one. + pauseAlertType(): string { + if (this.manuallyPaused) { + return 'info'; + } + return this.stopped ? 'error' : 'warning'; + }, }, created() { this.syncRateLimitCounters(); @@ -270,6 +331,71 @@ export default Vue.extend({ this.burstLimit = tracker.burstLimit; }, + sleep(ms: number): Promise { + return new Promise((resolve) => { window.setTimeout(resolve, ms); }); + }, + + /** + * Report that the run has ended and will not pick itself back up. + * + * Kept as its own event rather than another `scrobble_paused` reason: every + * terminal case used to share the event name with the transient ones, so + * answering "how often does a run stop early, and why" meant knowing by + * heart which of the seven reasons happen to be terminal. `scrobble_paused` + * now always means "waiting, will resume itself"; this always means "over + * until the user comes back". + * + * `auto_saved` records whether their progress actually survived, which is + * the difference between an interruption and lost work. + */ + trackStopped(reason: string, extra: Record = {}) { + this.endPacing(); + trackEvent('scrobble_stopped', this.progressProps({ + reason, + auto_saved: this.autoSaved, + ...extra, + })); + }, + + /** + * Enter the paced state, reporting it once. Repeated calls while already + * pacing are deliberately no-ops: the burst check runs per track, but a + * pacing stretch is one event, not one per track. + */ + beginPacing(tracker: RateLimitTracker, waitMs: number) { + if (this.pacing) { + return; + } + this.pacing = true; + this.pacingStartedAtMs = Date.now(); + this.pacedTracks = 0; + this.pacedWaitMs = 0; + trackEvent('scrobble_paused', this.progressProps({ + reason: 'burst_limit', + burst_count: tracker.burstCount, + burst_limit: tracker.burstLimit, + wait_ms: waitMs, + })); + }, + + /** + * Leave the paced state, reporting how much the pacing actually cost. Safe + * to call unconditionally — it is a no-op when we were not pacing. + */ + endPacing() { + if (!this.pacing) { + return; + } + const { pacedTracks, pacedWaitMs } = this; + this.pacing = false; + trackEvent('scrobble_pacing_ended', this.progressProps({ + reason: 'burst_limit', + paced_tracks: pacedTracks, + paced_wait_ms: pacedWaitMs, + pacing_duration_ms: Date.now() - this.pacingStartedAtMs, + })); + }, + /** * Progress properties attached to every scrobble analytics event. * @@ -299,10 +425,14 @@ export default Vue.extend({ async scrobble() { const tracker = this.rateLimitTracker(); + // Defensive: a previous run that was torn down mid-stretch would + // otherwise suppress the next `beginPacing`. + this.endPacing(); this.scrobbling = true; this.completed = false; this.paused = false; this.stopped = false; + this.manuallyPaused = false; this.autoSaved = false; this.pauseReason = ''; // A manual retry after giving up starts a fresh backoff ladder. @@ -339,8 +469,18 @@ export default Vue.extend({ // `i` is incremented conditionally at the end so a rate-limited track can be retried. for (let i = this.scrobbledTracks; i < tracks.length;) { - // Check if manually paused + // Check if manually paused. Transient waits clear `paused` before + // returning, so reaching here with it set means the user asked to stop. + // The save happens *here* rather than in `manualPause` so the snapshot + // is taken between tracks: saving mid-send would omit the in-flight + // track's increment and re-send it on resume, which for a re-tagged + // play means a brand new timestamp and a phantom duplicate scrobble. if (this.paused) { + this.endPacing(); + if (this.manuallyPaused) { + this.autoSave(); + this.trackStopped('manual', { track_index: i }); + } return; } @@ -350,15 +490,26 @@ export default Vue.extend({ // don't have. const burstWaitMs = tracker.msUntilBurstSafe(); if (burstWaitMs > 0) { - this.pauseReason = `Pacing to stay under Last.fm's rate limit — ${tracker.burstCount} scrobbles sent in the last 10 minutes. Resuming automatically.`; - trackEvent('scrobble_paused', this.progressProps({ - reason: 'burst_limit', - burst_count: tracker.burstCount, - burst_limit: tracker.burstLimit, - wait_ms: burstWaitMs, - })); - await this.pauseWithCountdown(burstWaitMs); + // Reported once for the whole stretch, not once per track: the window + // only ever frees one slot at a time, so this branch is taken for + // every remaining track once the limit is reached. + this.beginPacing(tracker, burstWaitMs); + this.pacedTracks += 1; + this.pacedWaitMs += burstWaitMs; + + if (burstWaitMs >= PACING_COUNTDOWN_THRESHOLD_MS) { + this.pauseReason = `Pacing to stay under Last.fm's rate limit — ${tracker.burstCount} scrobbles sent in the last 10 minutes. Resuming automatically.`; + await this.pauseWithCountdown(burstWaitMs); + } else { + // Sub-second spacing between sends. Deliberately *not* + // `pauseWithCountdown`: that shows the paused panel and only + // resolves on a 1s tick, which would both flicker the UI once per + // track and round every wait up to a full second. + await this.sleep(burstWaitMs); + } this.syncRateLimitCounters(); + } else { + this.endPacing(); } // Daily ceiling: a rolling 24h window, so it frees up gradually rather @@ -367,14 +518,13 @@ export default Vue.extend({ const dailyWaitMs = tracker.msUntilDailySafe(); if (dailyWaitMs > 0) { this.pauseReason = `You've reached Last.fm's daily limit of about ${DAILY_LIMIT} scrobbles. Come back in ${formatDuration(dailyWaitMs)} to continue where you left off.`; - trackEvent('scrobble_paused', this.progressProps({ - reason: 'daily_limit', - daily_count: tracker.dailyCount, - wait_ms: dailyWaitMs, - })); this.stopped = true; this.paused = true; this.autoSave(); + this.trackStopped('daily_limit', { + daily_count: tracker.dailyCount, + wait_ms: dailyWaitMs, + }); return; } @@ -419,10 +569,10 @@ export default Vue.extend({ // the queue *unprocessed*. Recording it as failed here would // count it once now and again when the resume re-sends it. this.pauseReason = 'Last.fm says you have hit your daily scrobble limit. Your progress is saved — come back tomorrow and resume.'; - trackEvent('scrobble_paused', this.progressProps({ reason: 'lastfm_daily_limit' })); this.stopped = true; this.paused = true; this.autoSave(); + this.trackStopped('lastfm_daily_limit'); return; } @@ -449,12 +599,16 @@ export default Vue.extend({ this.errorDetails = this.failedTracks[this.failedTracks.length - 1].error; this.showError = true; this.pauseReason = 'Paused because Last.fm rejected several scrobbles in a row.'; + this.stopped = true; this.paused = true; // These were permanent rejections, so the tracks are genuinely // processed — advance past this one before saving so the resume // doesn't re-send and re-count it. this.scrobbledTracks += 1; this.autoSave(); + this.trackStopped('repeated_rejections', { + consecutive_failures: consecutiveFailures, + }); return; } } catch (e) { @@ -462,6 +616,10 @@ export default Vue.extend({ // same track rather than counting it as a failure. if (LastFm.isRateLimitError(e)) { const rateLimitStartMs = Date.now(); + // Real throttling supersedes preventive pacing: close out the + // stretch so its cost is reported against the pacing, not the + // minutes we are about to spend backing off. + this.endPacing(); if (this.firstRateLimitAtMs === null) { this.firstRateLimitAtMs = rateLimitStartMs; } @@ -483,9 +641,6 @@ export default Vue.extend({ // Retrying further is not useful: recovery from a sustained rate // limit takes hours, not minutes. Save and hand control back. this.pauseReason = `Last.fm is still rate limiting your account after ${MAX_RATE_LIMIT_RETRIES} retries over ${formatDuration(Date.now() - this.firstRateLimitAtMs)}. This usually clears after a few hours — come back later and resume.`; - trackEvent('scrobble_paused', this.progressProps({ - reason: 'rate_limit_exhausted', - })); trackEvent('scrobble_rate_limit_gave_up', this.progressProps({ track_index: i, burst_count: tracker.burstCount, @@ -497,6 +652,10 @@ export default Vue.extend({ this.stopped = true; this.paused = true; this.autoSave(); + this.trackStopped('rate_limit_exhausted', { + rate_limit_pause_count: this.rateLimitPauseCount, + elapsed_since_first_rate_limit_ms: Date.now() - this.firstRateLimitAtMs, + }); return; } @@ -535,8 +694,16 @@ export default Vue.extend({ this.errorMessage = `${MAX_CONSECUTIVE_FAILURES} tracks failed in a row. There may be a problem with Last.fm or your authentication.`; this.errorDetails = (e as Error).message || String(e); this.showError = true; - this.pauseReason = 'Paused due to repeated failures.'; + this.pauseReason = 'Paused due to repeated failures. Your progress is saved.'; + this.stopped = true; this.paused = true; + // The track is left unconsumed (scrobbledTracks is not advanced): + // these were exceptions, not rejections, so a resume should retry + // it rather than skip it. + this.autoSave(); + this.trackStopped('repeated_failures', { + consecutive_failures: consecutiveFailures, + }); return; } } @@ -558,6 +725,7 @@ export default Vue.extend({ } } + this.endPacing(); this.completed = true; trackEvent('scrobble_completed', this.progressProps()); this.$emit('complete'); @@ -589,9 +757,13 @@ export default Vue.extend({ }, manualPause() { - this.pauseReason = 'Manually paused.'; - trackEvent('scrobble_paused', this.progressProps({ reason: 'manual' })); + this.pauseReason = 'Paused. Your progress is saved — resume whenever you like.'; this.paused = true; + // Tracked separately from `stopped`, which drives the red error styling. + // A deliberate pause is not a failure, but it is just as terminal: the + // loop returns, so the user needs a way back in. The scrobble loop picks + // this up and does the saving and reporting. + this.manuallyPaused = true; }, /** diff --git a/src/services/Analytics.ts b/src/services/Analytics.ts index 0f998ca..f933c9b 100644 --- a/src/services/Analytics.ts +++ b/src/services/Analytics.ts @@ -88,17 +88,37 @@ function normalizeErrorForTracking(error: Error): { trackedError: Error; rawMess }; } +/* + Coerces an arbitrary thrown value into an Error. `String(value)` throws for + Symbols and for objects with a throwing toString/Symbol.toPrimitive, and the + global handlers (window.onerror, unhandledrejection, Vue.config.errorHandler) + pass through whatever a third party threw — so this coercion has to be + survivable in its own right. +*/ +function toError(value: unknown): Error { + if (value instanceof Error) { + return value; + } + try { + return new Error(String(value)); + } catch (e) { + return new Error('Unstringifiable thrown value'); + } +} + /* Records an error against a named context (e.g. 'upload.parseZip') so failures can be grouped and investigated. Captures both a filterable custom event and, when available, PostHog's native exception so it shows up in error tracking. */ export function trackError(context: string, error: unknown, extra: Record = {}): void { - const err = error instanceof Error ? error : new Error(String(error)); - const { trackedError, rawMessage } = normalizeErrorForTracking(err); + const err = toError(error); if (initialized) { try { + // Kept inside the try: the global handlers hand us whatever a third party + // threw, so even reading .message/.stack off it can fail. + const { trackedError, rawMessage } = normalizeErrorForTracking(err); posthog.capture('scrobblify_error', { context, message: trackedError.message, diff --git a/tests/scrobblify.spec.ts b/tests/scrobblify.spec.ts index 228e03e..ba6c0d9 100644 --- a/tests/scrobblify.spec.ts +++ b/tests/scrobblify.spec.ts @@ -686,6 +686,173 @@ test.describe('Session Resume', () => { // Falls back to this file's own totals rather than reporting nothing. await expect(page.locator('.overall-progress')).toContainText('3 of 5'); }); + + test('preventive pacing keeps scrobbling, it does not pause per track', async ({ page }) => { + // Regression: `msUntilBurstSafe()` frees exactly one slot at a time, so once + // the rolling window is full *every* remaining track waits a fraction of a + // second. Those waits went through `pauseWithCountdown`, which flipped the + // whole view into the paused panel and emitted a `scrobble_paused` event — + // once per track, for the rest of the import. + test.setTimeout(120000); + await interceptLastFm(page); + await page.route('https://ws.audioscrobbler.com/**', async (route: Route) => { + const params = new URLSearchParams( + route.request().method() === 'POST' + ? route.request().postData() || '' + : new URL(route.request().url()).search, + ); + if (params.get('method') === 'track.scrobble') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ scrobbles: { '@attr': { accepted: 1, ignored: 0 } } }), + }); + return; + } + await route.fallback(); + }); + + await page.goto('/#/scrobble'); + await mockLastFmAuth(page); + + // A burst window filled to exactly the default limit, spaced at the rate + // that limit implies (500 sends / 10 minutes = one per 1.2s), so slots free + // up one at a time and every track waits a fraction of a second — the shape + // seen in production. + // + // Anchored slightly in the future because the window keeps draining while + // the resume UI is driven: whatever time that takes just frees that many + // slots. The lead has to stay under PACING_COUNTDOWN_THRESHOLD_MS (10s) so + // that even an instant start pauses below the countdown threshold, and the + // track count has to exceed the slots a slow start can free. + const SETUP_LEAD_MS = 8000; + const anchor = Date.now() + SETUP_LEAD_MS; + const sendTimestamps = Array.from({ length: 500 }, (_, k) => anchor - (499 - k) * 1200); + const tracks = Array.from({ length: 24 }, (_, n) => ({ + track: `Track ${n + 1}`, + artist: `Artist ${n + 1}`, + album: '', + timestamp: Date.UTC(2024, 0, n + 1), + })); + await seedSavedState(page, buildState({ + totalTracks: 24, + completedIndices: [], + tracks, + originalTotalTracks: 24, + originalSucceededCount: 0, + sendTimestamps, + })); + await page.reload(); + + await expect(page.locator('text=Resume previous session?')).toBeVisible({ timeout: 10000 }); + await page.getByRole('button', { name: 'Resume', exact: true }).click(); + await expect(page.locator('text=24 tracks ready to scrobble')).toBeVisible({ timeout: 5000 }); + await page.waitForTimeout(2500); + await page.getByRole('button', { name: 'Scrobble', exact: true }).click(); + + // Pacing must announce itself... + await expect(page.locator('text=Pacing to stay under')).toBeVisible({ timeout: 20000 }); + + // ...without ever handing the run over to the paused panel. Sampled + // repeatedly because the bug was a flicker — one flip per track — which a + // single instantaneous check could land between. + let sawPausedPanel = false; + let finished = false; + for (let i = 0; i < 600 && !finished; i++) { + // eslint-disable-next-line no-await-in-loop + await page.waitForTimeout(100); + // eslint-disable-next-line no-await-in-loop + if (await page.locator('text=Auto-resuming in').isVisible()) { + sawPausedPanel = true; + } + // eslint-disable-next-line no-await-in-loop + finished = await page.locator('text=Finished scrobbling').isVisible(); + } + + expect(sawPausedPanel).toBe(false); + expect(finished).toBe(true); + }); + + test('a manual pause saves, offers a way back, and does not re-send the in-flight track', async ({ page }) => { + // Regression: "Pause & Save" set `paused` but not `stopped`, so the paused + // panel rendered a *disabled* "Wait Here" button and waited forever for an + // auto-resume the loop had already returned from — a dead end. It also + // never actually saved, despite the label. + test.setTimeout(90000); + + const scrobbled: string[] = []; + const tracks = Array.from({ length: 12 }, (_, n) => ({ + track: `Track ${n + 1}`, + artist: `Artist ${n + 1}`, + album: `Album ${n + 1}`, + timestamp: Date.UTC(2024, 0, n + 1), + })); + + await interceptLastFm(page); + await page.route('https://ws.audioscrobbler.com/**', async (route: Route) => { + const params = new URLSearchParams( + route.request().method() === 'POST' + ? route.request().postData() || '' + : new URL(route.request().url()).search, + ); + if (params.get('method') === 'track.scrobble') { + scrobbled.push(params.get('track[0]') || ''); + // Slow enough that the pause lands mid-run rather than after the queue + // has already drained. + await new Promise((resolve) => { setTimeout(resolve, 700); }); + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ scrobbles: { '@attr': { accepted: 1, ignored: 0 } } }), + }); + return; + } + await route.fallback(); + }); + + await page.goto('/#/scrobble'); + await mockLastFmAuth(page); + await seedSavedState(page, buildState({ + totalTracks: 12, + completedIndices: [0, 1], + tracks, + originalTotalTracks: 12, + originalSucceededCount: 2, + })); + await page.reload(); + + await expect(page.locator('text=Resume previous session?')).toBeVisible({ timeout: 10000 }); + await page.getByRole('button', { name: 'Resume', exact: true }).click(); + await page.waitForTimeout(2500); + await page.getByRole('button', { name: 'Scrobble', exact: true }).click(); + + // Let a couple of tracks go out, then ask to stop. + await expect.poll(() => scrobbled.length, { timeout: 20000 }).toBeGreaterThanOrEqual(2); + await page.getByRole('button', { name: 'Pause & Save' }).click(); + + // The user must be offered a way back in, not a disabled button. + const resume = page.getByRole('button', { name: 'Resume Now' }); + await expect(resume).toBeVisible({ timeout: 10000 }); + await expect(resume).toBeEnabled(); + await expect(page.locator('text=Wait Here')).toHaveCount(0); + // And the label's promise must have been kept. + await expect(page.locator('text=Your progress has been saved automatically')) + .toBeVisible(); + + // The loop really stopped rather than quietly draining the queue. + const atPause = scrobbled.length; + await page.waitForTimeout(2000); + expect(scrobbled.length).toBe(atPause); + expect(atPause).toBeLessThan(10); + + await resume.click(); + await expect(page.locator('text=Finished scrobbling')).toBeVisible({ timeout: 40000 }); + + // The save is taken between tracks, so the track that was in flight when + // the user clicked must not be sent twice. A re-send of a re-tagged play + // would be allocated a fresh timestamp and become a phantom scrobble. + expect(scrobbled).toEqual(tracks.slice(2).map((t) => t.track)); + }); }); test.describe('Rate limit handling', () => {