fix(rivetkit): fix onError handler not working

[NathanFlurry]

No description provided.

[vercel]

Deployment failed with the following error:

Too many requests - try again in 3 minutes (more than 120, code: "api-deployments-flood-pro").

Learn More: https://vercel.com/docs/limits#rate-limits

[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(rivetkit): add ability to destroy actors #3458 : 2 dependent PRs (#3459 , #3462 )
• feat(rivet-engine): udb key parser #3457
• fix(rivetkit): fix zod not validating body if content-type header not provided #3456
• fix(rivetkit): fix onError handler not working #3455 👈 (View in Graphite)
• chore(site): tweak wording #3454
• chore(site): fix generating examples #3453
• chore: create branch for v3 website changes on v2 #3304
• chore(website): flatten actors docs structure #3280
• chore(rivetkit): switch dynamic node imports to use require #3461
• chore(rivetkit-typescript): remove dependency on node modules #3460
• 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 : 1 other dependent PR (#3451 )
• 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
• 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

Summary

This PR fixes the onError handler not working in RivetKit by simplifying the server initialization flow. The changes remove unnecessary abstraction that was preventing Hono's error handlers from functioning properly.

Changes Overview

1. serve.ts: Simplified crossPlatformServe() to accept a single app parameter instead of separate rivetKitRouter and userRouter parameters
2. mod.ts: Updated the call site to pass only the Hono router created by createManagerRouter()

Code Quality ✅

Strengths:

• The simplification is correct - the router created by createManagerRouter() already has onError and notFound handlers registered (see manager/router.ts:130-131)
• Removing the unused userRouter parameter and the conditional logic that created a new Hono instance improves clarity
• The change fixes a real bug: when a new Hono instance was created in crossPlatformServe, the error handlers from the original router were lost
• Type import change (import type { Hono }) is appropriate since Hono is only used as a type

Potential Issues 🔍

Minor:

1. Redundant object property shorthand (serve.ts:43)

   app: app,  // Could be just: app

   This is harmless but could be simplified to just app in modern JavaScript/TypeScript.

Performance Considerations ✅

No performance concerns. The change actually improves performance slightly by:

• Eliminating unnecessary Hono instance creation
• Removing dead code (commented-out routing logic)

Security Concerns ✅

No security issues identified. Error handling is properly configured on the router before being passed to this function.

Test Coverage ⚠️

Recommendation:
Consider adding a test that verifies onError handlers work correctly. This could prevent similar regressions. For example:

• Send a request that triggers an error
• Verify the error handler is invoked
• Check the error response format

Architecture Note 📝

The fix makes sense architecturally - crossPlatformServe() should be a platform adapter that takes a configured Hono app, not a router configurator. The error handling configuration belongs in createManagerRouter() and createActorRouter(), which is where it already exists.

Verdict: ✅ LGTM

This is a clean fix that correctly addresses the root cause. The previous implementation was creating a new Hono instance without the error handlers, and this PR fixes that by passing the already-configured router directly.

Optional improvement: Consider changing app: app, to app, on line 43 for consistency with modern JavaScript conventions.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Nov 13, 2025 3:24am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 3:24am
rivet-inspector	Ignored	Preview		Nov 13, 2025 3:24am
rivet-site	Ignored	Preview		Nov 13, 2025 3:24am

[claude]

Code Review

Summary

This PR fixes a bug where the onError handler was not working correctly in the RivetKit server. The fix simplifies the crossPlatformServe function by removing the unused userRouter parameter and having the caller pass in the fully-configured Hono app directly.

What Changed

Before: The crossPlatformServe function accepted both a rivetKitRouter and an optional userRouter, then conditionally created a new Hono app and mounted the rivetKitRouter on it.

After: The function now accepts a single pre-configured Hono app parameter, eliminating the router composition logic.

Analysis

Positive Aspects ✅

1. Root Cause Fix: The issue was that creating a new Hono app inside crossPlatformServe meant the error handlers (router.onError) registered in createManagerRouter (router.ts:131) were being applied to a different Hono instance than the one actually serving requests. This fix ensures the same router instance with registered error handlers is passed through to the server.

2. Simplified Architecture: Removing the router composition logic makes the code cleaner and more maintainable. The responsibility for router setup now clearly lives in one place (createManagerRouter).

3. Reduced Coupling: The function no longer needs to know about mounting logic or how routers should be composed.

4. Type Improvement: Changed import from import { Hono } to import type { Hono } which is more semantically correct since it's only used for typing.

5. Consistent Pattern: This aligns with how actor routers work (see actor/router.ts:229-230), where error handlers are registered on the router that gets served.

Potential Concerns ⚠️

1. No Test Coverage: This PR doesn't include tests to verify the error handler now works correctly. Consider adding a test that:

   • Triggers an error condition
   • Verifies the error is handled by handleRouteError
   • Confirms the proper error response format

2. Breaking Change Risk: If there were any external consumers expecting to pass a userRouter as the third parameter, this would be a breaking change. However, looking at the call site (mod.ts:225), this parameter was always undefined, so this appears safe.

3. Redundant Code: Line 43 has redundant syntax: app: app, could be simplified to just app, (ES6 shorthand property).

Code Quality

The changes follow the repository patterns well:

• Consistent with error handling setup in createActorRouter (actor/router.ts:230)
• Maintains the existing cross-platform server abstraction
• Follows TypeScript best practices with type imports

Recommendations

1. Add a test to verify error handling works:

// Example test structure
test('error handler catches and formats errors correctly', async () => {
  // Setup registry that throws an error
  // Make request that triggers the error
  // Assert error response matches expected format from handleRouteError
});

2. Minor cleanup - simplify line 43:

-	app: app,
+	app,

3. Consider adding a comment in crossPlatformServe explaining why the app must be pre-configured (to preserve error handlers).

Security

No security concerns identified.

Performance

No performance impact - if anything, this is slightly more efficient by removing unnecessary router composition.

Verdict

Approve with minor suggestions ✅

This is a good fix that addresses the root cause properly. The architectural simplification is a bonus. Would be even better with test coverage to prevent regression.

[claude]

Code Review

Summary

This PR fixes an issue where the onError handler in the manager router was not being properly applied. The root cause was that crossPlatformServe was creating a new Hono instance instead of using the router that already had error handlers configured via router.onError(handleRouteError) at manager/router.ts:131.

Positive Changes

1. Correct Architecture: The fix properly ensures that the configured router (with its error handlers) is used directly, rather than being potentially bypassed or replaced.
2. Simplified API: The function signature is cleaner - from 3 parameters down to 2. This removes unnecessary complexity.
3. Removed Dead Code: The commented-out routing code has been removed, which is good housekeeping.
4. Type Import Optimization: Changed from import Hono to import type Hono, which is a minor optimization for TypeScript compilation.

Code Quality Assessment

Overall: Good - The changes are minimal, focused, and solve a specific problem.

Strengths:

• The fix is surgical and addresses the root cause
• No unnecessary changes or scope creep
• Proper TypeScript type usage

Concerns

1. Missing Test Coverage - There don't appear to be tests added to verify that onError handlers work correctly. Consider adding a test that triggers an error in a route, verifies that handleRouteError is called, and checks the error response format matches expectations.

2. Redundant Property Assignment (Minor) - Line 43 in serve.ts has app: app which can be simplified to just app in modern JavaScript.

3. Lack of Context in PR Description - The PR description is empty. It would be helpful to include how the bug was discovered, steps to reproduce the original issue, and explanation of why the previous approach didn't work.

Security Considerations

No security concerns identified. The changes don't introduce new attack vectors.

Performance Considerations

Positive Impact: By removing the creation of an unnecessary Hono instance, there's a minor performance improvement (one less object allocation).

Suggestions for Follow-up

1. Add Integration Test: Create a test that verifies error handling works end-to-end to prevent regression.
2. Documentation: Consider adding a comment in crossPlatformServe explaining that the app parameter must be the fully-configured router with error handlers already attached.
3. Code Cleanup: Fix the redundant property shorthand on line 43.

Recommendation

Approve with minor suggestions - The core fix is solid and addresses a real bug. The suggestions above are nice-to-haves that can be addressed in follow-up PRs if needed. The most important one is adding test coverage to prevent regression.

Review performed by Claude Code following CLAUDE.md conventions

[claude]

Pull Request Review

Summary

This PR fixes a bug where the onError handler wasn't working properly in RivetKit. The fix simplifies the crossPlatformServe function by removing the userRouter parameter that was causing the router to be replaced with a new instance.

Code Quality ✅

Positive Changes:

• Simplified API: Removing the userRouter parameter simplifies the function signature and reduces complexity
• Import optimization: Changed from import { Hono } to import type { Hono } for better tree-shaking
• Redundant code removal: Eliminated unnecessary router mounting logic that was commented out anyway
• Bug fix: The core issue is resolved - by passing the hono router directly instead of creating a new one, the onError handler registered in router.ts:131 now properly handles errors

Code Style:

• Follows TypeScript best practices
• Consistent with the codebase style
• The change at line 43 (app: app → app: app,) is redundant but harmless (the app: key could just be app)

Architecture ✅

The fix correctly addresses the root cause:

• Before: A new Hono instance was created in crossPlatformServe when userRouter was undefined, which meant the error handlers registered on the original router were lost
• After: The router with error handlers (set up in manager/router.ts:131 via handleRouteError) is passed through correctly

Potential Issues & Suggestions

1. Minor Code Style (line 43 in serve.ts):

   // Current:
   const { injectWebSocket, upgradeWebSocket } = createNodeWebSocket({
       app: app,
   });
   
   // Could be simplified to:
   const { injectWebSocket, upgradeWebSocket } = createNodeWebSocket({ app });

2. Test Coverage: There don't appear to be tests for the error handling behavior. Consider adding a test that:

   • Triggers an error in a route handler
   • Verifies that handleRouteError is called correctly
   • Checks that the error response format matches expectations

3. Type Safety: The function now has a simpler contract, but you might want to add JSDoc comments explaining:

   • That the app parameter should already have error handlers registered
   • The expected router configuration state

Security ✅

No security concerns identified. The changes don't introduce any new attack vectors or security vulnerabilities.

Performance ✅

• The import change to import type is a positive performance optimization
• Removing the unnecessary router creation/mounting reduces runtime overhead slightly
• No performance regressions expected

Breaking Changes ⚠️

This is technically a breaking change for anyone who was passing a custom userRouter to crossPlatformServe, though:

1. This appears to be an internal API
2. The function signature change from mod.ts:223 shows it was only being called with two parameters anyway
3. The userRouter functionality appears to have been incomplete/non-functional (commented out mounting code)

Recommendation: Approve ✅

This is a solid bug fix that simplifies the code while resolving the error handling issue. The changes are focused, well-executed, and improve the codebase. Consider adding the minor style cleanup and test coverage as follow-up items.

[claude]

PR Review: Fix onError Handler Not Working

Summary

This PR fixes a bug where the onError handler registered in createManagerRouter was not being invoked because a new Hono instance was being created in crossPlatformServe instead of using the router passed from the registry.

Code Quality: ✅ Good

Positive Changes:

• Root Cause Fix: The change correctly addresses the issue by removing the logic that was creating a new Hono instance and mounting the RivetKit router onto it
• Simplified API: The function signature is now cleaner - crossPlatformServe now takes a single app parameter instead of rivetKitRouter and userRouter
• Type Improvement: Changed import { Hono } to import type { Hono } which is a good TypeScript practice for type-only imports (rivetkit-typescript/packages/rivetkit/src/registry/serve.ts:1)
• Removed Dead Code: The commented-out line // app.route("/registry", rivetKitRouter); is now removed as part of the cleanup

Technical Details:

• Before: A new Hono app was created, and the router (with its onError handler) was mounted as a sub-route, which can break error handling in Hono
• After: The router with the error handler is passed directly to the server, preserving all middleware and handlers

Potential Issues: ⚠️ Minor Concern

1. Redundant Property Shorthand (rivetkit-typescript/packages/rivetkit/src/registry/serve.ts:43):

   const { injectWebSocket, upgradeWebSocket } = createNodeWebSocket({
     app: app,  // Could be simplified to just `app`
   });

   This is a very minor style issue - could use object property shorthand (app instead of app: app).

Performance Considerations: ✅ Positive Impact

• Reduced Overhead: Eliminates the creation of an unnecessary intermediate Hono instance
• Direct Routing: Removes one layer of route mounting, which should have a minor positive impact on request handling performance

Security Concerns: ✅ No Issues

• No security concerns identified
• The change actually improves error handling by ensuring the handleRouteError handler is properly invoked, which could improve security by ensuring consistent error responses

Test Coverage: ⚠️ Needs Attention

Missing Tests:

• No test coverage appears to exist for verifying that the onError handler is properly invoked
• Recommend adding an integration test that:
  1. Triggers an error in a route handler
  2. Verifies that handleRouteError is called
  3. Validates the error response format

Test Suggestion:

// Example test structure
test("onError handler is invoked for route errors", async () => {
  // Setup: Create a registry with a route that throws
  // Act: Make a request to the error-throwing route
  // Assert: Verify the error response matches handleRouteError output
});

Breaking Changes: ✅ None

This is an internal implementation detail change with no API surface changes.

Recommendations:

1. Add Integration Tests: As mentioned above, add tests to prevent regression
2. Minor Style Fix: Use property shorthand for app: app → app
3. Consider Documentation: If the onError handler behavior is important for developers extending RivetKit, consider adding documentation about how error handling works

Verdict: ✅ APPROVE with suggestions

This is a solid bug fix that correctly addresses the root cause. The code quality is good, and it simplifies the API. My only concerns are around test coverage and a very minor style issue. I recommend merging after considering the test coverage suggestion.

---

Files Changed:

• rivetkit-typescript/packages/rivetkit/src/registry/mod.ts (+1, -1)
• rivetkit-typescript/packages/rivetkit/src/registry/serve.ts (+3, -10)