chore(rivetkit): make action cancelled error more descriptive

[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	Error			Nov 20, 2025 11:35pm

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 20, 2025 11:35pm
rivet-inspector	Ignored	Preview		Nov 20, 2025 11:35pm
rivet-site	Ignored	Preview		Nov 20, 2025 11:35pm

[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(engine): ensure first keepalive tick is not skipped to prevent timeout on ws hibernation #3507
• chore(examples): add counter-next-js example #3506
• feat(cloudflare-workers): add createInlineClient #3466
• fix(rivetkit): uri encode actor ids to fix cloudflare workers actor id with generation #3504
• chore(rivetkit): fix lockfile #3503
• chore(rivetkit): make action cancelled error more descriptive #3500 👈 (View in Graphite)
• fix(gateway): prevent gc from removing hibernating in flight req, check actor started after sub when hibernating #3505 : 1 other dependent PR (#3508 )
• fix(pb): add ability to timeout force resceduling pending state #3502
• fix(pb): stop actors from destroying on goingaway #3501
• fix(pegboard-gateway): ping hibernating requests both during open hws connections and during hibernation #3498 : 1 other dependent PR (#3499 )
• fix(rivetkit): fix graceful runner shutdown #3497
• fix(pegboard-gateway): write hws immediately on socket hibernate #3496
• chore(rivetkit): fix hibernation implementation #3494
• fix(tunnel): implement ping pong #3491 : 1 other dependent PR (#3475 )
• chore(pegboard-gateway): add new message id format & add deprecated tunnel ack #3492
• chore(pegboard-gateway): remove TunnelAck #3488
• chore(engine-runner): remove echo close on ToClientWebSocketClose #3487
• chore(rivetkit): implement new hibernating ws protocol #3464 : 1 other dependent PR (#3452 )
• fix(actor-kv): fix listing keys #3484
• fix(guard): handle actor stopped during ws/req #3481
• chore: rename retry -> hibernate in runner protocol #3473
• fix(udb): update backoff algo #3469
• chore: write/update docs #3468
• chore: add debug scripts #3467
• 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]

PR Review: Make action cancelled error more descriptive

Thanks for this improvement! The changes enhance debuggability by providing more context in error messages. Here's my analysis:

---

✅ Code Quality & Best Practices

Rust Change (pegboard-gateway/src/lib.rs:657)

• ✅ Change is clear and follows existing logging patterns
• ✅ Properly uses structured logging with tracing
• ✅ Message is lowercase as per CLAUDE.md conventions
• ✅ Adds helpful context to distinguish between different message types during hibernation

TypeScript Change (rivetkit/src/client/actor-conn.ts:534)

• ✅ Improves error messages with diagnostic information (close code and reason)
• ✅ Consistent with existing error formatting pattern used in line 509
• ✅ Template literal syntax is correct

---

🔍 Potential Issues

1. Missing Error Handling for Undefined Values
The new error message assumes closeEvent.code and closeEvent.reason are always defined, but they could potentially be undefined:

// Current (line 534):
`${wasClean ? "Connection closed" : "Connection lost"} (code: ${closeEvent.code}, reason: ${closeEvent.reason})`

// Could produce:
"Connection lost (code: undefined, reason: undefined)"

Recommendation: Add fallback values for better error messages:

const disconnectError = new Error(
  `${wasClean ? "Connection closed" : "Connection lost"} (code: ${closeEvent.code ?? 'unknown'}, reason: ${closeEvent.reason || 'none'})`,
);

Note that lines 509 and 516-517 already use these values directly, so this change maintains consistency, but defensive programming would improve robustness.

---

⚡ Performance Considerations

• ✅ No performance impact - both changes are in logging/error paths
• ✅ String interpolation is efficient and only occurs during error conditions

---

🔒 Security Concerns

• ✅ No security issues identified
• ✅ No user-controlled input is being logged that could cause injection issues
• ✅ Error messages don't expose sensitive information

---

🧪 Test Coverage

Observations:

• The TypeScript change affects error handling for in-flight actions during disconnection
• Existing tests in src/driver-test-suite/tests/actor-conn-state.ts cover connection lifecycle (lines 149-226)
• Tests do verify disconnection tracking but don't explicitly test error messages

Recommendation: Consider adding a test case that:

1. Initiates an action
2. Forces a connection close while the action is in-flight
3. Verifies the error message contains the close code and reason

Example test structure:

test("should include close code and reason in action error when connection closes", async (c) => {
  const { client } = await setupDriverTest(c, driverTestConfig);
  const conn = client.connStateActor.getOrCreate().connect();
  
  try {
    // Start an action that will be interrupted
    const actionPromise = conn.someLongRunningAction();
    
    // Force close the connection
    await conn.dispose();
    
    // Verify the error message
    await expect(actionPromise).rejects.toThrow(/code:.*reason:/);
  } catch (e) {
    // Verify error contains diagnostic info
  }
});

---

📋 Summary

Strengths:

• Improves debuggability significantly
• Follows existing code patterns
• No breaking changes
• Small, focused change

Suggestions:

1. Add null/undefined guards for closeEvent.code and closeEvent.reason
2. Consider adding explicit test coverage for the enhanced error messages

Overall Assessment: ✅ Approved with minor suggestions

The changes are valuable and safe to merge. The suggestions are optional improvements that would make the error handling even more robust.

[claude]

Code Review

This PR improves error messages by adding more diagnostic information. The changes are small but valuable for debugging.

Positive Points

1. Better debugging information: Adding the close code and reason to the error message will make it much easier to diagnose connection issues
2. Consistent with existing patterns: The TypeScript change follows the existing error message pattern seen elsewhere in the file (line 509)
3. Minimal change scope: Both changes are focused and non-breaking

Observations and Suggestions

TypeScript Change (rivetkit-typescript/packages/rivetkit/src/client/actor-conn.ts:534)

Potential Issue: The closeEvent.code and closeEvent.reason properties might be undefined in some contexts, particularly in Node.js environments.

Looking at line 499-502, there is a comment acknowledging that CloseEvent properties may be undefined in Node.js. The current implementation may produce messages like "Connection lost (code: undefined, reason: undefined)" in Node.js environments.

Recommendation: Add defensive handling for undefined values with nullish coalescing to ensure consistent formatting.

Rust Change (engine/packages/pegboard-gateway/src/lib.rs:657)

Looks good: The change from "received message" to "received websocket message" improves clarity. This distinguishes it from other types of messages like NATS messages.

Style check: Follows CLAUDE.md logging conventions - lowercase message.

Security and Performance

• Security: No concerns - these changes only affect error messages and logging
• Performance: No impact - these are error path changes

Overall Assessment

• Code Quality: Good - improves observability
• Risk Level: Low - but recommend defensive coding for undefined values in TypeScript
• Impact: Positive - will help with debugging production issues

Verdict: Approve with minor recommendation to handle undefined values in the TypeScript change using nullish coalescing operators.

[claude]

Code Review - PR 3500

Summary

This PR improves error messaging and logging clarity in two places:

1. Rust (pegboard-gateway): Makes a log message more specific about websocket messages during hibernation
2. TypeScript (rivetkit): Enhances error messages when actions are cancelled due to connection closure by including close code and reason

Strengths

Improved Debuggability
The TypeScript change adds valuable diagnostic information (code and reason) to the error message. This aligns well with the existing structured logging at actor-conn.ts:514-522 which already logs these fields. The Rust change clarifies that the message is specifically about websocket messages.

Consistency
The error message format in TypeScript now matches the pattern used in the onOpenPromise rejection at actor-conn.ts:509. Both now include code and reason in the error message.

Low Risk
Changes are purely informational (logging/error messages). No logic changes or behavioral modifications.

Observations and Suggestions

1. TypeScript: Potential Undefined Values
The closeEvent is cast from a generic Event type at actor-conn.ts:502. According to the comment, in NodeJS these properties may be undefined. The new error message should handle this gracefully by using nullish coalescing operators (closeEvent.code ?? 'unknown' and closeEvent.reason ?? 'none') to prevent showing undefined in error messages.

2. Test Coverage
No test changes included (and no existing tests found for these components). While this is a low-risk change, consider adding integration tests that verify error messages contain expected diagnostic information when connections close.

Verdict

LGTM with minor suggestion - The changes improve debugging experience with minimal risk. The only recommendation is to handle potentially undefined code/reason values in the TypeScript change.

Code Quality Metrics

• Follows existing patterns ✓
• No security concerns ✓
• No performance impact ✓
• Could handle undefined values more gracefully
• No test coverage (existing gap, not introduced by this PR)

[claude]

Code Review

Thanks for the improvements to error messages and logging. This PR makes debugging connection issues easier.

POSITIVES:

1. Better error messages - Adding code and reason to the disconnect error in TypeScript makes debugging connection issues much easier
2. Improved log clarity - The Rust change disambiguates between regular messages and websocket messages during hibernation
3. Minimal focused changes - Small well-scoped improvements that do not introduce risk
4. Consistent with existing patterns - Both changes follow existing logging and error handling patterns

OBSERVATIONS:

TypeScript Change (actor-conn.ts:534):
According to the comment on line 499-501, when running in NodeJS, closeEvent.code and closeEvent.reason may be undefined. The new error message could display "Connection lost (code: undefined, reason: undefined)". Consider adding a guard to handle undefined values more gracefully, or confirm that displaying undefined is acceptable.

Rust Change (pegboard-gateway/src/lib.rs:657):
The logging improvement is clear and unambiguous. No issues here.

OTHER CONSIDERATIONS:

• Test Coverage: These are logging/error message changes so no new tests required, but manual verification in both browser and NodeJS would be good
• Performance: No concerns
• Security: No concerns - error messages do not leak sensitive information

RECOMMENDATION: Approve (with optional consideration of undefined handling in TypeScript)

[claude]

Code Review

Summary

This PR improves error messaging by adding more context to log messages and error outputs. The changes are minimal but provide better debugging information.

---

✅ Code Quality & Best Practices

Positive aspects:

• Changes align with the project's structured logging patterns
• Error messages now include actionable debugging information (WebSocket close codes and reasons)
• Small, focused change that does exactly what the title describes

Suggestions:

1. TypeScript change (actor-conn.ts:534) - Consider handling potentially undefined values:

   const disconnectError = new Error(
       `${wasClean ? "Connection closed" : "Connection lost"} (code: ${closeEvent.code}, reason: ${closeEvent.reason})`,
   );

   The closeEvent.code and closeEvent.reason might be undefined in some environments (see line 502 comment about NodeJS). Consider defensive handling:

   const disconnectError = new Error(
       `${wasClean ? "Connection closed" : "Connection lost"} (code: ${closeEvent.code ?? 'unknown'}, reason: ${closeEvent.reason ?? 'none'})`,
   );

---

🐛 Potential Issues

Minor concern:

• The code already has a comment on line 499-501 acknowledging that CloseEvent properties may be undefined in NodeJS. The new error message uses these properties without null checks, which could result in messages like "Connection lost (code: undefined, reason: undefined)" in NodeJS environments.

---

🔒 Security Concerns

None identified. The changes only affect logging and error messages with no security implications.

---

⚡ Performance Considerations

None. String interpolation has negligible performance impact, and this code path only runs on connection close events.

---

🧪 Test Coverage

Observation:

• No tests were added/modified in this PR
• Given the nature of the change (improved error messages), manual testing may be sufficient
• However, if there are existing tests for connection error scenarios, verify they still pass and consider whether they should validate the new error message format

Recommendation:

• If you have integration tests that check WebSocket disconnect handling, consider validating that the error messages now include close codes and reasons
• Test both clean disconnects (wasClean=true) and unclean disconnects (wasClean=false) to ensure both message variants work correctly

---

📝 Documentation

The changes are self-documenting through improved error messages. No additional documentation needed.

---

Overall Assessment

Rating: ✅ Approve with minor suggestion

This is a good quality-of-life improvement for debugging. The only suggestion is to add null-safety for the closeEvent.code and closeEvent.reason properties to avoid "undefined" appearing in error messages in NodeJS environments.

[claude]

Code Review

This PR makes two small but valuable improvements to error messaging and logging. Overall, the changes are good and improve debuggability.

Positive Aspects

1. Improved Error Context: Adding the WebSocket close code and reason to the error message is a significant improvement for debugging connection issues.
2. Better Log Clarity: The clarification in lib.rs helps distinguish this log from other message-related logs.
3. Consistent with Existing Code: The error message format matches the pattern already used in the same file at line 509.

Code Quality

TypeScript Change (actor-conn.ts):

• The change follows existing patterns in the codebase
• The close code and reason are already being logged at line 516-517, so including them in the error message makes sense
• Potential Issue: Empty reason strings might result in messages like "Connection closed (code: 1000, reason: )". Consider handling empty reasons gracefully by only including the reason text when it is non-empty.

Rust Change (lib.rs):

• Follows the repository's logging conventions (lowercase messages)
• Uses structured logging with tracing
• More specific message improves log filtering capabilities

Test Coverage

• No test files found for actor-conn.ts. Consider adding unit tests to verify the error message format, especially for edge cases like empty reason strings, very long reason strings, and special characters in reason strings.

Security Considerations

• Minor Concern: The closeEvent.reason is controlled by the server and will be included in error messages. Ensure that the server doesn't include sensitive information in close reasons.

Performance Considerations

• No performance impact - string interpolation overhead is negligible
• No additional network calls or blocking operations

Suggestions

1. Handle empty reason strings to avoid awkward formatting when reason is empty
2. Consider adding tests for the error message formatting logic
3. If this error is user-facing, consider documenting common close codes for support teams

Summary

Verdict: Approve with minor suggestions

The changes are solid improvements to debuggability. The only concern is the potential for awkward formatting with empty reason strings, but this is a minor issue that doesn't block the PR. Great work on improving error messages!

[claude]

Code Review

Thanks for this PR! The changes improve error messages by adding more context.

Positive Aspects

1. Better debugging experience: Both changes add helpful context
2. Minimal, focused change: The PR is small and targets a specific improvement
3. Follows conventions: The commit message follows conventional commit format

Observations

TypeScript Change (actor-conn.ts:534): The addition of close code and reason is helpful, but closeEvent.reason may be undefined in Node.js environments (as noted in comments on line 499-501). This could result in messages displaying undefined. Consider defensive handling to only include reason when it exists.

Rust Change (lib.rs:672): The log message clarification is good and removes ambiguity.

Overall

The changes improve debuggability. Minor suggestion: add defensive checks for undefined values in the TypeScript error message.

Recommendation: Minor revision suggested for the TypeScript change to handle undefined values gracefully.

[claude]

Code Review

Summary: This PR improves error messaging by adding close code and reason to disconnection errors in TypeScript, and clarifies a log message in Rust.

Positive:

• Better debugging with code/reason details
• Consistent with existing pattern (line 509)
• Good logging conventions

Issues:

1. Potential undefined values - The comment on line 499-501 states that closeEvent properties may be undefined in Node.js, which could result in "Connection lost (code: undefined, reason: undefined)". Recommend adding a fallback check.

2. Empty PR description

Overall: Good improvement, but suggest handling potential undefined values for more robust error messages across different runtime environments.