Skip to content

Report pacing once per stretch, and stop stranding paused users - #76

Merged
taurheim merged 1 commit into
masterfrom
fix/scrobble-pause-telemetry
Aug 4, 2026
Merged

Report pacing once per stretch, and stop stranding paused users#76
taurheim merged 1 commit into
masterfrom
fix/scrobble-pause-telemetry

Conversation

@taurheim

@taurheim taurheim commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Why

The 2026-07-27 deploy swapped reactive rate-limit handling for preventive pacing. It worked — real Last.fm throttling all but disappeared — but the wait sits at the top of the per-track loop, and RateLimitTracker.msUntilWindowHasRoom only ever frees one slot at a time. So once the rolling window filled, every track took that branch.

scrobble_paused / burst_limit, per day:

07-17 07-20 07-27 07-28 07-29
rate_limit 327 128 63 5 5
burst_limit ~15 ~19 180 427 734

Those 734 came from 5 scrobble_started events. Median wait_ms was 630ms and median burst_count was 500 — pinned exactly at burst_limit. The old code paused 10 minutes and reset the counter, giving one event per ~950 scrobbles; this was roughly one per scrobble.

It wasn't only telemetry. pauseWithCountdown sets paused = true, which swaps the entire template into the paused panel — so users watched the UI flicker in and out of "Paused" once per track. And because it resolves on a 1s interval tick, every sub-second wait was rounded up to a full second.

What changed

Pacing is a stretch, not a pause. beginPacing/endPacing bracket it and report once on entry and once on exit (scrobble_pacing_ended, carrying paced_tracks / paced_wait_ms / pacing_duration_ms). Waits under PACING_COUNTDOWN_THRESHOLD_MS use a plain sleep behind an inline notice.

scrobble_stopped split out of scrobble_paused. Every terminal case previously reused the scrobble_paused name, and repeated_rejections / repeated_failures emitted nothing at all. Now:

  • scrobble_paused — transient, auto-resumes: burst_limit, rate_limit, network_error
  • scrobble_stopped — terminal: daily_limit, lastfm_daily_limit, rate_limit_exhausted, repeated_rejections, repeated_failures, manual

All terminal paths go through a trackStopped() helper so a new one can't silently skip the event, and each carries auto_saved.

Terminal paths all offer a way back in. The resume button was gated on stopped, which manualPause, repeated_rejections and repeated_failures never set — all three rendered a disabled "Wait Here" button waiting on an auto-resume the loop had already returned from. The only escape was leaving and re-importing. It now keys off canResume (stopped || manuallyPaused), with a manual pause styled info rather than red.

"Pause & Save" actually saves. It never called autoSave(), despite the label; nor did the repeated-failures stop. Note the save happens in the loop's between-tracks pause check, not in manualPause — snapshotting on the click would omit the in-flight track's increment and re-send it on resume, and for a re-tagged play that means a freshly allocated timestamp and a phantom duplicate scrobble.

trackError can no longer throw. It coerced with String(error) and normalised the payload outside its own try/catch, breaking the invariant the file documents. Both throw on a Symbol or an object with a throwing toString, and the global handlers pass through whatever a third party threw — so it lost the report and raised a fresh error out of a catch block. Coercion is now guarded in toError().

Testing

  • npm run lint:check — 0 errors (86 pre-existing warnings)
  • npm run build — clean
  • Two new regression tests, each verified to fail without its fix:
    • preventive pacing keeps scrobbling, it does not pause per track — samples repeatedly for the paused panel, since the bug was a per-track flicker a single check could land between
    • a manual pause saves, offers a way back, and does not re-send the in-flight track — asserts the enabled resume button, absence of "Wait Here", the save confirmation, that the loop halts, and that the resumed run scrobbles each track exactly once
  • 16/17 tests pass across Scrobble Step, Rate limit handling, Session Resume, No JS Errors, Import robustness. The one failure, gives up and saves instead of retrying a rate limit forever, is a pre-existing local-Windows flake — confirmed by stashing src/ and reproducing at HEAD; it passes in CI.

Heads-up

scrobble_stopped is a breaking change for any saved PostHog insight that filters scrobble_paused on a terminal reason — those reasons now live on the new event.

Also note burst_limit counts between 2026-07-27 and 2026-07-29 are inflated by this bug and aren't comparable with anything after it. AGENTS.md records that.

The 2026-07-27 deploy replaced reactive rate-limit handling with preventive
pacing, but left the wait at the top of the per-track loop. The rolling window
only ever frees one slot at a time, so once it filled, every single track took
that branch: `scrobble_paused`/`burst_limit` went from ~15 a day to 734, and the
whole view flipped into the paused panel and back once per track. The waits are
sub-second, so `pauseWithCountdown` was also rounding each one up to a full
second against a 1s interval tick.

Pacing is now a stretch rather than a pause. `beginPacing`/`endPacing` bracket
it, report once on entry and once on exit (`scrobble_pacing_ended`, with
`paced_tracks`/`paced_wait_ms`/`pacing_duration_ms`), and waits under
PACING_COUNTDOWN_THRESHOLD_MS use a plain sleep behind an inline notice instead
of taking over the view.

Separate scrobble_stopped from scrobble_paused
----------------------------------------------
Every terminal case used to reuse the `scrobble_paused` event name, and two of
them (`repeated_rejections`, `repeated_failures`) emitted nothing at all, so
"how often does a run end early, and why" was unanswerable. Terminal paths now
go through a `trackStopped()` helper emitting `scrobble_stopped`, carrying
`auto_saved` to distinguish an interruption from lost work.

Give every terminal path a way back in
---------------------------------------
The paused panel gated its resume button on `stopped`, which `manualPause`,
`repeated_rejections` and `repeated_failures` never set. All three rendered a
*disabled* "Wait Here" button waiting on an auto-resume the loop had already
returned from — a dead end whose only exit was leaving and re-importing. The
button now keys off `canResume` (`stopped || manuallyPaused`); a deliberate
pause sets `manuallyPaused` so it reads as `info` rather than a red error.

"Pause & Save" also never saved, despite the label, and neither did the
repeated-failures stop. Both save now. The save deliberately happens in the
loop's between-tracks pause check rather than in `manualPause`: snapshotting on
the click would omit the in-flight track's increment and re-send it on resume,
which for a re-tagged play means a freshly allocated timestamp and a phantom
duplicate scrobble.

Don't let the error reporter throw
-----------------------------------
`trackError` coerced with `String(error)` and normalised the payload *outside*
its try/catch, breaking the one invariant the file documents. Both throw on a
Symbol or an object with a throwing toString, and the global handlers pass
through whatever a third party threw — losing the report and raising a fresh
error out of a catch block or a global handler. Coercion is now guarded in
`toError()` and normalisation happens inside the try.

Both new behaviours are covered by regression tests, each confirmed to fail
without its fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f5423b1e-b316-412a-bade-c28bf90eeb0f
@taurheim
taurheim merged commit 3ee4d2f into master Aug 4, 2026
3 checks passed
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