refactor(rivetkit): extract WebSocket protocol parsing to shared utility

[NathanFlurry]

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	Nov 10, 2025 1:14am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 10, 2025 1:14am
rivet-inspector	Ignored	Preview		Nov 10, 2025 1:14am
rivet-site	Ignored	Preview		Nov 10, 2025 1:14am

[NathanFlurry]

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

• feat(site): add typedoc docs #3446
• chore(rivetkit): remove deprecated packages #3447
• chore(rivetkit): add support for conn params on onRequest and onWebSocket #3445
• feat(rivetkit): add asyncapi spec #3442
• chore(rivetkit): add actor router to the openapi spec #3441
• fix(rivetkit): skip sending RivetKit messages to conns that do not support it #3440
• fix(rivetkit): fix race condition with websocket open events #3439
• fix(rivetkit): standardize error metadata return type #3438
• fix(rivetkit): remove incorrect getEndpoint call in metadata handler #3433
• feat(rivetkit): add json protocol support #3432
• chore(rivetkit): add required zod validation for json encoding #3431
• chore(rivetkit): add listing actors by only name to manager api #3430
• refactor(rivetkit): extract WebSocket protocol parsing to shared utility #3429 👈 (View in Graphite)
• refactor(rivetkit): restructure connection lifecycle contexts #3428
• chore(rivetkit): add WebSocketContext and RequestContext #3427
• chore(rivetkit): add conn type #3426
• chore(rivetkit): add connection for raw http #3425
• chore(rivetkit): rename onFetch to onRequest #3424
• refactor(rivetkit): add state manager for conn with symbol-based access #3423
• fix(rivetkit): use loggerWithoutContext for websocket error when actor undefined #3412
• feat(rivetkit): add GET /actors/names #3411
• feat(api-public): expose actor kv api #3410
• chore(rivetkit): auto-generate inspector token for engine driver #3409
• chore(rivetkit): close websockets on out of sequence messages #3404
• fix(rivetkit): properly apply clientEndpoint from metadata lookup #3402
• fix(rivetkit): properly handle msgIndex for hibernatable websocket reconnection #3401
• chore(rivetkit): rename onstart/onstop to onwake/onsleep #3422
• chore(rivetkit): split ActorInstance logic in to multiple classes #3421
• chore(rivetkit): move conns to separate persisted kv keys #3420
• chore(rivetkit): remove deprecated conn tokens & sse support #3419
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

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.

[claude]

Code Review for PR #3429

Summary

This PR successfully refactors WebSocket protocol parsing logic by extracting duplicated code into a shared utility function parseWebSocketProtocols(). The refactoring reduces code duplication and improves maintainability.

✅ Positive Changes

1. Good DRY principle application: Successfully eliminates ~50 lines of duplicated parsing logic across 3 files
2. Clear function documentation: The new parseWebSocketProtocols() function has a clear JSDoc comment
3. Consistent behavior: All three call sites now use the same parsing logic, reducing the risk of inconsistent behavior
4. Improved maintainability: Future changes to WebSocket protocol parsing only need to happen in one place

🔍 Code Quality & Best Practices

Issue 1: Silent fallback behavior change in getRequestEncoding()

The PR changes getRequestEncoding() to return "json" as a default instead of throwing an error when the encoding header is missing:

// Before:
if (!encodingParam) {
    throw new errors.InvalidEncoding("undefined");
}

// After:
if (!encodingParam) {
    return "json";
}

Concerns:

• This is a breaking behavioral change that affects error handling in common/router.ts:66 where the try-catch expects this function to throw
• The comment mentions supporting "vanilla curl requests," but this changes existing error handling semantics
• Could mask configuration issues where clients forget to send the encoding header

Recommendation: Consider one of these approaches:

1. Keep the original error-throwing behavior and add a separate getRequestEncodingOrDefault() function for cases where a default is acceptable
2. Add an optional parameter: getRequestEncoding(req: HonoRequest, defaultEncoding?: Encoding)
3. If this change is intentional, update the error handler in common/router.ts to remove the now-unnecessary try-catch

Issue 2: Missing error handling in parseWebSocketProtocols()

const encoding = EncodingSchema.parse(encodingRaw);
const connParams = connParamsRaw ? JSON.parse(connParamsRaw) : undefined;

Both EncodingSchema.parse() and JSON.parse() can throw exceptions:

• If encodingRaw is undefined or invalid, EncodingSchema.parse() will throw
• If connParamsRaw contains invalid JSON, JSON.parse() will throw

Recommendation:

• Use EncodingSchema.safeParse() like the original getRequestEncoding() function does, and throw an InvalidEncoding error with proper context
• Wrap JSON.parse() in try-catch and throw an appropriate error (e.g., InvalidRequest or MalformedMessage)
• This ensures consistent error messages for clients

Example:

const result = EncodingSchema.safeParse(encodingRaw);
if (!result.success) {
    throw new errors.InvalidEncoding(encodingRaw);
}
const encoding = result.data;

let connParams: unknown;
if (connParamsRaw) {
    try {
        connParams = JSON.parse(connParamsRaw);
    } catch (error) {
        throw new errors.MalformedMessage(`Invalid connection parameters: ${error}`);
    }
}

🐛 Potential Bugs

Bug 1: encodingRaw can be undefined

If no rivet_encoding. protocol is provided, encodingRaw will be undefined, causing EncodingSchema.parse(undefined) to throw without a clear error message.

Expected behavior: Should either:

1. Throw InvalidEncoding("undefined") for consistency with the previous code
2. Default to "json" if that's the intended new behavior

Bug 2: Inconsistent default behavior

• getRequestEncoding() now defaults to "json" when missing
• parseWebSocketProtocols() will throw when encoding is missing

This creates inconsistent behavior between HTTP requests and WebSocket connections.

🔒 Security Concerns

Low-severity concern: Missing URL decode validation

connParamsRaw = decodeURIComponent(
    protocol.substring(WS_PROTOCOL_CONN_PARAMS.length),
);

While decodeURIComponent() can throw on malformed input, the error isn't caught or handled gracefully. Consider wrapping in try-catch to provide a clearer error message.

⚡ Performance Considerations

No significant performance concerns. The refactoring actually improves performance slightly by:

• Removing code duplication (smaller bundle size)
• Using the same parsing logic (more consistent V8 optimization)

🧪 Test Coverage

Missing test coverage:

• No unit tests found for the new parseWebSocketProtocols() function
• No tests for getRequestEncoding() default behavior change

Recommendation:
Add tests covering:

1. Valid protocol parsing with both encoding and connParams
2. Valid protocol with encoding only
3. Valid protocol with connParams only
4. No protocols provided (should test error/default behavior)
5. Invalid encoding value
6. Malformed JSON in connParams
7. Malformed URL encoding in connParams

📋 Suggested Changes

1. Add proper error handling to parseWebSocketProtocols() (High priority)
2. Reconsider the default behavior change in getRequestEncoding() (High priority)
3. Add unit tests (Medium priority)
4. Ensure consistent default behavior between HTTP and WebSocket paths (Medium priority)
5. Add error handling for decodeURIComponent() (Low priority)

Summary Rating

• Code Quality: ⚠️ Good with concerns - The refactoring is solid, but error handling needs improvement
• Security: ✅ Acceptable - No major security issues
• Performance: ✅ Good - Slight improvement
• Test Coverage: ❌ Insufficient - Missing tests for new functionality

Overall, this is a good refactoring that improves code maintainability, but it needs stronger error handling and test coverage before merging.

[pkg-pr-new]

More templates

• example-ai-agent
• example-background-jobs
• example-better-auth-external-db
• example-bots
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-cursors
• example-cursors-raw-websocket
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• 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
• example-raw-websocket-handler-proxy
• example-react
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc
• example-workflows

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3429

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3429

commit: 52895aa