-
Notifications
You must be signed in to change notification settings - Fork 13
useStream: streamStarted ref never resets — HTTP POST only fires once per mount #19
Description
Bug
useStream cannot drive more than one stream per component lifetime. After the first stream completes, subsequent calls with a new streamId silently skip the HTTP POST and fall back to the database query, losing the real-time streaming UX.
Root cause
Three related issues in src/react/index.ts:
1. streamStarted ref is set but never cleared
const streamStarted = useRef(false);
useEffect(() => {
if (driven && streamId && !streamStarted.current) {
// ...start stream...
return () => {
streamStarted.current = true; // ← set on cleanup, never reset
};
}
}, [driven, streamUrl, streamId, ...]);When streamId changes (user sends a second message), the effect re-runs. The cleanup from the previous run sets streamStarted.current = true. The new iteration checks !streamStarted.current → false → skips the fetch entirely.
The intent was a React Strict Mode double-mount guard, but it permanently blocks all future streams.
2. streamBody is never reset between streams
When streamId changes there is no setStreamBody("") call, so text from stream N would concatenate with stream N+1 if the fetch did fire.
3. No AbortController — stale streams leak
If the component re-renders with a new streamId while the previous fetch is mid-read, the old reader.read() loop continues calling setStreamBody, mixing text across streams. There's no way to cancel the in-flight request.
Reproduction
- Mount a component using
useStreamwithdriven=true - Complete one stream (wait for the HTTP response to finish)
- Change
streamIdto a new value (e.g., user sends another message) - Observe: no HTTP POST is made; the hook falls through to
usePersistence=true
Proposed fix
Replace the streamStarted ref with a ref tracking the active streamId, reset streamBody/streamEnded on new streams, and use an AbortController for cleanup:
const [streamBody, setStreamBody] = useState("");
const [streamEnded, setStreamEnded] = useState<boolean | null>(null);
const activeStreamRef = useRef<StreamId | undefined>(undefined);
useEffect(() => {
if (!driven || !streamId) return;
if (streamId === activeStreamRef.current) return; // Strict Mode guard
activeStreamRef.current = streamId;
setStreamBody("");
setStreamEnded(null);
const controller = new AbortController();
void (async () => {
try {
const response = await fetch(streamUrl, {
method: "POST",
body: JSON.stringify({ streamId }),
headers: {
"Content-Type": "application/json",
...opts?.headers,
...(opts?.authToken ? { Authorization: `Bearer ${opts.authToken}` } : {}),
},
signal: controller.signal,
});
if (response.status === 205 || !response.ok || !response.body) {
setStreamEnded(false);
return;
}
const reader = response.body.getReader();
const decoder = new TextDecoder();
for (;;) {
const { done, value } = await reader.read();
const text = decoder.decode(value, { stream: !done });
if (text) setStreamBody((prev) => prev + text);
if (done) {
setStreamEnded(true);
return;
}
}
} catch (e) {
if (!controller.signal.aborted) {
console.error("Error reading stream", e);
setStreamEnded(false);
}
}
})();
return () => controller.abort();
}, [driven, streamId, streamUrl, opts?.authToken]);This also fixes a minor perf issue: the current code allocates a new TextDecoder() per chunk inside the while loop.
Versions
@convex-dev/persistent-text-streaming: 0.3.0
Happy to open a PR if this approach looks right.