From 8516bd4569d7382d789cc3827756dabd57ac3f81 Mon Sep 17 00:00:00 2001 From: Kaustav Ghosh <44238657+teetangh@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:12:53 +0530 Subject: [PATCH 1/3] fix(stream): commit both clients in one settled state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 , then , 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 --- providers/StreamProviderImpl.tsx | 135 ++++++++++++++++++------------- 1 file changed, 81 insertions(+), 54 deletions(-) diff --git a/providers/StreamProviderImpl.tsx b/providers/StreamProviderImpl.tsx index 3d8c5c609..a05d093a8 100644 --- a/providers/StreamProviderImpl.tsx +++ b/providers/StreamProviderImpl.tsx @@ -10,12 +10,7 @@ // dynamic()-wrapping a component defined in the same file does not code-split, // since its static imports stay in the parent chunk. See StreamProvider.tsx. -import { - useCallback, - useEffect, - useState, - useRef, -} from "react"; +import { useCallback, useEffect, useState, useRef } from "react"; import { StreamChat } from "stream-chat"; import { Chat } from "stream-chat-react"; import { StreamVideo, StreamVideoClient } from "@stream-io/video-react-sdk"; @@ -58,18 +53,30 @@ const clientSyncCompletedUsers = new Set(); const apiKey = process.env.NEXT_PUBLIC_STREAM_API_KEY; +/** + * The two clients as ONE value, deliberately. Held separately they were set by + * two async connects that race, so the element in the wrapper slot below + * changed TYPE between renders — `children`, then ``, then + * ``, 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 remount storm behind + * "I pressed Join ten times" — an in-flight join was torn down under the user. + * + * `null` means "not settled yet", which is distinct from a settled result whose + * `chat` or `video` is null because that connect failed. + */ +interface SettledStreamClients { + chat: StreamChat | null; + video: StreamVideoClient | null; +} + const StreamProviderImpl = ({ children, userId, enableChat = true, enableVideo = true, }: StreamProviderProps) => { - // Connection states - always initialize to null/false - // Let the connection functions handle global client detection - const [chatClient, setChatClient] = useState(null); - const [videoClient, setVideoClient] = useState( - null, - ); + const [clients, setClients] = useState(null); const [chatConnected, setChatConnected] = useState(false); const [videoConnected, setVideoConnected] = useState(false); const [isConnecting, setIsConnecting] = useState(false); @@ -144,17 +151,20 @@ const StreamProviderImpl = ({ return Math.min(1000 * Math.pow(2, attempt), 30000); // Max 30 seconds }, []); + // connectChat/connectVideo RESOLVE to their client (or null) instead of each + // setting its own state, so the caller can commit both at once and the tree + // changes shape a single time. See SettledStreamClients. const connectChat = useCallback(async () => { - if (!enableChat || !userDetails || !apiKey) return; + if (!enableChat || !userDetails || !apiKey) return null; // Check if we already have a global client for this user - adopt it - if (getCurrentStreamUserId() === userDetails.id && getGlobalChatClient()) { + const adoptable = getGlobalChatClient(); + if (getCurrentStreamUserId() === userDetails.id && adoptable) { streamLogger.debug("Adopting existing chat client", { userId: userDetails.id, }); - setChatClient(getGlobalChatClient()); setChatConnected(true); - return; + return adoptable; } // Prevent concurrent connectUser calls (e.g. connectVideo re-render race) @@ -162,7 +172,7 @@ const StreamProviderImpl = ({ streamLogger.debug("Chat connection already in progress, skipping", { userId: userDetails.id, }); - return; + return getGlobalChatClient(); } isChatConnectingRef.current = true; @@ -182,9 +192,8 @@ const StreamProviderImpl = ({ }); setGlobalChatClient(client); setCurrentStreamUserId(userDetails.id); - setChatClient(client); setChatConnected(true); - return; + return client; } // Ensure user exists in Stream's database (only if not synced before) @@ -218,7 +227,6 @@ const StreamProviderImpl = ({ setGlobalChatClient(client); setCurrentStreamUserId(userDetails.id); - setChatClient(client); setChatConnected(true); // Initial channel sync — once per user per browser session. @@ -261,6 +269,7 @@ const StreamProviderImpl = ({ streamLogger.info("Chat connection established", { userId: userDetails.id, }); + return client; } catch (error) { streamLogger.warn("Chat connection failed (will retry)", { userId: userDetails.id, @@ -273,16 +282,16 @@ const StreamProviderImpl = ({ }, [enableChat, userDetails, getCachedToken]); const connectVideo = useCallback(async () => { - if (!enableVideo || !userDetails || !apiKey) return; + if (!enableVideo || !userDetails || !apiKey) return null; // Check if we already have a global client for this user - adopt it - if (getCurrentStreamUserId() === userDetails.id && getGlobalVideoClient()) { + const adoptable = getGlobalVideoClient(); + if (getCurrentStreamUserId() === userDetails.id && adoptable) { streamLogger.debug("Adopting existing video client", { userId: userDetails.id, }); - setVideoClient(getGlobalVideoClient()); setVideoConnected(true); - return; + return adoptable; } try { @@ -304,11 +313,11 @@ const StreamProviderImpl = ({ setGlobalVideoClient(client); setCurrentStreamUserId(userDetails.id); - setVideoClient(client); setVideoConnected(true); streamLogger.info("Video connection established", { userId: userDetails.id, }); + return client; } catch (error) { streamLogger.error("Video connection failed", error, { userId: userDetails.id, @@ -326,12 +335,25 @@ const StreamProviderImpl = ({ setError(null); try { - const promises = []; - // Always try to connect - the functions will handle global client detection - if (enableChat) promises.push(connectChat()); - if (enableVideo) promises.push(connectVideo()); + // allSettled, not all: `all` rejects on the first failure and abandons the + // other client's result, so a chat failure discarded a perfectly good + // video client. Both outcomes are now committed together, which is also + // what keeps the tree from changing shape twice. + const [chatResult, videoResult] = await Promise.allSettled([ + connectChat(), + connectVideo(), + ]); + + setClients({ + chat: chatResult.status === "fulfilled" ? chatResult.value : null, + video: videoResult.status === "fulfilled" ? videoResult.value : null, + }); + + const failure = [chatResult, videoResult].find( + (result) => result.status === "rejected", + ); + if (failure?.status === "rejected") throw failure.reason; - await Promise.all(promises); connectionAttemptsRef.current = 0; // Reset on success setRetryCount(0); } catch (error) { @@ -362,15 +384,10 @@ const StreamProviderImpl = ({ } finally { setIsConnecting(false); } - }, [ - isLoading, - userDetails, - enableChat, - enableVideo, - connectChat, - connectVideo, - getRetryDelay, - ]); + // enableChat/enableVideo are not read here any more: each connect returns + // null when its own flag is off, and both are already deps of those + // callbacks, so listing them again only invalidates this one needlessly. + }, [isLoading, userDetails, connectChat, connectVideo, getRetryDelay]); const retryConnection = useCallback(() => { connectionAttemptsRef.current = 0; @@ -399,11 +416,11 @@ const StreamProviderImpl = ({ const run = () => { // Check if user changed - if so, disconnect old user first. - // Use disconnectStreamClients (global refs) rather than the local - // disconnect() here: on a fresh remount for a different user, local - // chatClient/videoClient state is null while the GLOBAL clients still - // point at the PREVIOUS user. local disconnect() would no-op and leak - // the prior user's connection, which the new connect would then adopt. + // Use disconnectStreamClients (global refs) rather than any local + // teardown: on a fresh remount for a different user the local `clients` + // state is null while the GLOBAL clients still point at the PREVIOUS + // user, so a local teardown would no-op and leak the prior user's + // connection, which the new connect would then adopt. if ( getCurrentStreamUserId() && getCurrentStreamUserId() !== userDetails.id @@ -420,9 +437,12 @@ const StreamProviderImpl = ({ .catch((err) => { // Never block the new user's connect on a prior-user disconnect // failure; disconnectStreamClients already clears global refs. - streamLogger.warn("Prior-user disconnect failed, connecting anyway", { - error: err, - }); + streamLogger.warn( + "Prior-user disconnect failed, connecting anyway", + { + error: err, + }, + ); }) .finally(() => { connectServices(); @@ -488,17 +508,24 @@ const StreamProviderImpl = ({ // render children immediately; the Stream context providers wrap them once the // clients are ready, and the video/chat consumers already guard a null client. - // Render providers + // The wrapper set is derived from ONE settled value and nested in a fixed + // order — Chat outside, StreamVideo inside — so the shape here is a pure + // function of `clients` rather than of which socket won the race. In the + // normal case that means exactly one shape change for the whole session: + // unwrapped while connecting, then wrapped once both connects settle. + // + // A connect that genuinely FAILS still costs a second change if a later retry + // succeeds. That is accepted: it is a degraded path, the retry loop is capped + // at 5 attempts, and withholding the client that did connect would break the + // sidebar's chat-unread badge on every route (#248). let content = children; - // Wrap with video provider if enabled and connected - if (enableVideo && videoClient) { - content = {content}; + if (clients?.video) { + content = {content}; } - // Wrap with chat provider if enabled and connected - if (enableChat && chatClient) { - content = {content}; + if (clients?.chat) { + content = {content}; } // The connection-failed banner renders alongside children (not in place of From 1191bd577a7b00ebe42bfc90d804a5132c795d4d Mon Sep 17 00:00:00 2001 From: Kaustav Ghosh <44238657+teetangh@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:12:53 +0530 Subject: [PATCH 2/3] fix(csp): allow Stream's real origins and stop 429ing violation reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- app/api/csp-report/route.ts | 11 ++++++----- lib/rate-limit.ts | 17 +++++++++++++++++ next.config.mjs | 29 ++++++++++++++++++++++++++--- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/app/api/csp-report/route.ts b/app/api/csp-report/route.ts index f88e7671b..1e08e352b 100644 --- a/app/api/csp-report/route.ts +++ b/app/api/csp-report/route.ts @@ -6,9 +6,10 @@ * we accept either that or plain JSON since the spec is in flux. * * The route does NOT require auth — anyone can post a CSP report (the - * browser is the originator, not the user). We rate-limit via the - * existing `spamLimiter` keyed on IP to keep a hostile receiver from - * flooding our logs. + * browser is the originator, not the user). We rate-limit on IP to keep a + * hostile poster from flooding our logs, but with a ceiling sized for + * browser-generated traffic: see cspReportLimiter for why the old + * spamLimiter budget made this endpoint useless. * * The report is logged as a structured event (`event: "csp_violation"`) * so an operator scanning `console` output during the report-only @@ -19,7 +20,7 @@ */ import { NextResponse, type NextRequest } from "next/server"; -import { applyRateLimit, spamLimiter } from "@/lib/rate-limit"; +import { applyRateLimit, cspReportLimiter } from "@/lib/rate-limit"; export async function POST(req: NextRequest) { const ip = @@ -27,7 +28,7 @@ export async function POST(req: NextRequest) { req.headers.get("x-real-ip") ?? "unknown"; - const rl = await applyRateLimit(spamLimiter, `csp:${ip}`); + const rl = await applyRateLimit(cspReportLimiter, `csp:${ip}`); if (rl) return rl; const body = await req.json().catch(() => null); diff --git a/lib/rate-limit.ts b/lib/rate-limit.ts index f35e93e04..01a9a9874 100644 --- a/lib/rate-limit.ts +++ b/lib/rate-limit.ts @@ -8,6 +8,7 @@ * - waitlistLimiter: 3/hr per IP — POST /api/waitlist (newsletter signup spam) * - referralApplyLimiter: 3/24h per user — POST /api/referrals/apply (farming) * - spamLimiter: 5/hr per user — support-tickets, feedbacks, reviews, report + * - cspReportLimiter: 120/min per IP — POST /api/csp-report (browser-generated) * - trialRequestLimiter: 3/24h per user — POST /api/trials (spam prevention) * - requestApprovalLimiter: 10/hr per user — POST /api/slots/request-for-approval * - searchLimiter: 60/min per IP — GET /api/user/consultants, /api/consultants/search @@ -69,6 +70,22 @@ export const referralApplyLimiter = makeLimiter(3, "24 h", "rl:referral-apply"); /** 5 per hour — support-tickets, feedbacks, reviews, report (scope key by route) */ export const spamLimiter = makeLimiter(5, "1 h", "rl:spam"); +/** + * 120 per minute per IP — POST /api/csp-report. + * + * Was on spamLimiter's 5/hr, which is sized for a HUMAN deciding to file a + * support ticket. A CSP report is emitted by the browser, unprompted, once per + * violated directive per page load — so one person opening a few dashboard + * pages exhausted the hour's quota in seconds and every report after that was + * dropped with a 429. The report-only rollout was therefore blind in exactly + * the situation it exists to observe: a directive drifting on a real user. + * + * Sized for a page that violates a handful of directives on every navigation, + * with headroom, while still capping a hostile poster. Reports are logged, not + * stored, so the cost of a generous ceiling is log volume rather than writes. + */ +export const cspReportLimiter = makeLimiter(120, "1 m", "rl:csp-report"); + /** 3 per 24 hours — POST /api/trials (prevents flooding consultant inboxes) */ export const trialRequestLimiter = makeLimiter(3, "24 h", "rl:trial-request"); diff --git a/next.config.mjs b/next.config.mjs index 964c6f50b..9b28ad5c2 100644 --- a/next.config.mjs +++ b/next.config.mjs @@ -32,14 +32,37 @@ const withBundleAnalyzer = * added here AND in the matching client. * - `frame-src` allows Razorpay's + Stripe's checkout iframes. * - `media-src` is the load-bearing entry for Stream call audio / - * video / recording playback (`blob:` + getstream.io). + * video / recording playback. + * + * Stream.io does NOT run on getstream.io at runtime + * ------------------------------------------------- + * `getstream.io` is the marketing/docs domain. The SDKs actually talk to + * three separate domains, none of which `*.getstream.io` matches, so every + * dashboard load was filing violation reports: + * + * - `*.stream-io-api.com` REST + the chat/video websockets + * (`wss://video.stream-io-api.com`) + * - `*.stream-io-video.com` the edge-latency hint (`hint.…`) the client + * fetches BEFORE a call to pick an SFU, then the + * SFU edge itself + * - `*.stream-io-cdn.com` recordings and chat attachments + * + * Confirmed against a real network log on a deploy preview, not inferred from + * docs. `*.getstream.io` stays because Stream still serves some static assets + * there and removing it is a separate, unobserved risk. + * + * Not added, deliberately: `worker-src`. Nothing in this app constructs a + * Worker and the Stream background-filter/noise-cancellation add-ons (the + * things that would need `blob:` workers and `wasm-unsafe-eval`) are not + * installed. If those are ever enabled, this is the directive that will break + * first, and `default-src 'self'` is what it will fall back to. */ const CSP_DIRECTIVES = [ "default-src 'self'", "script-src 'self' 'unsafe-inline' 'unsafe-eval' https://checkout.razorpay.com https://js.stripe.com https://*.sentry.io https://*.getstream.io https://*.supabase.co", - "connect-src 'self' https://*.getstream.io wss://*.getstream.io https://*.supabase.co https://*.upstash.io https://api.razorpay.com https://api.stripe.com https://*.sentry.io https://api.resend.com https://*.novu.co wss://*.novu.co", + "connect-src 'self' https://*.getstream.io wss://*.getstream.io https://*.stream-io-api.com wss://*.stream-io-api.com https://*.stream-io-video.com wss://*.stream-io-video.com https://*.stream-io-cdn.com https://*.supabase.co https://*.upstash.io https://api.razorpay.com https://api.stripe.com https://*.sentry.io https://api.resend.com https://*.novu.co wss://*.novu.co", "img-src 'self' data: https: blob:", - "media-src 'self' blob: https://*.getstream.io", + "media-src 'self' blob: https://*.getstream.io https://*.stream-io-cdn.com https://*.stream-io-api.com", "style-src 'self' 'unsafe-inline'", "frame-src 'self' https://checkout.razorpay.com https://js.stripe.com https://hooks.stripe.com", "font-src 'self' data:", From 7dc80814c06e6e6a10855145abe3c9c7153afad2 Mon Sep 17 00:00:00 2001 From: Kaustav Ghosh <44238657+teetangh@users.noreply.github.com> Date: Sat, 1 Aug 2026 17:28:49 +0530 Subject: [PATCH 3/3] docs(stream): correct the provider, CSP and troubleshooting docs 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 --- .../05-security-headers.md | 24 ++- docs/stream/03-provider-authentication.md | 195 ++++++++++-------- docs/stream/troubleshooting.md | 39 ++++ 3 files changed, 173 insertions(+), 85 deletions(-) diff --git a/docs/enterprise/20-iam-and-security/05-security-headers.md b/docs/enterprise/20-iam-and-security/05-security-headers.md index ffa9b0641..c56407f06 100644 --- a/docs/enterprise/20-iam-and-security/05-security-headers.md +++ b/docs/enterprise/20-iam-and-security/05-security-headers.md @@ -47,6 +47,8 @@ flowchart LR LOG --> OP["operator scans during rollout, fixes allow-list"] ``` +**The observation window only works if the reports actually arrive.** `/api/csp-report` originally shared `spamLimiter`, which allows 5 requests per hour — a budget sized for a human deciding to file a support ticket. A browser emits one report per violated directive per navigation, so a single person opening a few dashboard pages exhausted the hour in seconds and every subsequent report was rejected with a `429`. The rollout was therefore blind in precisely the situation it exists to observe. The endpoint now has its own limiter (`cspReportLimiter`) sized for browser-generated volume. If you add a report sink in future, size its limiter by who generates the traffic, not by how much you want to receive. + ## Header inventory (production) All seven production headers, their values, and what each one defends against, are listed in the table below. @@ -71,11 +73,27 @@ and the public marketplace pages alike. Anything outside the directives below will be blocked once `ENABLE_CSP_ENFORCE=true`, so each external origin earns its place by being load-bearing for a real product surface. -The `script-src` directive keeps `'self' 'unsafe-inline' 'unsafe-eval'`, which is non-negotiable until Next.js 16 ships hashed inline runtime chunks. Its external origins are Razorpay's checkout CDN (`https://checkout.razorpay.com`, the payment SDK), Sentry (`https://*.sentry.io`, error reporting), Stream.io (`https://*.getstream.io`, the call widget), and Supabase (`https://*.supabase.co`, storage signed URLs). +The `script-src` directive keeps `'self' 'unsafe-inline' 'unsafe-eval'`, which is non-negotiable until Next.js 16 ships hashed inline runtime chunks. Its external origins are Razorpay's checkout CDN (`https://checkout.razorpay.com`, the payment SDK), Stripe (`https://js.stripe.com`), Sentry (`https://*.sentry.io`, error reporting), Stream.io (`https://*.getstream.io`), and Supabase (`https://*.supabase.co`, storage signed URLs). The Stream entry is inherited rather than observed — the SDK is bundled from npm and self-served, so no `script-src` fetch to Stream was seen in practice. + +The `connect-src` directive governs XHR, fetch, and WebSocket targets. It opens `https://api.razorpay.com` for payments, the three Stream domains described below, `https://*.supabase.co` and `https://*.upstash.io` for storage and Redis, `https://*.sentry.io` for error reporting, `https://api.resend.com` for transactional email, and `https://*.novu.co` plus `wss://*.novu.co` for the notification inbox. + +The `media-src` directive serves Stream.io recording and call audio/video, so it requires `blob:` for local recording playback alongside Stream's CDN and API origins. + +### Stream.io does not run on getstream.io + +This is the mistake the allow-list originally made, and it is worth stating plainly because it is easy to repeat: `getstream.io` is Stream's marketing and documentation domain. No SDK traffic goes there. The clients talk to three unrelated domains, and a CSP host wildcard does not span them: + +| Domain | Carries | +| --- | --- | +| `*.stream-io-api.com` | REST calls and both websockets (`wss://video.stream-io-api.com`, `wss://chat.stream-io-api.com`) | +| `*.stream-io-video.com` | the edge-latency hint (`hint.stream-io-video.com`) the client fetches before a call to choose an SFU, then the SFU edge itself | +| `*.stream-io-cdn.com` | call recordings and chat attachments | + +Because only `*.getstream.io` was listed, every dashboard load filed violation reports for traffic the product cannot function without, and video calling would have failed outright the moment `ENABLE_CSP_ENFORCE=true` was set. The domains above were confirmed against a real browser network log on a deploy preview rather than read off Stream's docs, which is the only way to catch this class of drift. -The `connect-src` directive governs XHR, fetch, and WebSocket targets. It opens `https://api.razorpay.com` for payments, both `wss://*.getstream.io` and `https://*.getstream.io` for Stream call signalling and media, `https://*.supabase.co` and `https://*.upstash.io` for storage and Redis, `https://*.sentry.io` for error reporting, and `https://api.resend.com` for transactional email. +`*.getstream.io` remains in the list: Stream still serves some static assets from it, and dropping it is a separate change with its own unobserved blast radius. -The `media-src` directive serves Stream.io recording and call audio/video, so it requires both `blob:` (local recording playback) and the getstream.io CDN. +`worker-src` is deliberately absent. Nothing in the app constructs a `Worker`, and Stream's background-filter and noise-cancellation add-ons — the features that would need `blob:` workers and `wasm-unsafe-eval` — are not installed. If they are ever adopted, `worker-src` is the directive that breaks first, and it will fall back to `default-src 'self'`. The `frame-src` directive allows Razorpay's checkout iframe; without this entry, payments break the moment CSP is flipped to enforce mode. diff --git a/docs/stream/03-provider-authentication.md b/docs/stream/03-provider-authentication.md index 53d833832..4de4090e4 100644 --- a/docs/stream/03-provider-authentication.md +++ b/docs/stream/03-provider-authentication.md @@ -18,16 +18,33 @@ ### Dual-Client Design Pattern -**File:** `/providers/StreamProvider.tsx` (Lines 1-382) +**File:** `providers/StreamProviderImpl.tsx` (the SDK-free shell lives in `providers/StreamProvider.tsx`) -StreamProvider implements a **dual-client architecture**, managing two independent SDK clients simultaneously: +StreamProvider manages two SDK clients — one for chat, one for video — but holds them in a **single piece of state**: ```typescript -// Two separate client instances -const [chatClient, setChatClient] = useState(null); -const [videoClient, setVideoClient] = useState(null); +// One settled value, not two independent ones +interface SettledStreamClients { + chat: StreamChat | null; + video: StreamVideoClient | null; +} +const [clients, setClients] = useState(null); ``` +`null` means "not settled yet", which is deliberately distinct from a settled result whose `chat` or `video` is `null` because that particular connect failed. + +#### Why one state and not two + +This is the most important thing to understand before changing this file, because the obvious refactor — a `useState` per client — is the bug. + +The provider wraps its children in `` and `` only once a client exists. With two independent states set by two async connects that race, the element occupying that wrapper slot changed **type** between renders: `children`, then ``, then ``, in whichever order the sockets happened to settle. React cannot reconcile a change of element type in place — it unmounts the old tree and mounts a new one — and the subtree here is the entire dashboard. + +The user-visible symptom was a join button that appeared to do nothing: the click started a join, the dashboard remounted underneath it as the second client connected, and the in-flight join was destroyed. People pressed it repeatedly. + +Committing both clients in one `setClients` makes the tree shape a pure function of one value, so it changes exactly once per session: unwrapped while connecting, then wrapped once both connects settle. A connect that genuinely _fails_ can still cost a second change if a later retry succeeds; that is accepted, because withholding the client that did connect would break the sidebar's chat-unread badge on every route (#248). + +The nesting order is fixed — `` outside, `` inside — for the same reason. Order must not depend on arrival order. + #### 1. Chat Client (`StreamChat`) **Package:** `stream-chat` @@ -105,9 +122,8 @@ export default function StreamProvider({ enableChat = true, enableVideo = true, }: StreamProviderProps) { - // Connection state - const [chatClient, setChatClient] = useState(null); - const [videoClient, setVideoClient] = useState(null); + // Connection state — both clients in ONE value, see "Why one state and not two" + const [clients, setClients] = useState(null); const [chatConnected, setChatConnected] = useState(false); const [videoConnected, setVideoConnected] = useState(false); const [isConnecting, setIsConnecting] = useState(false); @@ -132,22 +148,20 @@ export default function StreamProvider({ }; }, [userDetails, isLoading, apiKey]); + // Built up in a fixed order from the single settled value, so the tree shape + // never depends on which socket connected first. + let content = children; + if (clients?.video) { + content = {content}; + } + if (clients?.chat) { + content = {content}; + } + return ( - {enableVideo && videoClient ? ( - - {enableChat && chatClient ? ( - {children} - ) : ( - children - )} - - ) : enableChat && chatClient ? ( - {children} - ) : ( - children - )} + {content} ); @@ -158,7 +172,9 @@ export default function StreamProvider({ - Outermost: `StreamErrorBoundary` (error handling) - Middle: `StreamConnectionContext` (connection state) -- Inner: `StreamVideo` → `Chat` → `children` (SDK providers) +- Inner: `Chat` → `StreamVideo` → `children` (SDK providers) + +Note that the connection _flags_ (`chatConnected`, `videoConnected`, `isConnecting`) are still separate state. That is fine and intentional: they feed the context value, which changes what consumers render but not the shape of the tree above them. --- @@ -475,11 +491,26 @@ const connectServices = useCallback(async () => { setError(null); try { - const promises = []; - if (enableChat && !chatConnected) promises.push(connectChat()); - if (enableVideo && !videoConnected) promises.push(connectVideo()); + // allSettled, not all: `all` rejects on the first failure and abandons the + // other promise's result, so a chat failure threw away a good video client. + // Each connect RESOLVES to its client (or null) rather than setting state, + // so both land in one commit below. + const [chatResult, videoResult] = await Promise.allSettled([ + connectChat(), + connectVideo(), + ]); + + setClients({ + chat: chatResult.status === "fulfilled" ? chatResult.value : null, + video: videoResult.status === "fulfilled" ? videoResult.value : null, + }); + + // Retry is still driven by a rejection, so re-throw the first one. + const failure = [chatResult, videoResult].find( + (result) => result.status === "rejected", + ); + if (failure?.status === "rejected") throw failure.reason; - await Promise.all(promises); // Parallel execution setConnectionAttempts(0); // Reset on success } catch (error) { const errorMessage = @@ -620,25 +651,21 @@ export function ConnectionStatus() { ### Loading States -Provider shows loading UI while connecting (Lines 321-334): +**The provider no longer blocks children behind a spinner.** It used to: while either client was unconnected it returned a spinner instead of `children`, which gated the entire dashboard on two websocket handshakes even on routes with no chat or video UI at all. + +Children now render immediately and the wrappers appear around them once the clients settle. Video consumers already guard a null client, and chat consumers only render on the chat route, underneath ``. + +The only spinner left is in the SDK-free shell (`providers/StreamProvider.tsx`), shown during the brief window where the lazy impl chunk is still downloading: ```typescript -if ( - (enableChat && !chatClient && !error) || - (enableVideo && !videoClient && !error) || - isConnecting -) { - return ( -
-
- {isConnecting && ( -

Connecting to Stream...

- )} -
- ); -} +const LazyStreamProviderImpl = dynamic( + () => import("@/providers/StreamProviderImpl"), + { ssr: false, loading: () => }, +); ``` +If you are tempted to reintroduce a connection gate here, note that it interacts badly with the single-commit design above: gating on "all clients ready" turns one shape change into a shape change plus an unmount, and a permanently failing service would hold the whole dashboard hostage. + ### Error States Error UI shown after max retries (Lines 337-352): @@ -775,37 +802,35 @@ See: [Troubleshooting - Token Expiry Race Condition](./troubleshooting.md#token- ### Cache Invalidation (Lines 276-298) +Disconnection is **not** owned by the provider and does **not** happen on unmount. The clients live in module-level refs in `lib/stream/disconnect.ts` — an SDK-free module so that callers which only need to disconnect (Navbar, UserDropdown, the org/admin/staff layouts) do not statically link the heavy SDK into their bundles. + ```typescript -const disconnect = useCallback(async () => { - const promises = []; - - if (chatClient) { - promises.push( - chatClient.disconnectUser().then(() => { - console.log("Chat client disconnected"); - setChatClient(null); - setChatConnected(false); - }), - ); - } +export async function disconnectStreamClients(): Promise { + const promises: Promise[] = []; + if (globalChatClient) promises.push(globalChatClient.disconnectUser().then(...)); + if (globalVideoClient) promises.push(globalVideoClient.disconnectUser().then(...)); + + // allSettled (not all): a rejected disconnectUser() must NOT skip the global + // teardown below — stale refs after a failed logout would let the next login + // adopt the prior user's connection. + await Promise.allSettled(promises); + + globalChatClient = null; + globalVideoClient = null; + currentUserId = null; + clearAllStreamCaches(); +} +``` - if (videoClient) { - // Note: StreamVideoClient doesn't have explicit disconnect method - setVideoClient(null); - setVideoConnected(false); - } +**Why unmount does not disconnect:** the clients are deliberately kept alive across remounts and tab switches, so navigating between dashboard routes does not pay for a fresh websocket handshake each time. The provider adopts an existing global client for the same user rather than building a new one. - await Promise.all(promises); - setTokenCache({}); // Clear token cache -}, [chatClient, videoClient]); -``` +**The one case that must tear down** is a _different_ user appearing — on a fresh mount the provider's local `clients` state is `null` while the globals still point at the previous user. The provider therefore calls `disconnectStreamClients()` (global refs) rather than any local teardown, which would no-op and leak the prior user's connection for the next connect to adopt. -**Cache cleared on:** +**Disconnect happens on:** -- User logout -- Component unmount -- Manual disconnect -- Connection error (after max retries) +- User logout (the primary path) +- A different user mounting the provider +- Not on unmount, and not on connection error — the retry loop owns that --- @@ -1070,26 +1095,32 @@ const { retryConnection } = useStreamConnection(); ## Advanced Topics -### Cleanup on Unmount (Lines 301-309) +### Cleanup on Unmount -```typescript -useEffect(() => { - if (!isLoading && userDetails && apiKey) { - connectServices(); - } +Unmount cancels _pending work_ and nothing else. It does not disconnect. - return () => { - disconnect(); // Cleanup function - }; -}, [userDetails, isLoading, apiKey]); +```typescript +return () => { + // Cancel the scheduled idle connect and any pending retry so nothing calls + // setState after unmount. + if (idleHandle !== undefined) cancelIdleCallback(idleHandle); + if (timeoutHandle) clearTimeout(timeoutHandle); + if (retryTimeoutRef.current) clearTimeout(retryTimeoutRef.current); + + // Intentionally NOT calling disconnect() here. + // Global clients are reused across component remounts. +}; ``` -**Cleanup Process:** +**Cleanup process:** + +1. Cancel the deferred `requestIdleCallback` connect (#248) +2. Clear the retry backoff timer +3. Leave the clients connected + +The third point is the whole design. Disconnecting here would mean a websocket handshake on every dashboard navigation, and — before the single-commit change described above — the provider remounted on its own during connection anyway, so an unmount-disconnect would have torn down the connection it had just established. -1. Disconnect chat client (`chatClient.disconnectUser()`) -2. Nullify video client (no explicit disconnect method) -3. Clear token cache -4. Reset connection states +Actual disconnection happens on logout, via `disconnectStreamClients()`. See §Disconnection. ### Connection State Persistence diff --git a/docs/stream/troubleshooting.md b/docs/stream/troubleshooting.md index 7e32047ec..abe1386a8 100644 --- a/docs/stream/troubleshooting.md +++ b/docs/stream/troubleshooting.md @@ -392,6 +392,45 @@ curl -X GET "https://chat.stream-io-api.com/health" ## Connection Issues +### Issue: The dashboard flickers or reloads as the page settles, and the first Join click does nothing + +#### Symptoms + +- The dashboard visibly remounts a second or two after load +- Clicking Join appears to do nothing, so the user clicks it several more times +- Component state (open dialogs, scroll position, half-filled forms) resets on its own +- Possibly a React error #310 — "rendered more hooks than during the previous render" — pointing into Stream SDK internals + +#### Cause + +The provider held the chat and video clients in two independent `useState`s. Their connects race, so the element wrapping the dashboard changed *type* between renders (`children` → `` → ``, in socket-arrival order). React cannot reconcile a type change in place, so it remounted the whole subtree — destroying any in-flight join. + +#### Fix + +Both clients are committed in a single `setClients` via `Promise.allSettled`, so the tree shape is a pure function of one settled value. See §Why one state and not two in `docs/stream/03-provider-authentication.md`. + +**If you see this again**, the first thing to check is whether someone has reintroduced a second source of truth for client state, or made the wrapper nesting order depend on which client arrived first. + +### Issue: Video works locally but fails in production, or the console fills with CSP violations + +#### Symptoms + +- `Refused to connect to 'https://hint.stream-io-video.com/…'` or `'wss://video.stream-io-api.com/…'` +- Calls connect locally (where CSP is often not exercised) but not on a deploy preview or production +- `POST /api/csp-report` returning `429` + +#### Cause + +Stream does **not** use `getstream.io` at runtime — that is the marketing domain. The SDKs talk to `*.stream-io-api.com`, `*.stream-io-video.com`, and `*.stream-io-cdn.com`. An allow-list containing only `*.getstream.io` does not match any of them. + +Separately, `/api/csp-report` was rate-limited at 5/hour, so the violation reports that would have revealed this were themselves being dropped. + +#### Fix + +Both are corrected in `next.config.mjs` and `lib/rate-limit.ts`. The full domain breakdown is in `docs/enterprise/20-iam-and-security/05-security-headers.md`. + +**Verify with the browser, not the docs.** This class of drift is only visible in a real network log; Stream's documentation does not enumerate the SFU and hint domains in one place. + ### Issue: "Chat connection failed" #### Symptoms