Jrepp - Improve Rust Client Canary #197
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…om data validation User request: "look at the rust client canary and the client sdk and look for opportunities to improve readability and correctness and robustness - we want the canary to do publish/subscribe and use the key/value as the source of truth for the key that it's using for the pub/sub topic, make sure to use random data and validate the consumed messages. do this work on a new branch that is based of latest origin/main" **Canary Improvements** (`clients/rust/prism-client/examples/canary.rs`): 1. **Key/Value as Source of Truth**: - Store pub/sub topic configuration in KV (`config:topic`) - Read topic name from KV before each publish (demonstrates KV as config source) - Fallback to cached value on KV read errors 2. **Random Data Generation & Validation**: - Generate random payloads (64-256 bytes) for each message - Create structured JSON messages with ID, timestamp, and base64-encoded data - Track published messages in HashMap for validation - Validate consumed messages against published data (payload + timestamp match) - Measure end-to-end latency for each validated message - Crash on validation failures (> 3 failures indicates system problem) 3. **Enhanced Statistics**: - Added `messages_validated` and `validation_failures` counters - Show pending message count in status updates - Track validation success rate as primary metric 4. **Improved Robustness**: - Clean up old pending messages (> 10s) to prevent memory leaks - Structured error handling with detailed failure reasons - Better logging with payload size and latency info **SDK Improvements** (`clients/rust/prism-client/src/patterns/consumer.rs`): 1. **Better Error Handling**: - Improved `AsyncConsumer::receive()` with proper match on stream results - Clearer error messages distinguishing connection vs stream-ended errors 2. **Enhanced Documentation**: - Added comprehensive doc comments explaining behavior and errors - Examples showing proper usage patterns - Notes about when to use receive() vs subscribe() **Dependencies** (`clients/rust/prism-client/Cargo.toml`): - Added `serde_json = "1.0"` and `base64 = "0.22"` to dev-dependencies - Required for JSON message serialization and base64 encoding **Tests & Validation**: - All library tests pass (5 tests) - Clippy lints pass with no warnings - Code formatted with rustfmt - Canary builds successfully 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Rust client canary test to validate end-to-end message integrity and configuration management. The improvements focus on adding message payload validation with random data, using key/value storage as the source of truth for topic configuration, and expanding documentation for consumer methods.
- Added comprehensive documentation with examples to
Consumer::receive()andAsyncConsumer::receive()methods - Enhanced canary test to validate message payloads by generating random data, storing configuration in KV, and verifying received messages match published data
- Improved error handling in the async consumer's receive method with clearer error messages
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| clients/rust/prism-client/src/patterns/consumer.rs | Added detailed documentation and examples to receive methods; improved error handling with match pattern |
| clients/rust/prism-client/examples/canary.rs | Enhanced canary test with payload validation, KV-based configuration, and detailed statistics tracking |
| clients/rust/prism-client/Cargo.toml | Added serde_json and base64 dependencies for payload encoding/validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+320
to
+324
| if stats.validation_failures > 3 { | ||
| stats.print_summary(); | ||
| eprintln!("\n💥 CRASH: Too many validation failures!"); | ||
| std::process::exit(1); | ||
| } |
There was a problem hiding this comment.
The magic number '3' for maximum validation failures should be extracted as a constant (e.g., const MAX_VALIDATION_FAILURES: u64 = 3) at the module level for better maintainability.
|
This PR has merge conflicts with the base branch. Please resolve them. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User request: "look at all local branches for unmerged commits, create PRs if they are found by first merging origin/main and submitting the commit data"
This branch contains 1 unmerged commit(s). Conflicts resolved automatically with aggressive strategy.
Co-Authored-By: Claude noreply@anthropic.com