Fix race condition: Atomic stream finish with message save#224
Fix race condition: Atomic stream finish with message save#224sethconvex merged 2 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
3122e0e to
fa9cadc
Compare
commit: |
src/client/start.ts
Outdated
| messages: serialized.messages, | ||
| embeddings, | ||
| failPendingSteps: false, | ||
| finishStreamId: finishStreamId as any, // optional stream to finish atomically |
src/client/streaming.ts
Outdated
| public async getOrCreateStreamId(): Promise<string> { | ||
| await this.getStreamId(); | ||
| return this.streamId!; |
There was a problem hiding this comment.
let's just have getStreamId() return this.streamId
|
|
||
| // If we deferred the final step save, do it now with atomic stream finish | ||
| if (pendingFinalStep && streamer) { | ||
| const finishStreamId = await streamer.getOrCreateStreamId(); |
There was a problem hiding this comment.
not sure why the streamId wouldn't exist yet - but i guess better than streamer.streamId!?
| if (args.finishStreamId) { | ||
| await finishHandler(ctx, { streamId: args.finishStreamId }); |
There was a problem hiding this comment.
| if (args.finishStreamId) { | |
| await finishHandler(ctx, { streamId: args.finishStreamId }); | |
| if (finishStreamId) { | |
| await finishHandler(ctx, { streamId: finishStreamId }); |
a19d166 to
c209e89
Compare
462e158 to
cf75fc1
Compare
|
Addressed review feedback:
|
cf75fc1 to
b6894b8
Compare
c209e89 to
0eb6f6a
Compare
b6894b8 to
34d83da
Compare
17cd6b6 to
b25ec92
Compare
34d83da to
15c0ddf
Compare
Merge activity
|
Fixes the intermittent crash "TypeError: The stream is not in a state that permits enqueue" when using saveStreamDeltas with tool calls. The bug occurs when: 1. Tool executes successfully -> onStepFinish callback saves to DB 2. Stream finishes reading -> AI SDK closes stream via attemptClose() 3. Delta-save mutation still in flight -> tries to write to closed stream 4. Crash: Cannot enqueue chunk on closed stream This fix (from commit 87e3657 on seth/fix-193 branch) implements atomic stream finish by: - Deferring final step save when streaming is enabled - Saving step atomically with stream finish in same mutation - Stream stays open until database confirms all saves complete Changes: - streamText.ts: Track pendingFinalStep, defer save, atomic finish - streaming.ts: Add markFinishedExternally() and getOrCreateStreamId() - start.ts: Add finishStreamId parameter to save() - messages.ts: Atomically finish stream with message save Resolves issue #181 Addresses user report: #217 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
15c0ddf to
5264d40
Compare

Summary
Fixes the intermittent crash "TypeError: The stream is not in a state that permits enqueue" when using
saveStreamDeltaswith tool calls (issue #181).The bug occurs when:
onStepFinishcallback saves to DBattemptClose()The fix implements atomic stream finish by:
Cherry-picked from #217 (commit
5cd4c5e). Resolves #181.Test plan
npm run buildpassesnpm testpasses🤖 Generated with Claude Code