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 13, 2025 8:55pm

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 13, 2025 8:55pm
rivet-inspector	Ignored	Preview		Nov 13, 2025 8:55pm
rivet-site	Ignored	Preview		Nov 13, 2025 8:55pm

[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

• fix(rivetkit): remove incorrect getEndpoint call in metadata handler #3433 : 2 dependent PRs (#3438 , #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 👈 (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: af583ac

[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

[claude]

Code Review

Summary

This PR fixes a bug where actor.rLog was being used in an error handler when the actor variable could be undefined. The fix correctly uses loggerWithoutContext() instead.

Analysis

The Bug:
In the early return path at lines 122-139 in handleWebSocketConnect, when actorDriver.loadActor(actorId) fails, the code returns WebSocket handlers that need to handle errors. However, since the actor failed to load, the actor variable is undefined in this scope.

The onError handler has a try-catch that attempts to use actor.rLog, which would throw an error since actor is undefined at this point.

The Fix:
The PR correctly changes line 126 to use loggerWithoutContext() instead of actor.rLog. This is the right approach because:

1. The comment at rivetkit-typescript/packages/rivetkit/src/actor/log.ts:3 explicitly states: Prefer to use ActorInstance.rlog child logger. This does not provide context in the log, should only be used as a last resort if you cannot pass the actors child logger.
2. This is exactly such a last resort scenario - the actor failed to load, so we cannot access its logger.
3. The change is consistent with other usages in the codebase (see lines 523, 532 in the same file).

Issues Found

Critical Issue - Incomplete Fix:
The fix only addresses line 126 (in the onOpen handler), but there is another usage of actor.rLog at line 305 in the onError handler of the same error path that still needs to be fixed. Both references at lines 301 and 305 need to be changed to loggerWithoutContext() as well.

Test Coverage:
There do not appear to be any tests for this error path. Consider adding tests that:

1. Verify proper error handling when actorDriver.loadActor() fails
2. Ensure the WebSocket handlers can log errors without crashing
3. Test that the correct error codes are sent to the client

Recommendation

Action Required:
Please update line 305 to also use loggerWithoutContext() instead of actor.rLog.

Positive Notes

• Good catch on this edge case bug
• The fix uses the correct pattern established in the codebase
• The commit message follows conventional commit format
• The change is minimal and focused

---

Verdict: Needs changes - please fix the additional actor.rLog reference at line 305 before merging.

[claude]

Code Review

Summary

This PR fixes a bug where attempting to access actor.rLog would cause an error when the actor fails to load during WebSocket connection initialization. The fix correctly uses loggerWithoutContext() instead of actor.rLog in the error path at router-endpoints.ts:126.

Analysis

✅ Correctness
The fix is correct and necessary. The error path at line 123-139 is reached when actorDriver.loadActor(actorId) throws an exception (line 119), meaning the actor variable is undefined at that point. Attempting to access actor.rLog would result in a reference error.

✅ Consistency
This change aligns with the proper usage pattern for loggerWithoutContext(). According to the function's documentation in log.ts:3:

"Prefer to use ActorInstance.rlog child logger. This does not provide context in the log, should only be used as a last resort if you cannot pass the actor's child logger."

This is exactly the right use case - when the actor failed to load and we don't have access to its logger.

✅ Pattern Verification
I verified that all other deconstructError calls in the file correctly use actor.rLog when the actor is guaranteed to exist:

• Lines 195-202: Inside try block after actor is loaded ✓
• Lines 223-230: In message handler after actor exists ✓
• Lines 235-242: In parse error handler after actor exists ✓
• Lines 247-255: In handlers promise catch after actor exists ✓
• Lines 303-308: In error handler where actor is accessed via try-catch ✓

Observations

Minor: Error handling at line 298-309
The onError handler (lines 298-310) also accesses actor.rLog and wraps it in a try-catch. While this works, it could theoretically fail if the actor was never loaded. However, this handler is only called after onOpen succeeds, so the actor should always exist at that point. Still, for defensive programming, you might consider using loggerWithoutContext() in the catch block at line 303 as well, though this is not critical since that code path should never execute.

Verdict

LGTM ✅

This is a clean, focused bug fix that resolves a potential crash when actor loading fails during WebSocket initialization. The change is minimal, correct, and follows the established logging patterns in the codebase.

---

Review conducted following CLAUDE.md conventions

[claude]

PR Review

Summary

This PR fixes a bug where actor.rLog is accessed when the actor is undefined, which would cause a runtime error. The fix correctly uses loggerWithoutContext() instead when handling the early error case where actor loading fails.

Code Quality ✅

Excellent fix! This is a well-targeted bug fix that addresses a clear edge case.

• Correct error handling: The fix properly handles the scenario where actor is undefined after a failed loadActor() call
• Consistent with codebase patterns: Looking at log.ts:3, the comment explicitly states this is the intended use case: "should only be used as a last resort if you cannot pass the actor's child logger" - which perfectly describes this error path
• Minimal change: The fix is surgical and only changes what's necessary

Potential Issues

None identified. However, I did notice a few related observations:

1. Similar pattern elsewhere: Lines 246-255 have a similar error handler that uses actor.rLog inside a .catch() block. However, this appears safe because:

   • It's inside the handlersPromise.catch() which only executes if handlersReject() is called
   • handlersReject() is only called from line 193, where actor is guaranteed to be defined (inside the onOpen handler after actor is successfully loaded)

2. Typo on line 372: There's a comment typo: "Remvoe" should be "Remove" - but this is unrelated to this PR

Security Considerations ✅

No security concerns. The fix:

• Properly sanitizes errors using the existing deconstructError utility
• Respects the exposeInternalError flag for error exposure
• Maintains proper WebSocket close codes (1011 for server error)

Performance Considerations ✅

No performance impact. The change is in an error path that only executes when actor loading fails.

Test Coverage 📝

Suggestion: Consider adding a test case that:

1. Mocks actorDriver.loadActor() to throw an error
2. Attempts to open a WebSocket connection
3. Verifies the WebSocket receives the proper close code without throwing
4. Verifies logging occurs without errors

This would prevent regression of this bug. The test would fit well in a driver test suite given the RivetKit architecture.

Recommendations

✅ Approve and merge - This is a clean, necessary bug fix with no issues identified.

---

🤖 Generated by Claude Code

[claude]

Code Review

Summary

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

Analysis

The Bug: In handleWebSocketConnect at line 118-140, when actorDriver.loadActor(actorId) throws an error, the code catches it and returns WebSocket handlers. However, since the actor failed to load, the actor variable is never assigned. The original code at line 125 was trying to use actor.rLog, which would cause a runtime error.

The Fix: The change correctly replaces actor.rLog with loggerWithoutContext(). This is the appropriate solution because:

1. Context is unavailable - Since the actor failed to load, we do not have actor context to log with
2. Follows existing pattern - Line 126 already uses loggerWithoutContext() in the same error path
3. Matches the function purpose - According to actor/log.ts:3, loggerWithoutContext() is explicitly designed for scenarios where you cannot pass the actor child logger

Code Quality: Good

• The fix is minimal and targeted
• Uses the appropriate utility function for this edge case
• Consistent with the logging pattern used elsewhere in the same error handler

Potential Issues: Minor observation

1. Test coverage: There do not appear to be tests specifically covering this error path (WebSocket connection when actor fails to load). Consider adding a test in the driver-test-suite.
2. Context information: Since we are using loggerWithoutContext(), the logs will not include the actor ID. You might want to include actorId in the extraLog parameter to help with debugging.

Performance: No concerns

Security: No concerns

The change maintains the same error exposure behavior via exposeInternalError.

Recommendation: Approve

This is a valid bug fix. The change is correct and necessary. Consider adding the test coverage and extra logging context mentioned above as follow-up improvements.

---

Review generated with Claude Code

[graphite-app]

Merge activity

• Nov 13, 9:15 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 13, 9:15 PM UTC: CI is running for this pull request on a draft pull request (#3465) due to your merge queue CI optimization settings.
• Nov 13, 9:16 PM UTC: Merged by the Graphite merge queue via draft PR: #3465.