fix(rivetkit): use loggerWithoutContext for websocket error when actor undefined

[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 9, 2025 5:46am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 9, 2025 5:46am
rivet-inspector	Ignored	Preview		Nov 9, 2025 5:46am
rivet-site	Ignored	Preview		Nov 9, 2025 5:46am

[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
• 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 👈 (View in Graphite)
• 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.

[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@3412

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 28c0736

[claude]

Code Review

Summary

This PR fixes a bug where actor.rLog was being accessed when the actor object was undefined in the error handler of handleWebSocketConnect. The fix correctly uses loggerWithoutContext() instead, which is appropriate since the actor failed to load.

Analysis

The Bug:
Looking at lines 138-162 in router-endpoints.ts, when actorDriver.loadActor(actorId) throws an error, the catch block creates an error handler that references actor.rLog on line 147 (before this fix). However, at this point, the actor variable has not been successfully assigned, so accessing actor.rLog would cause a runtime error.

The Fix:
The change on line 148 correctly switches from actor.rLog (undefined, would crash) to loggerWithoutContext() (safe fallback logger).

This is the correct approach because:

1. When the actor fails to load, there is no actor context to log with
2. loggerWithoutContext() is explicitly documented as the fallback for cases where an actor logger cannot be used (see src/actor/log.ts:3)
3. The same pattern is already used elsewhere in this file (e.g., lines 404, 442, 702)

Observations

Consistency:
This fix makes the error handling consistent with other error paths in the same file where the actor might not be available:

• Line 404: Uses actor.rLog ?? loggerWithoutContext() with a null coalescing operator
• Line 442: Uses loggerWithoutContext() in a catch block
• Line 702: Uses loggerWithoutContext() when actor is not available

Potential Related Issues:
While reviewing, I noticed a similar pattern on line 329 inside the onError handler where actor.rLog is used within a try-catch. However, since actor is guaranteed to exist at that point (it's assigned on line 141 and the early return on line 144 ensures we only reach the handlers if actor loaded successfully), this is actually safe.

Test Coverage

The codebase appears to have driver tests in the tests/ directory. It would be valuable to add a test case that verifies proper error handling when loadActor fails, ensuring:

1. The WebSocket connection is properly closed with the correct error code
2. No exceptions are thrown when logging the error
3. The error message is properly formatted for the client

Consider adding a test that mocks actorDriver.loadActor to throw an error and verifies the onOpen handler closes the connection gracefully.

Verdict

LGTM - This is a correct bug fix that:

• Prevents a potential crash when actor loading fails
• Uses the appropriate fallback logger
• Follows established patterns in the codebase
• Has minimal scope and low risk

Suggestions:

1. Consider adding a test case for this error path
2. The PR could benefit from a brief description explaining the bug and fix in the PR body

Great catch on this edge case!

[claude]

Code Review: PR #3412

Summary

This PR fixes a bug where actor.rLog was being accessed when actor is undefined in the websocket error handler. The fix correctly uses loggerWithoutContext() instead.

✅ Positive Feedback

1. Correct Bug Fix: The change correctly addresses a potential crash scenario. When loadActor() throws an error (lines 140-142), the actor variable is undefined, so accessing actor.rLog on line 147 would cause a runtime error.

2. Consistent Pattern: The fix aligns with the pattern used elsewhere in the codebase. For example, line 404 shows defensive coding: const rLog = actor.rLog ?? loggerWithoutContext();

3. Appropriate Logger Choice: According to the comment in log.ts:3, loggerWithoutContext() is the correct choice when the actor instance is unavailable. This is precisely the case in the catch block.

4. Minimal Change: The PR makes the smallest possible change to fix the issue, which reduces risk.

📋 Observations

1. Error Handling Path: This error path (lines 142-162) handles the case where actor loading fails before the websocket connection is established. Using loggerWithoutContext() is appropriate here since we have no actor context to log with.

2. Code Symmetry: The fix makes the error handler consistent - when there's no actor, use loggerWithoutContext(). This is self-documenting code.

🤔 Considerations

1. Test Coverage: Consider adding a test case that exercises this error path to prevent regression:

   • Mock actorDriver.loadActor() to throw an error
   • Verify that the websocket connection properly closes with code 1011
   • Ensure no unhandled errors occur from accessing undefined actor.rLog

2. Other Error Paths: You might want to audit other usages of actor.rLog in this file to ensure similar bugs don't exist elsewhere. A quick scan shows line 404 already handles this defensively.

✨ Verdict

LGTM - This is a clean, correct bug fix that prevents a potential runtime error. The change is minimal, follows existing patterns, and uses the appropriate logger for the context.

The only suggestion would be to add test coverage for this specific error path to prevent future regressions.

Suggested Test Scenario:

// Test that websocket error handler works when actor loading fails
it('should handle websocket connection when actor fails to load', async () => {
  // Mock actorDriver.loadActor to throw
  // Attempt websocket connection
  // Verify graceful error handling without accessing undefined actor.rLog
});

[claude]

PR Review: fix(rivetkit): use loggerWithoutContext for websocket error when actor undefined

Summary

This PR fixes a critical bug where the code attempts to access actor.rLog when the actor has failed to load, which would cause a runtime error since actor is undefined in the error handler scope.

Code Quality ✅

Positive aspects:

• Correct fix: The change properly addresses the root cause - when actorDriver.loadActor(actorId) throws an error (lines 140-142), the actor variable is never initialized, making actor.rLog undefined
• Consistent pattern: This follows the existing pattern used elsewhere in the codebase (see line 404: const rLog = actor.rLog ?? loggerWithoutContext())
• Minimal change: The fix is surgical and only touches the problematic line

Analysis:
Looking at the error handling flow in handleWebSocketConnect:

1. Lines 140-142: Attempt to load actor in a try-catch block
2. Lines 143-162: Error handler returns early with WebSocket close handlers
3. Line 148: The bug - tries to use actor.rLog but actor is undefined in this error path
4. The fix correctly uses loggerWithoutContext() which is designed for exactly this scenario (see src/actor/log.ts:3 - "should only be used as a last resort if you cannot pass the actor's child logger")

Potential Issues ⚠️

Minor consideration:

• While this fix is correct, there's a similar error handler at lines 326-337 (onError callback) that uses actor.rLog without a try-catch. However, this appears safe because by the time onError is called, the actor has already been successfully loaded (line 141).

Best Practices ✅

The change adheres to the codebase conventions:

• Uses the designated fallback logger function
• Maintains structured logging patterns
• Follows the comment guidance in log.ts about when to use loggerWithoutContext()

Security Concerns ✅

No security issues identified. The change only affects logging behavior in error scenarios.

Performance Considerations ✅

No performance impact - loggerWithoutContext() is a lightweight function that returns a logger instance.

Test Coverage 📝

Recommendation:
Consider adding a test case that verifies:

1. When actorDriver.loadActor() fails, the error handler properly logs using loggerWithoutContext()
2. The WebSocket closes with the expected error code (1011)
3. No runtime errors occur when accessing the logger

Example test scenario:

// When actor fails to load
// Then error should be logged without runtime errors
// And WebSocket should close with code 1011

Overall Assessment ✅

This is a solid bug fix. The change is:

• Necessary to prevent runtime errors when actor loading fails
• Minimal and focused
• Consistent with existing patterns in the codebase
• Following the documented best practices for fallback logging

Recommendation: Approve and merge after considering the test coverage suggestion above.

---

🤖 Review generated by Claude Code

[claude]

Code Review

Summary

This PR fixes a bug where actor.rLog was being used in an error handler when the actor might be undefined. The fix correctly uses loggerWithoutContext() instead, which is the appropriate fallback logger for cases where actor context is unavailable.

Analysis

Bug Context:
In handleWebSocketConnect, when actorDriver.loadActor(actorId) throws an error (lines 118-120), the actor variable is not initialized. However, the error handler at line 124 was attempting to use actor.rLog, which would cause a runtime error since actor is undefined at that point.

Fix:
The change at line 126 correctly replaces actor.rLog with loggerWithoutContext(), which aligns with the documented pattern:

• Per src/actor/log.ts:3, loggerWithoutContext() is intended for "last resort" scenarios when the actor's child logger cannot be passed
• This is exactly that scenario - the actor failed to load, so actor.rLog is inaccessible

Code Quality ✅

Strengths:

1. Correct fix: The change properly handles the undefined actor scenario
2. Consistent pattern: Matches other usages in the codebase (e.g., router-endpoints.ts:523, router-endpoints.ts:532)
3. Minimal change: Only modifies what's necessary to fix the bug
4. Proper error handling: Maintains the error deconstructing and WebSocket close behavior

Potential Issues

Minor Observation:
The same error path also uses actor.rLog in other event handlers (lines 249, 301, 305). However, these are not problematic because:

• Line 249: Inside handlersPromise.catch() - only reachable if actor was successfully loaded
• Lines 301, 305: Inside onError handler's try-catch - the try block would fail before reaching the catch

Test Coverage:
No automated tests exist for this code path. Consider adding test coverage for:

• WebSocket connection when loadActor fails
• Verify proper error logging and WebSocket close behavior
• Ensure no runtime errors when actor is undefined

Performance ✅

No performance concerns. The change doesn't affect the execution path.

Security ✅

No security concerns. The fix maintains the same error exposure behavior through exposeInternalError.

Best Practices ✅

Follows the repository's error handling patterns and logging conventions from CLAUDE.md.

Recommendation

LGTM ✅

This is a good bug fix that prevents a potential runtime error. The implementation is correct and follows existing patterns in the codebase.

Optional follow-up:
Consider adding test coverage for this error path to prevent future regressions.

---

🤖 Generated with Claude Code