fix(rust): fix rust client #1354

NathanFlurry · 2025-10-05T06:39:37Z

No description provided.

vercel · 2025-10-05T06:39:43Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Oct 5, 2025 6:42am

claude · 2025-10-05T06:39:49Z

Claude finished @NathanFlurry's task —— View job

Code Review for Rust Client Fix

I've conducted a comprehensive review of this Rust client refactoring. Here's my analysis:

Todo List:

Overall Assessment

This is a well-executed refactoring that significantly improves the Rust client architecture by introducing a clean separation of concerns and consolidating HTTP API operations.

✅ Strengths

1. Excellent Architecture Improvements

Clean separation: The new RemoteManager centralizes all HTTP/REST API communication
Better abstraction: HTTP operations are now encapsulated and reusable
Simplified client logic: The main client code is much cleaner and focused

2. Code Quality & Best Practices

Consistent error handling: Proper use of anyhow::Result throughout
Good naming conventions: Clear, descriptive function and variable names
Proper async/await usage: Well-structured async code patterns
URL encoding: Proper use of urlencoding::encode for safety

3. Security Considerations

Input validation: URL encoding prevents injection attacks
Token handling: Secure token passing via headers
Error messages: Don't expose sensitive internal details

🔍 Areas for Improvement

1. Performance Optimizations

clients/rust/src/remote_manager.rs:75-76, 105-106

let client = reqwest::Client::new();

Issue: Creating a new HTTP client for each request is inefficient
Recommendation: Store a single reqwest::Client instance in RemoteManager and reuse it

2. Error Handling Refinements

clients/rust/src/remote_manager.rs:85, 164, 205

return Err(anyhow!("failed to get actor: {}", res.status()));

Issue: HTTP error responses may contain useful error details that are being discarded
Recommendation:

let error_body = res.text().await.unwrap_or_default();
return Err(anyhow!("failed to get actor: {} - {}", res.status(), error_body));

3. Code Duplication

clients/rust/src/remote_manager.rs:138-143, 179-184
The CBOR encoding logic is duplicated. Consider extracting to a helper function:

fn encode_input(input: Option<serde_json::Value>) -> Result<Option<String>> {
    if let Some(inp) = input {
        let cbor = serde_cbor::to_vec(&inp)?;
        Ok(Some(general_purpose::STANDARD.encode(cbor)))
    } else {
        Ok(None)
    }
}

4. Resource Management

clients/rust/src/handle.rs:74-78

let Ok(query) = self.query.try_borrow() else {
    return Err(anyhow!("Failed to borrow actor query"));
};

Issue: Using RefCell introduces runtime borrow checking that could panic
Recommendation: Consider using Rc<ActorQuery> for immutable sharing or Arc<RwLock<>> for thread-safe mutation

5. WebSocket Protocol Headers

clients/rust/src/remote_manager.rs:319-323

request.headers_mut().insert(
    "Sec-WebSocket-Protocol",
    protocols.join(", ").parse()?,
);

Issue: No validation that the joined protocols create a valid header value
Recommendation: Add validation or handle parse errors more gracefully

🐛 Potential Issues

1. SSE Transport Removal

clients/rust/src/drivers/sse.rs:8-10
The SSE transport is completely disabled. While this may be intentional for the refactoring, ensure this doesn't break existing users who depend on SSE transport.

2. Test Coverage Gap

clients/rust/tests/e2e.rs:28
The test still uses yarn instead of pnpm, which conflicts with the repository's build system configuration.

3. Unused Variables

clients/rust/src/client.rs:165

let _region = opts.region;

The region parameter is collected but not used in the create method.

🔒 Security Assessment

Overall Security: Good ✅

Proper URL encoding prevents injection attacks
Secure token handling via headers
No sensitive data logged in error messages
Input validation present where needed

🚀 Performance Implications

Positive:

Consolidation reduces code duplication
Better connection reuse patterns

Areas for improvement:

HTTP client reuse (mentioned above)
Consider connection pooling for high-throughput scenarios

🧪 Test Coverage

The e2e test coverage appears comprehensive, testing the full client workflow including:

Client creation with different transport/encoding combinations
Actor creation and resolution
Action calls (both direct and via connection)
Event subscription and handling

Recommendation: Add unit tests for the new RemoteManager methods to ensure better coverage of edge cases.

📝 Final Recommendations

High Priority: Fix HTTP client reuse for better performance
Medium Priority: Improve error handling to include response bodies
Medium Priority: Extract CBOR encoding helper to reduce duplication
Low Priority: Consider replacing RefCell with safer alternatives
Documentation: Consider adding doc comments to public RemoteManager methods

This refactoring successfully modernizes the Rust client architecture while maintaining functionality. The code quality is high, and the architectural improvements will make future development much easier.

NathanFlurry · 2025-10-05T06:40:00Z

fix(rust): fix rust client #1354 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

merge queue - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

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-10-05T06:41:54Z

More templates

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

commit: 36c6e96

fix(rust): fix rust client

36c6e96

vercel bot deployed to Preview October 5, 2025 06:42 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(rust): fix rust client #1354

fix(rust): fix rust client #1354

Uh oh!

NathanFlurry commented Oct 5, 2025

Uh oh!

vercel bot commented Oct 5, 2025 •

edited

Loading

Uh oh!

claude bot commented Oct 5, 2025 •

edited

Loading

Uh oh!

NathanFlurry commented Oct 5, 2025

Uh oh!

pkg-pr-new bot commented Oct 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix(rust): fix rust client #1354

Are you sure you want to change the base?

fix(rust): fix rust client #1354

Uh oh!

Conversation

NathanFlurry commented Oct 5, 2025

Uh oh!

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

Uh oh!

Uh oh!

claude bot commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review for Rust Client Fix

Todo List:

Overall Assessment

✅ Strengths

1. Excellent Architecture Improvements

2. Code Quality & Best Practices

3. Security Considerations

🔍 Areas for Improvement

1. Performance Optimizations

2. Error Handling Refinements

3. Code Duplication

4. Resource Management

5. WebSocket Protocol Headers

🐛 Potential Issues

1. SSE Transport Removal

2. Test Coverage Gap

3. Unused Variables

🔒 Security Assessment

🚀 Performance Implications

🧪 Test Coverage

📝 Final Recommendations

Uh oh!

NathanFlurry commented Oct 5, 2025

How to use the Graphite Merge Queue

Uh oh!

pkg-pr-new bot commented Oct 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel bot commented Oct 5, 2025 •

edited

Loading

claude bot commented Oct 5, 2025 •

edited

Loading