fix(stream): one settled client state, and a CSP that matches Stream's real origins - #1087
Conversation
The chat and video clients were two independent useStates set by two async connects that race. The element in the provider's wrapper slot therefore changed TYPE between renders — children, then <StreamVideo>, then <Chat>, in whichever order the sockets settled — and React cannot reconcile a type change in place, so it remounted the entire dashboard subtree each time. An in-flight join was torn down underneath the user. Both connects now resolve to their client instead of setting state, and Promise.allSettled commits the pair at once, so the tree shape is a pure function of one settled value and changes exactly once in the normal case. allSettled also stops a chat failure from discarding a good video client, which Promise.all did by rejecting on the first failure. Co-authored-by: Cursor <cursoragent@cursor.com>
Every directive allow-listed *.getstream.io, which is Stream's marketing domain. At runtime the SDKs talk to *.stream-io-api.com (REST and both websockets), *.stream-io-video.com (the edge-latency hint fetched before a call, then the SFU) and *.stream-io-cdn.com (recordings, attachments). None of those match, so a dashboard load filed violations for traffic the app cannot work without — and would have broken outright under ENABLE_CSP_ENFORCE. Confirmed against a deploy-preview network log. /api/csp-report was on spamLimiter's 5/hr, a budget sized for a human filing a support ticket. Browsers emit a report per violated directive per navigation, so a few page loads exhausted the hour and the rest were dropped — the report-only rollout was blind exactly when it had something to say. It now has its own limiter with a ceiling sized for browser-generated volume. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
✅ Deploy Preview for familiarise ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
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 |
The provider doc described the two-independent-useState pattern that caused the dashboard remount, including a render tree nested in the opposite order to the code, a spinner gate removed in #248, and an unmount disconnect the provider deliberately does not do. Someone following it would have rebuilt the bug. The security-headers doc asserted that *.getstream.io covers Stream's traffic. It does not, and that claim is why the allow-list was wrong; the three real domains are now named with what each carries. Adds the two symptoms to the Stream troubleshooting guide, since that is where anyone hitting them will look first. Co-authored-by: Cursor <cursoragent@cursor.com>
|
1 participant
Three defects on the Stream surface, found while investigating the React #310 crash that was blocking #1067 / #1083 / #1069.
1. The provider remounted the whole dashboard
StreamProviderImplheld the chat and video clients in two independentuseStates, each set by its own async connect. Those connects race, so the element in the wrapper slot changed type between renders —children, then<StreamVideo>, then<Chat>— in whichever order the sockets happened to settle. React cannot reconcile a type change in place: it unmounts and remounts the entire subtree, which here is the whole dashboard. That is the mechanism behind "I pressed Join and nothing happened, so I pressed it ten times": an in-flight join was being torn down underneath the user.Both connects now resolve to their client instead of setting state, and
Promise.allSettledcommits the pair in a single state update, so the tree shape is a pure function of one settled value and changes exactly once in the normal case.allSettledrather thanallis also a fix in its own right:allrejects on the first failure and abandons the other promise's result, so a chat failure threw away a perfectly good video client.2. The CSP never allowed the domains Stream actually uses
Every directive allow-listed
*.getstream.io, which is Stream's marketing and docs domain. At runtime the SDKs talk to three others:*.stream-io-api.comwss://video.stream-io-api.com)*.stream-io-video.com*.stream-io-cdn.comNone of those match
*.getstream.io, so every dashboard load was filing violation reports for traffic the app cannot function without — and video would have broken outright the momentENABLE_CSP_ENFORCE=truewas set. Confirmed against a real network log on a deploy preview, not inferred from docs.*.getstream.iostays: Stream still serves some static assets there, and removing it is a separate unobserved risk.Deliberately not added:
worker-src. Nothing in this app constructs aWorker, and the Stream add-ons that would needblob:workers andwasm-unsafe-eval(background filters, noise cancellation) are not installed. Noted in the comment as the directive that will break first if they ever are.3.
/api/csp-reportwas rate-limited into uselessnessIt was sharing
spamLimiter, which is 5 per hour — a budget sized for a human deciding to file a support ticket. Browsers emit a CSP report per violated directive per navigation, so a single person opening a few dashboard pages exhausted the hour's quota in seconds and everything after that came back429. The report-only rollout was blind in exactly the situation it exists to observe.It now has its own limiter sized for browser-generated volume, still bounded against a hostile poster. Reports are logged rather than stored, so a generous ceiling costs log volume, not writes.
What this does not claim
The provider change is a strong candidate for the #310 root cause — the crash lands inside Stream SDK internals whose hook count varies with whether a client is present, and this removes the null-to-present transition that drives it. I have not yet confirmed the crash is gone, so I am not closing the #310 issue on this PR.
Test plan
tsc --noEmitclean from a cold.tsbuildinfoeslintclean on all four changed files, zero warningsprettier --checkclean/api/csp-reportreturns 204 rather than 429Made with Cursor