Skip to content

Correct misleading crash-recovery comment (release pair) - #234

Open
ryanbr wants to merge 1 commit into
masterfrom
fix/ivs-crash-comment-correction
Open

Correct misleading crash-recovery comment (release pair)#234
ryanbr wants to merge 1 commit into
masterfrom
fix/ivs-crash-comment-correction

Conversation

@ryanbr

@ryanbr ryanbr commented May 21, 2026

Copy link
Copy Markdown
Owner

Comment-only docfix on the release pair. No behavioral change, no version bump (per the no-per-PR-version convention) — bullet added under ## Unreleased for the next roll-up.

What the old comment said (false)

The IVS WASM worker error handler in vaft.user.js + vaft-ublock-origin.js previously claimed:

"…and existing reload cooldown prevents runaway restart loops."

That's verifiably false. The crash handler calls doTwitchPlayerTask(false, true, 'early') directly on the main thread. The reload-cooldown logic (tooSoonSinceLastReload, escalation, 30s/90s) lives in the worker's processM3U8 reload-decision path — which crash-recovery never traverses. doTwitchPlayerTask itself has no throttle. So a hypothetical persistently-crashing WASM would currently produce an unbounded crash → uncooled hard reload → fresh worker → crash loop, not a cooldown-throttled one.

This misleading comment was what originally pulled me into the circuit-breaker thread (closed PR #230). Fixing the comment regardless removes the documented-but-absent safety property trap.

What the new comment says (accurate)

  • Lists all three observed WASM-error variants: index out of bounds, indirect call signature mismatch, indirect call to null (the v82 and v83 logs gave us the latter two).
  • States the per-worker crashed flag dedupes the multi-event burst from one crash — and is per-closure (does NOT survive the reload).
  • States honestly: no cross-worker storm guard is implemented here. Intentional — observed crashes have all been single + recoverable.
  • Notes the trip-wire for revisiting: clustered crashes ≥2 within ~60s in one session. If that ever appears in field logs, the circuit-breaker design from closed PR v68.4.0: circuit-breaker for IVS WASM crash recovery #230 is the ready answer.

Scope

  • vaft.user.js + vaft-ublock-origin.js: comment block rewritten (6 lines → 10 lines, both files synced).
  • CHANGELOG.md: bullet under ## Unreleased### Documentation.
  • Testing pair NOT touched. The testing variant (from the v651 testing-direct work during v68.4.0: circuit-breaker for IVS WASM crash recovery #230, which was never reverted) already has the full circuit-breaker code + its own accurate comment. Leaving that as intentional release-vs-testing divergence, per CLAUDE.md "Testing files include experimental features."

acorn-clean on both release files. Single squashed commit.

🤖 Generated with Claude Code

Fixes a documented-but-absent safety property in the IVS WASM worker
crash-recovery comment at vaft.user.js + vaft-ublock-origin.js. The
old comment claimed 'existing reload cooldown prevents runaway
restart loops' — verifiably false; the crash handler calls
doTwitchPlayerTask(..., 'early') directly on the main thread,
bypassing the worker-side cooldown logic in processM3U8.

Rewritten to honestly state: per-worker 'crashed' flag dedupes the
multi-event burst from one crash; no cross-worker storm guard is
implemented (intentionally — observed crashes have all been single +
recoverable; revisit only if field logs ever show clustered crashes
>=2 within ~60s). Lists all three observed WASM-error variants.

No behavioral change — comment text only. Testing pair untouched
(already has the full circuit-breaker + accurate comment from the
v651 testing-direct work, intentional release-vs-testing divergence).
No version bump per the no-per-PR-version convention; bullet added
under ## Unreleased for the next roll-up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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