chore(rivetkit): improve conn inspector properties #3317

NathanFlurry · 2025-11-03T22:20:26Z

No description provided.

vercel · 2025-11-03T22:20:33Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Nov 4, 2025 8:26pm

3 Skipped Deployments

Project	Deployment	Preview	Updated (UTC)
rivet-cloud	Ignored		Nov 4, 2025 8:26pm
rivet-inspector	Ignored	Preview	Nov 4, 2025 8:26pm
rivet-site	Ignored	Preview	Nov 4, 2025 8:26pm

NathanFlurry · 2025-11-03T22:20:39Z

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

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 · 2025-11-03T22:26:01Z

More templates

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 4fddf6b

claude · 2025-11-03T22:27:57Z

PR Review: Connection Inspector Properties Enhancement

Summary

This PR enhances the connection inspector by adding more useful properties for debugging and monitoring connection status. Overall, the changes are well-implemented and follow good practices.

Code Quality ✅

Strengths:

Clean, focused changes that enhance observability
Consistent with existing code style and patterns
Property reordering improves logical grouping (connection data first, then metadata)
Added structured logging with meaningful counters in #checkConnectionsLiveness()

Suggestions:

The reordering of properties in the inspector (params, state, then status, etc.) improves readability - good call putting the most important data first.

Implementation Review

Inspector Changes (lines 224-235)

{
  id,
  params: conn.params as any,
  state: conn.__stateEnabled ? conn.state : undefined,
  status: conn.status,
  subscriptions: conn.subscriptions.size,
  lastSeen: conn.lastSeen,
  stateEnabled: conn.__stateEnabled,
  isHibernatable: conn.isHibernatable,
}

Positive:

All new properties (status, subscriptions, lastSeen, isHibernatable) are exposed as public getters on the Conn class
Using .size for subscriptions count is more efficient than returning the entire Set
Properties align with the existing Conn interface (verified in conn.ts:125-152)

Note: The lastSeen timestamp is already in milliseconds (Date.now() format), which is consistent with the codebase's timestamp convention mentioned in CLAUDE.md.

Logging Enhancement (lines 1298-1345)

let connected = 0;
let reconnecting = 0;
let removed = 0;

Positive:

Adds valuable metrics for connection health monitoring
Structured logging follows the tracing pattern from CLAUDE.md (lowercase messages with structured fields)
The summary log provides a complete picture of connection state

Minor observation: The log message uses ? prefix for objects (e.g., ?x) which is correct for the tracing crate in Rust, but I don't see it consistently used elsewhere in this TypeScript file. The current format looks fine though.

Potential Issues 🔍

No blocking issues found, but a few observations:

Type Safety: The any type cast on params (line 228) is pre-existing and acceptable here since params is generic and being passed to the inspector.
Performance: Calling conn.isHibernatable (line 234) on every inspector call invokes getConnDriverKindFromState() and driver lookups. This is probably fine for debugging purposes, but if the inspector is called frequently in production, consider caching this value or documenting that it's for debug use.
Edge Case: In #checkConnectionsLiveness(), the counter logic doesn't account for connections that might be removed while iterating (since #removeConn() is called). However, this is safe because:
- We're iterating over values() which creates a snapshot
- #removeConn() modifies the map but doesn't affect the iterator
- The counters are just for logging, not critical logic

Testing 🧪

Observation: I couldn't find test files for the instance or connection classes. For a change like this that enhances the inspector:

Consider manually testing the inspector output with various connection states
Verify that hibernatable connections correctly report their status
Test with connections in both "connected" and "reconnecting" states

Security ✅

No security concerns. The inspector is for debugging/monitoring and doesn't expose sensitive token information (correctly uses metadata fields only).

Performance ✅

Negligible performance impact:

Inspector changes only add property lookups
Counter variables in liveness check are simple increments
Single additional log statement per liveness check

Documentation 📝

Suggestion: Since this enhances the inspector interface, consider adding a comment documenting the inspector connection properties, or ensure this is reflected in any external documentation/README that describes the inspector API.

Summary Rating: Approved ✅

This is a clean, focused improvement that enhances debugging capabilities without introducing risks. The additional properties provide valuable insights into connection state, and the logging improvements will help with troubleshooting connection lifecycle issues.

Recommendation: Merge after addressing any documentation needs for the inspector API (if applicable).

claude · 2025-11-04T19:09:03Z

Pull Request Review: chore(rivetkit): improve conn inspector properties

Overview

This PR enhances the connection inspector by adding additional properties and improving the logging of connection liveness checks. The changes are focused on observability improvements for debugging and monitoring actor connections.

Code Quality: ✅ Good

Strengths:

Better observability: The inspector now exposes more useful connection properties (status, subscriptions, lastSeen, isHibernatable)
Improved logging: Connection liveness checks now log aggregated statistics (connected/reconnecting/removed counts)
Consistent with existing patterns: Follows the codebase's structured logging approach using tracing
Non-breaking changes: Additional properties don't remove existing functionality

Minor suggestions:

Property ordering (instance.ts:226-235): The reordering moves stateEnabled down from the second position. While this doesn't break functionality, consider if there's a semantic reason for the new order or if grouping related properties would be clearer:
```
// Current order after PR:
id, params, state, status, subscriptions, lastSeen, stateEnabled, isHibernatable

// Alternative logical grouping:
id, status, params, state, stateEnabled, subscriptions, lastSeen, isHibernatable
```
This groups: identity → connection status → data (params/state) → metadata
Log message consistency (instance.ts:1339): The log message uses past tense "checked" which is good, but the initial log at 1296 uses present progressive "checking". Consider consistency:
```
// Line 1296
this.#rLog.debug({ msg: "checking connections liveness" });
// Could be: "checking connection liveness" (already singular "connection")
```

Potential Issues: ⚠️ Minor

Integer overflow safety (instance.ts:1298-1300): The counters initialize inside the loop but are updated within it. While this works correctly, the pattern could be clearer with initialization before the loop:
```
// Current (works fine):
let connected = 0;
let reconnecting = 0; 
let removed = 0;
for (const conn of this.#connections.values()) { ... }

// This is actually the right pattern - no issue here!
```
Actually, on review, this is the correct pattern. No issue.

Total calculation (instance.ts:1341): The total calculation as connected + reconnecting may not match the actual .size of #connections after removals during the loop iteration:

total: connected + reconnecting,  // Doesn't include removed count

Consider if you want to log the pre-check total vs post-check total:

const initialCount = this.#connections.size;
// ... loop ...
this.#rLog.debug({
  msg: "checked connection liveness",
  initialCount,
  connected,
  reconnecting,
  removed,
  finalCount: connected + reconnecting, // Should match this.#connections.size
});

Performance Considerations: ✅ Good

The changes add minimal overhead (just property access and integer increments)
No new iterations or expensive operations introduced
Logging is at debug level, appropriate for this detail level

Security Concerns: ✅ None

No sensitive data exposed in inspector properties
All properties were already accessible through other means
Connection tokens remain properly protected

Test Coverage: ⚠️ Unknown

No test files found for instance or connection classes
These changes are observability-focused, so manual testing via inspector UI is likely sufficient
Consider adding integration tests for the inspector API if not already covered elsewhere

Recommendations:

Documentation: Consider if the inspector API properties should be documented, especially since they're exposed through getConnections() which appears to be part of the inspector interface
Type safety: The inspector returns conn.params as any - consider if a more specific type could be used here for better type safety in consumers
Consider adding: If useful for debugging, you could also expose:
- Connection creation timestamp
- Number of messages sent/received
- Last message timestamp

Verdict: ✅ Approved with minor suggestions

This is a solid improvement to observability. The changes are low-risk, well-structured, and follow the project's conventions. The minor suggestions above are optional enhancements, not blockers.

Checklist:

✅ Code quality and best practices
✅ No potential bugs identified
✅ Performance impact negligible
✅ No security concerns
⚠️ Test coverage unknown (likely acceptable for observability changes)

graphite-app · 2025-11-04T20:09:51Z

Merge activity

Nov 4, 8:09 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Nov 4, 8:11 PM UTC: CI is running for this pull request on a draft pull request (#3349) due to your merge queue CI optimization settings.
Nov 4, 8:41 PM UTC: The Graphite merge queue removed this pull request due to removal of a downstack PR #3342.
Nov 4, 8:49 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Nov 4, 8:50 PM UTC: CI is running for this pull request on a draft pull request (#3352) due to your merge queue CI optimization settings.
Nov 4, 8:52 PM UTC: Merged by the Graphite merge queue via draft PR: #3352.

claude · 2025-11-04T20:34:08Z

PR Review: Improve Connection Inspector Properties

Summary

This PR enhances the connection inspector by adding more useful properties for debugging and improves logging in the connection liveness checker with aggregate statistics.

Code Quality ✅

Positive aspects:

Clean, focused changes that improve observability
Follows structured logging patterns correctly using tracing's key-value format
Properties are logically ordered (core properties first, then metadata)
Good use of existing getter methods from the Conn class

Changes Analysis

1. Enhanced Connection Inspector (lines 225-236)

Added properties:

status - Connection status (connected/reconnecting)
subscriptions - Count of active subscriptions
lastSeen - Timestamp of last activity
isHibernatable - Whether connection supports hibernation

Observations:

Property ordering change: moved stateEnabled and isHibernatable to the end, which groups runtime state together logically
All new properties use existing public getters from the Conn class, ensuring consistency
The subscriptions.size correctly accesses the Set size property

2. Improved Liveness Logging (lines 1298-1345)

Added counters:

connected - Count of active connections
reconnecting - Count of connections attempting to reconnect
removed - Count of dead connections removed

Observations:

Aggregate statistics provide better visibility into connection health
Final log message follows lowercase convention correctly: "checked connection liveness"
Structured logging properly uses field syntax: ?connected, ?reconnecting, etc.

Potential Issues ⚠️

Minor consideration:

Counter accuracy edge case: In the liveness checker, the reconnecting counter is incremented before checking if the connection will actually be removed (line 1309). This means a connection that gets removed is counted in both reconnecting AND removed.

Current behavior:
```
} else {
    reconnecting += 1;  // Incremented here
    
    const lastSeen = conn.__persist.lastSeen;
    const sinceLastSeen = Date.now() - lastSeen;
    if (sinceLastSeen < this.#config.options.connectionLivenessTimeout) {
        // Connection is actually reconnecting
        continue;
    }
    
    // Connection is dead, remove it
    removed += 1;  // Also incremented here
    this.#removeConn(conn);
}
```
The total: connected + reconnecting calculation (line 1341) will include removed connections twice in the total. Consider:
- Moving reconnecting += 1 after the timeout check, or
- Calculating total as connected + reconnecting + removed if you want all-inclusive count, or
- Accepting this behavior if "reconnecting" is meant to include "attempted reconnections before removal"
This isn't a bug per se, but may cause confusion when debugging: if you have 5 connections and remove 2, you might see total: 7 instead of 5.

Security ✅

No security concerns. Changes only affect debugging/observability.

Performance ✅

Negligible impact:

Inspector properties: O(1) getter calls
Liveness checker: 3 additional integer increments per iteration (trivial overhead)

Test Coverage

No tests included, but this is reasonable for:

Debug/inspector enhancements
Logging improvements

If you want to add tests, consider:

Verifying inspector returns correct connection properties
Validating liveness checker log output contains expected counters

Recommendations

Optional improvement:

// Option 1: Only count truly reconnecting connections
} else {
    const lastSeen = conn.__persist.lastSeen;
    const sinceLastSeen = Date.now() - lastSeen;
    if (sinceLastSeen < this.#config.options.connectionLivenessTimeout) {
        reconnecting += 1;  // Move increment here
        this.#rLog.debug({
            msg: "connection might be alive, will check later",
            connId: conn.id,
            lastSeen,
            sinceLastSeen,
        });
        continue;
    }
    
    // Connection is dead, remove it
    removed += 1;
    this.#removeConn(conn);
}

Or add a clarifying comment explaining the current counting logic if intentional.

Verdict: ✅ Approved with minor suggestion

The changes are solid and provide valuable debugging information. The counter logic issue is minor and may even be intentional depending on how you want to interpret "reconnecting". Overall, this is a quality improvement to observability.

claude · 2025-11-04T20:46:15Z

Code Review

This PR improves observability for the actor connection inspector by adding more useful properties and better logging. Overall, the changes look good and follow the project's conventions. Here's my detailed feedback:

✅ Strengths

Better Observability: The additional properties (status, subscriptions, lastSeen, isHibernatable) provide valuable runtime information for debugging and monitoring connections.
Improved Logging: The summary log at the end of #checkConnectionsLiveness() with aggregated counts (connected, reconnecting, removed) is helpful for understanding connection lifecycle at a glance.
Consistent Property Access: Using public getters (conn.status, conn.lastSeen, conn.isHibernatable) instead of private properties maintains proper encapsulation.
Logical Property Ordering: Moving stateEnabled to a more logical position in the output object improves readability.

🔍 Observations

Mixed Property Access Pattern: In getConnections(), you're mixing public API access (conn.status, conn.lastSeen) with internal access (conn.__stateEnabled). While this works, it creates an inconsistency:
- Lines 230-232, 234: Use public getters (status, lastSeen, isHibernatable)
- Line 229, 233: Use internal property (conn.__stateEnabled)
Suggestion: Consider adding a public stateEnabled getter in the Conn class if this property should be part of the public API, or document why direct access is intentional here.

Counting Logic: In #checkConnectionsLiveness(), the reconnecting counter increments for all non-connected connections, but some of those will be removed. This means:

total: connected + reconnecting will include connections that were removed
This might be confusing in logs

Consider: Either rename reconnecting to nonConnected or adjust the logic to only count connections that survive the timeout check. For example:

let connected = 0;
let alive_but_reconnecting = 0;
let removed = 0;

// ... in the loop
if (conn.__status === "connected") {
    connected += 1;
} else {
    if (sinceLastSeen < this.#config.options.connectionLivenessTimeout) {
        alive_but_reconnecting += 1;
    } else {
        removed += 1;
        this.#removeConn(conn);
    }
}

// total: connected + alive_but_reconnecting

Subscriptions Count: Good choice to expose subscriptions.size rather than the full Set - this prevents leaking internal state while still providing useful information.

📝 Minor Notes

The changes align well with the TypeScript portion of the codebase
No security concerns identified
No performance issues (the added operations are O(1) lookups)

🧪 Testing

No test changes were included. Consider adding/updating tests for:

The new inspector properties in getConnections()
The logging behavior in #checkConnectionsLiveness()

Summary

The changes improve observability without introducing bugs or breaking changes. The only suggestion is to clarify the counting logic in the liveness check to make the logs more intuitive. Otherwise, this is ready to merge! 👍

This was referenced Nov 3, 2025

chore: set minimum node version to 20 #3310

Closed

fix(rivetkit): fix "Method get ArrayBuffer.prototype.byteLength called on incompatible receiver #<ArrayBuffer>" #3312

Closed

vercel bot deployed to Preview – rivetkit-serverless November 3, 2025 22:22 View deployment

MasterPtato approved these changes Nov 4, 2025

View reviewed changes

This was referenced Nov 4, 2025

fix(guard): include method in cache key #3338

Closed

chore(guard): simplify gateway routing #3339

Closed

chore(guard): add support for routing runner ws to /runners/connect #3340

Closed

chore(guard): handle cors on the gateway #3341

Closed

NathanFlurry force-pushed the 11-02-chore_rivetkit_improve_conn_inspector_properties branch from 2b9939b to f50eb7f Compare November 4, 2025 18:59

vercel bot deployed to Preview – rivetkit-serverless November 4, 2025 19:00 View deployment

NathanFlurry force-pushed the 11-02-fix_rivetkit_don_t_throw_error_if_websocket_is_not_open_causing_realtime_to_break_after_a_connection_disconnects branch from 8872a11 to cc31979 Compare November 4, 2025 19:24

NathanFlurry force-pushed the 11-02-chore_rivetkit_improve_conn_inspector_properties branch from f50eb7f to 59e900d Compare November 4, 2025 19:24

vercel bot deployed to Preview – rivetkit-serverless November 4, 2025 19:25 View deployment

This was referenced Nov 4, 2025

chore(rivetkit): debounce ws message ack #3345

Closed

chore(rivetkit): auto-update auto-start engine version #3344

Closed

ci: trigger rust ci for generated bare artifacts #3346

Closed

graphite-app bot pushed a commit that referenced this pull request Nov 4, 2025

chore(rivetkit): improve conn inspector properties (#3317)

c0463cb

chore(rivetkit): improve conn inspector properties

4fddf6b

NathanFlurry force-pushed the 11-02-fix_rivetkit_don_t_throw_error_if_websocket_is_not_open_causing_realtime_to_break_after_a_connection_disconnects branch from cc31979 to 43ed336 Compare November 4, 2025 20:25

NathanFlurry force-pushed the 11-02-chore_rivetkit_improve_conn_inspector_properties branch from 59e900d to 4fddf6b Compare November 4, 2025 20:25

vercel bot deployed to Preview – rivetkit-serverless November 4, 2025 20:26 View deployment

graphite-app bot pushed a commit that referenced this pull request Nov 4, 2025

chore(rivetkit): improve conn inspector properties (#3317)

ce5736d

graphite-app bot closed this Nov 4, 2025

graphite-app bot deleted the 11-02-chore_rivetkit_improve_conn_inspector_properties branch November 4, 2025 20:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(rivetkit): improve conn inspector properties #3317

chore(rivetkit): improve conn inspector properties #3317

NathanFlurry commented Nov 3, 2025

Uh oh!

vercel bot commented Nov 3, 2025 •

edited

Loading

Uh oh!

NathanFlurry commented Nov 3, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Nov 3, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 3, 2025

Uh oh!

claude bot commented Nov 4, 2025

Uh oh!

graphite-app bot commented Nov 4, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 4, 2025

Uh oh!

claude bot commented Nov 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chore(rivetkit): improve conn inspector properties #3317

chore(rivetkit): improve conn inspector properties #3317

Conversation

NathanFlurry commented Nov 3, 2025

Uh oh!

vercel bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NathanFlurry commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

pkg-pr-new bot commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Nov 3, 2025

PR Review: Connection Inspector Properties Enhancement

Summary

Code Quality ✅

Implementation Review

Inspector Changes (lines 224-235)

Logging Enhancement (lines 1298-1345)

Potential Issues 🔍

Testing 🧪

Security ✅

Performance ✅

Documentation 📝

Summary Rating: Approved ✅

Uh oh!

claude bot commented Nov 4, 2025

Pull Request Review: chore(rivetkit): improve conn inspector properties

Overview

Code Quality: ✅ Good

Potential Issues: ⚠️ Minor

Performance Considerations: ✅ Good

Security Concerns: ✅ None

Test Coverage: ⚠️ Unknown

Recommendations:

Verdict: ✅ Approved with minor suggestions

Uh oh!

graphite-app bot commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

claude bot commented Nov 4, 2025

PR Review: Improve Connection Inspector Properties

Summary

Code Quality ✅

Changes Analysis

1. Enhanced Connection Inspector (lines 225-236)

2. Improved Liveness Logging (lines 1298-1345)

Potential Issues ⚠️

Security ✅

Performance ✅

Test Coverage

Recommendations

Verdict: ✅ Approved with minor suggestion

Uh oh!

claude bot commented Nov 4, 2025

Code Review

✅ Strengths

🔍 Observations

📝 Minor Notes

🧪 Testing

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel bot commented Nov 3, 2025 •

edited

Loading

NathanFlurry commented Nov 3, 2025 •

edited

Loading

pkg-pr-new bot commented Nov 3, 2025 •

edited

Loading

graphite-app bot commented Nov 4, 2025 •

edited

Loading