fix(cloudflare-workers): fix aborting sse streams #1280

NathanFlurry · 2025-09-18T06:04:48Z

No description provided.

vercel · 2025-09-18T06:04:52Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Sep 18, 2025 6:07am

claude · 2025-09-18T06:05:02Z

Claude finished @NathanFlurry's task —— View job

PR Review: Fix Aborting SSE Streams

Review Checklist

Read repository guidelines (CLAUDE.md)
Analyze changed files and understand the fix
Review code quality and best practices
Check for potential bugs or issues
Evaluate performance considerations
Assess security concerns
Review test coverage
Provide comprehensive feedback

Summary

This PR fixes a critical issue with aborting SSE (Server-Sent Events) streams in Cloudflare Workers. The main changes focus on properly handling SSE stream lifecycle, implementing ping mechanisms for connection liveness detection, and improving cleanup procedures.

Code Quality & Best Practices

✅ Good

Clear separation of concerns: The SSE connection logic is well-structured with proper error handling
Consistent logging: Uses structured logging with appropriate log levels (debug, error)
Promise-based async handling: Proper use of Promise.withResolvers() for cleanup coordination
Export constants: SSE_PING_INTERVAL is properly exported and used consistently

🔍 Areas for Improvement

Debug Logging in Production Code

File: packages/rivetkit/src/actor/router-endpoints.ts:368, 456

console.log("=== stream open ===");
console.log("=== stream error ===");

Issue: Direct console.log statements should be removed or replaced with proper logging:

actor.rLog.debug("sse stream opened");
// or in error handler:
loggerWithoutContext().error({ msg: "sse stream error", error: err });

Commented Dead Code

File: packages/rivetkit/src/actor/router-endpoints.ts:417-420

// // HACK: Will throw if not configured
// try {
// 	c.executionCtx.waitUntil(abortResolver.promise);
// } catch {}

Issue: Remove commented code or provide clear documentation about why it's commented out.

Bug Analysis

✅ Fixes Implemented

SSE Abort Handling: Properly handles stream abort events with cleanup
Ping Mechanism: Implements regular pings to detect closed connections (critical for Cloudflare Workers)
Connection Cleanup: Ensures proper cleanup of streams and connections on abort/error

🚨 Potential Issues

Ping Event Filtering

File: packages/rivetkit/src/client/actor-conn.ts:341-342

// Ignore pings
if (ev.type === "ping") return;

Issue: This checks ev.type but SSE events have an event property, not type. Should be:

if (ev.event === "ping") return;

Infinite Loop Risk

File: packages/rivetkit/src/actor/router-endpoints.ts:425-437

while (true) {
    if (stream.closed || stream.aborted) {
        // break;
    }
    await stream.writeSSE({ event: "ping", data: "" });
    await stream.sleep(SSE_PING_INTERVAL);
}

Issue: If stream.writeSSE() throws an exception, the while loop could become infinite. Consider wrapping in try-catch:

while (true) {
    if (stream.closed || stream.aborted) break;
    
    try {
        await stream.writeSSE({ event: "ping", data: "" });
        await stream.sleep(SSE_PING_INTERVAL);
    } catch (error) {
        actor?.rLog.warn({ msg: "ping failed, stream likely closed", error });
        break;
    }
}

Performance Considerations

✅ Good

Efficient ping interval: 1-second intervals are reasonable for connection liveness
Proper cleanup: Resources are cleaned up on disconnect/error
Non-blocking operations: Async operations don't block the main thread

⚠️ Considerations

Memory usage: Each SSE connection maintains a ping loop - monitor resource usage under high connection counts
Network overhead: Ping messages every second add minimal but constant network traffic

Security Assessment

✅ No Major Concerns

Input validation preserved from existing code
No new attack vectors introduced
Proper cleanup prevents resource leaks

Test Coverage

✅ Good Test Updates

Added proper imports for SSE_PING_INTERVAL
Improved test reliability with proper async waiting patterns
Used vi.waitFor() for better test stability in Cloudflare Workers environment

📝 Test Recommendations

Consider adding specific tests for:

SSE ping mechanism behavior
Stream abort handling scenarios
Connection cleanup on various failure modes

Minor Issues

Version Bump: Wrangler version updated from ^4.22.0 to ^4.37.1 - ensure this doesn't introduce breaking changes
Removed Debug Logs: Removed console.log("project dir", projectDir) - good cleanup

Overall Assessment

This is a solid fix that addresses the core SSE streaming issues in Cloudflare Workers. The implementation is well-structured and follows good practices. The main concerns are:

Replace console.log with proper logging
Fix the ping event type checking bug
Add error handling to the ping loop
Clean up commented code

Recommendation: ✅ Approve with minor fixes

The core functionality is sound, but addressing the ping event filtering bug is important for proper operation.

NathanFlurry · 2025-09-18T06:05:04Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

merge queue - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

pkg-pr-new · 2025-09-18T06:06:32Z

More templates

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/actor@1280

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/cloudflare-workers@1280

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/core@1280

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/db@1280

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/framework-base@1280

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/next-js@1280

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/react@1280

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivetkit@1280

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivetkit/@rivetkit/sql-loader@1280

commit: f5fa331

graphite-app · 2025-09-24T06:06:46Z

Merge activity

Sep 24, 6:06 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Sep 24, 6:07 AM UTC: CI is running for this pull request on a draft pull request (#1294) due to your merge queue CI optimization settings.
Sep 24, 6:08 AM UTC: Merged by the Graphite merge queue via draft PR: #1294.

fix(cloudflare-workers): fix aborting sse streams

f5fa331

NathanFlurry mentioned this pull request Sep 18, 2025

chore: remove unused FakeEventSource #1279

Closed

vercel bot had a problem deploying to Preview September 18, 2025 06:07 Failure

This was referenced Sep 18, 2025

chore(core): fix test for sending message to specific connection sse race condition #1286

Closed

chore(core): fix missing transport for inline tests #1287

Closed

jog1t mentioned this pull request Sep 18, 2025

09-16-fix_next-js_fix_example_for_next-js #1250

Closed

MasterPtato mentioned this pull request Sep 18, 2025

feat: serverless #1288

Closed

graphite-app bot pushed a commit that referenced this pull request Sep 24, 2025

fix(cloudflare-workers): fix aborting sse streams (#1280)

610accf

graphite-app bot closed this Sep 24, 2025

graphite-app bot deleted the 09-17-fix_cloudflare-workers_fix_aborting_sse_streams branch September 24, 2025 06:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cloudflare-workers): fix aborting sse streams #1280

fix(cloudflare-workers): fix aborting sse streams #1280

Uh oh!

NathanFlurry commented Sep 18, 2025

Uh oh!

vercel bot commented Sep 18, 2025 •

edited

Loading

Uh oh!

claude bot commented Sep 18, 2025 •

edited

Loading

Uh oh!

NathanFlurry commented Sep 18, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Sep 18, 2025

Uh oh!

graphite-app bot commented Sep 24, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(cloudflare-workers): fix aborting sse streams #1280

fix(cloudflare-workers): fix aborting sse streams #1280

Uh oh!

Conversation

NathanFlurry commented Sep 18, 2025

Uh oh!

vercel bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Fix Aborting SSE Streams

Review Checklist

Summary

Code Quality & Best Practices

✅ Good

🔍 Areas for Improvement

Debug Logging in Production Code

Commented Dead Code

Bug Analysis

✅ Fixes Implemented

🚨 Potential Issues

Ping Event Filtering

Infinite Loop Risk

Performance Considerations

✅ Good

⚠️ Considerations

Security Assessment

✅ No Major Concerns

Test Coverage

✅ Good Test Updates

📝 Test Recommendations

Minor Issues

Overall Assessment

Uh oh!

NathanFlurry commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

pkg-pr-new bot commented Sep 18, 2025

Uh oh!

graphite-app bot commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Sep 18, 2025 •

edited

Loading

claude bot commented Sep 18, 2025 •

edited

Loading

NathanFlurry commented Sep 18, 2025 •

edited

Loading

graphite-app bot commented Sep 24, 2025 •

edited

Loading