fix: fix serverless config req, handle serverless sse close

[MasterPtato]

No description provided.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Oct 8, 2025 10:57pm

[claude]

Claude finished @MasterPtato's task —— View job

---

Code Review Analysis

Reviewing PR for code quality, potential bugs, performance considerations, security concerns, and test coverage.

Progress Checklist:

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

---

📋 Review Summary

This PR introduces improvements to serverless configuration handling and enhanced smoke testing. The changes are generally well-implemented but there are several areas for improvement.

✅ Positive Aspects

1. Better Error Handling: The serverless configuration function now has comprehensive error handling with descriptive messages
2. Proper Logging: Follows repository guidelines with structured logging using logger().info/debug/error
3. Sleep Test Enhancement: Adds realistic sleep behavior to smoke tests, improving test coverage for actor lifecycle
4. SSE Close Handling: Implements proper cleanup with stream.onAbort(() => this.shutdown(true))

🔍 Issues & Recommendations

🚨 Critical Issues

1. Potential Type Error - packages/rivetkit/src/registry/mod.ts:255

let dcsRes = await dcsResponse.json() as { datacenters: { name: string }[] };

Issue: Type assertion without validation violates repository guidelines
Risk: Runtime errors if API response format changes
Fix: Use zod for runtime validation or add proper type checking

2. Missing Error Context - packages/rivetkit/src/drivers/engine/actor-driver.ts:414

stream.onAbort(() => this.shutdown(true));

Issue: Immediate shutdown without graceful cleanup or logging
Risk: Potential data loss or resource leaks
Fix: Add logging and consider graceful shutdown:

stream.onAbort(() => {
  logger().warn({ msg: "sse stream aborted, shutting down" });
  this.shutdown(true);
});

⚠️ Medium Priority Issues

3. Hardcoded Values - packages/rivetkit/src/registry/mod.ts:262-269

The serverless configuration uses multiple hardcoded defaults:

• max_runners: 100
• request_lifespan: 15 * 60 (15 minutes)
• slots_per_runner: 1000

Recommendation: Extract to constants or configuration file for better maintainability.

4. Inconsistent Timeout Values - examples/smoke-test/src/smoke-test/spawn-actor.ts:46

await new Promise(res => setTimeout(res, 1000));

Issue: Hardcoded 1-second sleep doesn't align with repository test guidelines
Fix: Per CLAUDE.md test guidelines, use await waitFor(driverTestConfig, 1000) for proper test timing

5. Missing JSDoc - Multiple locations

Public APIs lack JSDoc comments, which violates repository guidelines for public APIs.

🔧 Minor Issues

6. Variable Naming - packages/rivetkit/src/registry/mod.ts:255

let dcsRes = await dcsResponse.json() as { datacenters: { name: string }[] };

Should follow camelCase convention: dcsResponse → datacentersResponse

7. Duplicate Code - examples/smoke-test/scripts/connect.ts:7-36 and examples/smoke-test/scripts/connect.ts:23-35

The counter creation and increment logic is duplicated. Consider extracting to a helper function.

🔒 Security Assessment

• ✅ Authorization Headers: Properly conditionally included based on token presence
• ✅ No Secrets: No hardcoded secrets or sensitive data
• ✅ Error Messages: Don't expose internal implementation details

🚀 Performance Considerations

• ✅ Efficient Promises: Uses Promise.all() appropriately for concurrent operations
• ⚠️ Sequential HTTP Calls: Serverless configuration makes sequential API calls - consider if they can be parallelized
• ✅ Resource Cleanup: Proper WebSocket and actor cleanup patterns

🧪 Test Coverage

Strengths:

• Enhanced smoke test now covers actor sleep/wake cycles
• Tests multiple reconnection scenarios

Gaps:

• No tests for the new serverless configuration function
• Missing error handling tests for malformed API responses
• No integration tests for SSE abort handling

📝 Recommendations

1. Add Runtime Validation: Replace type assertions with zod schemas
2. Improve Error Handling: Add graceful shutdown logging and consider cleanup timeouts
3. Extract Constants: Move hardcoded configuration values to constants
4. Add Tests: Create unit tests for configureServerlessRunner function
5. Follow Test Guidelines: Use repository-specific test timing utilities
6. Add JSDoc: Document public APIs per repository guidelines

🎯 Overall Assessment

Code Quality: Good - follows most repository conventions
Security: Good - no major security concerns
Performance: Good - efficient resource usage
Test Coverage: Fair - enhanced but could be more comprehensive

The changes are solid improvements but would benefit from addressing the type safety and error handling concerns before merging.

[MasterPtato]

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

• chore: make default runner name 'default' #1369
• fix: fix serverless config req, handle serverless sse close #1368 👈 (View in Graphite)
• fix(examples): add sleeping to smoke test #1367
• main

---

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]

More templates

• example-ai-agent
• example-better-auth-external-db
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-game
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler-proxy
• example-react
• example-smoke-test
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

commit: 276a060

[graphite-app]

Merge activity

• Oct 9, 12:47 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Oct 9, 12:48 AM UTC: CI is running for this pull request on a draft pull request (#1370) due to your merge queue CI optimization settings.
• Oct 9, 12:49 AM UTC: Merged by the Graphite merge queue via draft PR: #1370.