perf(featured-stream): stop probing Twitch on every page load (avoid rate limits) - #4810
Conversation
Liveness was detected in the browser by constructing a real autoplaying Twitch.Player for each configured channel and reading its ONLINE/OFFLINE events. Measured on a cold load with 3 channels configured and none live: 327 Twitch requests, for a panel that sat at opacity:0 the whole time. featured-stream.json now lists only channels the API's Helix cron sees live, in priority order, so channels[0] is simply what to embed. Nobody live is 0 Twitch requests (the SDK never loads), and an offline period costs one small JSON poll per minute instead of a player mount per channel. The mounted player is still watched: OFFLINE/ENDED hides the panel, which covers a stream ending mid-session and the ~3 min worst-case staleness of the served config (2 min cron + 60s edge cache). Dropping the probe also means the embed is only ever constructed when it will be visible.
WalkthroughThe featured stream component now selects live channels from the application API instead of using client-side probing. It stores the selected channel as reactive state, polls the API every minute when no player displays or when gameplay pauses, mounts one player for the selected channel, and resumes polling after player offline, ended, or mount failure events. The ChangesFeatured stream lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FeaturedStream
participant getFeaturedStream
participant TwitchPlayer
FeaturedStream->>getFeaturedStream: Poll for live channels
getFeaturedStream-->>FeaturedStream: Return selected channel
FeaturedStream->>TwitchPlayer: Mount selected channel
TwitchPlayer-->>FeaturedStream: READY event
FeaturedStream-->>FeaturedStream: Display panel
TwitchPlayer-->>FeaturedStream: ENDED or offline event
FeaturedStream->>getFeaturedStream: Resume polling
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
mountPlayer awaits the Twitch SDK, and only re-checked dismissed/inGame before that await. Closing the panel during the load then fell through to the "no mount node" branch and scheduled a poll. Harmless in practice (poll() early-returns once dismissed) but it left a timer behind. Adds tests for the close path: closing removes the panel and stops polling entirely, and it stays gone when a game ends. Includes a positive control showing the same 10 minute jump does keep polling while the panel is waiting for someone to go live, so the "stopped" assertion has teeth. Minimize deliberately keeps streaming, since Twitch disallows an obscured embed, so the minimized panel stays a visible thumbnail.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/FeaturedStream.ts (1)
255-271: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard the SDK-load failure path against a mid-load game start.
The catch block checks only
this.dismissedbefore callingschedulePoll(). The success path a few lines below (line 270) checks boththis.inGameandthis.dismissed, and the comment right above it states the poll must not stay scheduled behind a game start. If the SDK load fails while a game has started in the meantime, this branch still callsschedulePoll(), which contradicts that stated invariant.Match the guard used on line 270 so both paths handle the same race consistently.
🐛 Proposed fix
} catch (e) { // SDK blocked (extension, network): try again on the next poll rather than never. console.error("featured-stream: Twitch SDK load failed", e); - if (this.dismissed) return; + if (this.inGame || this.dismissed) return; this.goOffline(); this.schedulePoll(); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/FeaturedStream.ts` around lines 255 - 271, Update the SDK-load failure catch block in mountPlayer to guard the retry path with both this.inGame and this.dismissed, matching the success-path guard; when either state is active, return without calling goOffline or schedulePoll.
🧹 Nitpick comments (1)
src/client/FeaturedStream.ts (1)
272-309: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle a
Twitch.Playerconstruction failure the same way as an SDK load failure.
loadTwitchSdk()failures are caught and recovered withgoOffline()+schedulePoll(), butnew Twitch.Player(...)itself is not wrapped in a try/catch. If the constructor throws (bad host state, invalid options, or an SDK-internal error),mountPlayerrejects. Sincepoll()is invoked asvoid this.poll()from several callers, this becomes an unhandled promise rejection, and noschedulePoll()runs afterward, so the panel stops recovering until an unrelated event (leave-lobby) triggers a freshpoll().Wrap the construction in a try/catch that mirrors the SDK-load failure recovery, so an external-call failure here degrades gracefully instead of silently stalling.
♻️ Proposed fix
this.teardownPlayer(); // destroy the previous player so its listeners can't fire host.innerHTML = ""; const gen = ++this.mountGen; const fresh = () => gen === this.mountGen; // ignore events from a superseded mount - const player = new Twitch.Player(host, { - channel, - parent: [window.location.hostname], // bare host; self-adapts to any domain/subdomain - muted: true, // required for autoplay - autoplay: true, - width: "100%", - height: "100%", - }); + let player: TwitchPlayer; + try { + player = new Twitch.Player(host, { + channel, + parent: [window.location.hostname], // bare host; self-adapts to any domain/subdomain + muted: true, // required for autoplay + autoplay: true, + width: "100%", + height: "100%", + }); + } catch (e) { + console.error("featured-stream: Twitch player construction failed", e); + if (this.inGame || this.dismissed) return; + this.goOffline(); + this.schedulePoll(); + return; + } this.player = player; const P = Twitch.Player;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/FeaturedStream.ts` around lines 272 - 309, Wrap the Twitch.Player construction in mountPlayer with try/catch, covering the new Twitch.Player(host, options) call before assigning this.player or registering listeners. In the catch path, mirror loadTwitchSdk failure recovery by calling goOffline() and schedulePoll(), then return without continuing initialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/client/FeaturedStream.ts`:
- Around line 255-271: Update the SDK-load failure catch block in mountPlayer to
guard the retry path with both this.inGame and this.dismissed, matching the
success-path guard; when either state is active, return without calling
goOffline or schedulePoll.
---
Nitpick comments:
In `@src/client/FeaturedStream.ts`:
- Around line 272-309: Wrap the Twitch.Player construction in mountPlayer with
try/catch, covering the new Twitch.Player(host, options) call before assigning
this.player or registering listeners. In the catch path, mirror loadTwitchSdk
failure recovery by calling goOffline() and schedulePoll(), then return without
continuing initialization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 229b69b3-d4ec-44b2-a398-a6946c3fdb48
📒 Files selected for processing (2)
src/client/FeaturedStream.tstests/client/components/FeaturedStreamPanel.test.ts
github-project-automation
Bot
moved this from Triage
to Complete
in OpenFront Release Management
2 participants
What
The featured-stream panel no longer works out liveness itself.
featured-stream.jsonnow lists only channels the API's Helix cron sees live, in priority order, sochannels[0]is simply what to embed. Nothing Twitch-related loads until there is something to show.Pairs with openfrontio/infra#488.
Why
Liveness was detected by constructing a real autoplaying
Twitch.Playerfor each configured channel and reading itsONLINE/OFFLINEevents, walking the list until one was live. With 3 channels configured and none live, that boots three players behind anopacity-0panel.Meaning many requests were constantly being sent out.. That is not page breaking considering not much is happening on the main page. But still not the cleanest architecture..
Result
Same measurement method, after:
An offline period now costs one small JSON poll per minute against our own API (60s edge cache) instead of a player mount per channel.
Correctness
The mounted player is still watched, so this does not depend on the served config being perfectly fresh:
OFFLINE/ENDEDhides the panel and resumes polling. Covers a stream ending mid-session.READYwithgetEnded()already true (noOFFLINE), read synchronously as before. Covers the ~3 min worst case staleness of the served config (2 min cron + 60s edge cache).Unchanged: the CrazyGames/desktop-shell exclusion, the ad-free close and its per-day persistence, minimize, drag/corner snapping, flick-to-dismiss, and staying up through the lobby wait.
Two side benefits:
Deploy order
Merge openfrontio/infra#488 first. If this ships first the API still serves the unfiltered list; the panel then embeds
channels[0]and hides it if it turns out to be offline, so nothing breaks, but the multi-channel walk is gone until infra lands (a lower-priority live channel would not be picked up).Tests
npx vitest run tests/client— 65 files, 777 passed.New
tests/client/components/FeaturedStreamPanel.test.tsmounts the component with a fake SDK and asserts the regression directly:Twitch.Playeris constructed when the API reports nobody live (and none when the feature is off)Plus
channelToEmbedunit tests for the config-to-channel decision.