Add ping-pong example with middleware #105
Conversation
Reviewer's GuideAdds a new ping-pong example demonstrating routing, serialization, and middleware, updates README with an Examples section and run instructions, and marks the initial examples item as complete in the roadmap. Sequence diagram for ping-pong request handling with middlewaresequenceDiagram
participant Client
participant WireframeServer
participant LoggingService
participant PongService
participant HandlerService
Client->>WireframeServer: Send Ping message
WireframeServer->>LoggingService: Pass request
LoggingService->>PongService: Pass request
PongService->>HandlerService: Call ping_handler (no-op)
HandlerService-->>PongService: Return
PongService->>LoggingService: Return Pong or ErrorMsg
LoggingService->>WireframeServer: Return response
WireframeServer->>Client: Send Pong or ErrorMsg
Class diagram for ping_pong.rs example with middlewareclassDiagram
class Ping {
+u32 0
+to_bytes()
+from_bytes()
}
class Pong {
+u32 0
+to_bytes()
+from_bytes()
}
class ErrorMsg {
+String 0
+to_bytes()
+from_bytes()
}
class PongMiddleware {
}
class PongService {
+inner: S
+call(req: ServiceRequest) Result<ServiceResponse, Infallible>
}
class Logging {
}
class LoggingService {
+inner: S
+call(req: ServiceRequest) Result<ServiceResponse, Infallible>
}
class HandlerService {
+id()
+from_service(id, service)
}
PongMiddleware --|> Transform
PongService --|> Service
Logging --|> Transform
LoggingService --|> Service
HandlerService <.. PongService : inner
HandlerService <.. LoggingService : inner
PongMiddleware ..> HandlerService : transform
Logging ..> HandlerService : transform
WireframeApp <.. build_app : factory
WireframeServer <.. main : uses
build_app --> WireframeApp
main --> WireframeServer
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new Rust example, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WireframeServer
participant PongMiddleware
participant Logging
participant HandlerService
Client->>WireframeServer: Send Ping message
WireframeServer->>Logging: Pass request frame
Logging->>PongMiddleware: Pass request frame
PongMiddleware->>HandlerService: Decode Ping, call handler
HandlerService-->>PongMiddleware: Return (no action)
PongMiddleware-->>Logging: Encode Pong, return frame
Logging-->>WireframeServer: Log and return response frame
WireframeServer-->>Client: Send Pong message
Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Consider simplifying
ping_handlerby making it anasync fninstead of boxing a pinned future for better readability. - There are several
unwrap()calls on (de)serialization that can panic at runtime—consider propagating or handling errors to make the example more robust. - The
Transformimplementations forPongMiddlewareandLoggingshare a lot of boilerplate; extracting a small helper or macro could DRY up the middleware setup.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider simplifying `ping_handler` by making it an `async fn` instead of boxing a pinned future for better readability.
- There are several `unwrap()` calls on (de)serialization that can panic at runtime—consider propagating or handling errors to make the example more robust.
- The `Transform` implementations for `PongMiddleware` and `Logging` share a lot of boilerplate; extracting a small helper or macro could DRY up the middleware setup.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)docs/roadmap.md(1 hunks)examples/ping_pong.rs(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[typographical] ~181-~181: Consider using an em dash in dialogues and enumerations.
Context: ...the examples/ directory: - echo.rs – minimal echo server using routing - `pi...
(DASH_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test
🔇 Additional comments (7)
docs/roadmap.md (1)
120-120: Roadmap completion accurately reflects delivered examples.The completion of this checklist item aligns perfectly with the addition of the
ping_pong.rsexample and the Examples section in the README.README.md (2)
177-184: Examples section enhances documentation clarity.The new Examples section provides clear descriptions of available examples, helping users understand the practical demonstrations of framework features.
181-183: Consider using em dashes for better typography.The enumeration would benefit from using em dashes (—) instead of hyphens (-) for better typographical consistency.
-- `echo.rs` – minimal echo server using routing -- `ping_pong.rs` – showcases serialization and middleware in a ping/pong +- `echo.rs` — minimal echo server using routing +- `ping_pong.rs` — showcases serialization and middleware in a ping/pong protocolLikely an incorrect or invalid review comment.
examples/ping_pong.rs (4)
1-18: Message types and imports are well-structured.The bincode derives and imports are appropriate for demonstrating serialization and middleware functionality.
39-45: Middleware logic correctly demonstrates request/response transformation.The PongService effectively shows how middleware can intercept requests, process them, and transform responses. The ping-pong logic is sound.
58-87: Logging middleware provides clear demonstration of request/response inspection.The implementation effectively shows how middleware can observe traffic without modification, using simple console output appropriate for an example.
91-101: Server setup correctly demonstrates factory pattern and middleware chaining.The middleware ordering is logical: PongMiddleware transforms responses, then Logging observes the final traffic.
examples/ping_pong.rs
Outdated
| .unwrap() | ||
| .serializer(BincodeSerializer) | ||
| .route(PING_ID, Arc::new(ping_handler)) | ||
| .unwrap() | ||
| .wrap(PongMiddleware) | ||
| .unwrap() | ||
| .wrap(Logging) | ||
| .unwrap() |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider consolidating error handling in factory function.
Multiple unwrap() calls in the factory could be replaced with a more elegant error handling approach.
let factory = || {
- WireframeApp::new()
- .unwrap()
+ WireframeApp::new()?
.serializer(BincodeSerializer)
- .route(PING_ID, Arc::new(ping_handler))
- .unwrap()
- .wrap(PongMiddleware)
- .unwrap()
- .wrap(Logging)
- .unwrap()
+ .route(PING_ID, Arc::new(ping_handler))?
+ .wrap(PongMiddleware)?
+ .wrap(Logging)
};Note: This would require adjusting the factory's return type to handle the Result.
🤖 Prompt for AI Agents
In examples/ping_pong.rs around lines 93 to 100, multiple unwrap() calls are
used in sequence, which can cause panics and reduce code clarity. Refactor the
factory function to return a Result type and use combinators like ? to propagate
errors instead of unwrap(). This consolidates error handling, making the code
safer and more readable by handling all errors in one place.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Consider adding a usage snippet or client command to the README so users can quickly try out the ping_pong example.
- In PongService, rather than silently returning
ServiceResponse::default()on decode/encode errors, consider propagating an error or returning a structured error response to aid debugging. - You may inline the small
wrap_servicehelper or move it to a shared util module to reduce indirection in the example code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a usage snippet or client command to the README so users can quickly try out the ping_pong example.
- In PongService, rather than silently returning `ServiceResponse::default()` on decode/encode errors, consider propagating an error or returning a structured error response to aid debugging.
- You may inline the small `wrap_service` helper or move it to a shared util module to reduce indirection in the example code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
examples/ping_pong.rs (2)
28-36: Excellent improvement to handler documentation.The explanatory comments now clearly explain why the handler is intentionally empty, addressing the previous concern about potential confusion for users learning the framework.
115-125: Multiple unwrap() calls still present in factory function.The factory function continues to use multiple
unwrap()calls which could cause panics and doesn't demonstrate good error handling practices for learners.Consider refactoring the factory to return a
Resulttype:- let factory = || { + let factory = || -> Result<_, Box<dyn std::error::Error>> { - WireframeApp::new() - .unwrap() + Ok(WireframeApp::new()? .serializer(BincodeSerializer) - .route(PING_ID, Arc::new(ping_handler)) - .unwrap() - .wrap(PongMiddleware) - .unwrap() - .wrap(Logging) - .unwrap() + .route(PING_ID, Arc::new(ping_handler))? + .wrap(PongMiddleware)? + .wrap(Logging)?) };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
examples/ping_pong.rs(1 hunks)
🔇 Additional comments (2)
examples/ping_pong.rs (2)
1-18: Clean imports and utility function.The imports are well-organised and the
wrap_servicehelper function provides a clean abstraction for converting services.
51-69: Excellent improvement to error handling.The error handling has been significantly improved from the previous version. Using match statements instead of
unwrap()demonstrates proper error handling practices and makes the example more educational.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@sourcery-ai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `examples/ping_pong.rs:54` </location>
<code_context>
+ }
+ };
+ let mut response = self.inner.call(req).await?;
+ let pong_resp = Pong(ping_req.0 + 1);
+ match pong_resp.to_bytes() {
+ Ok(bytes) => *response.frame_mut() = bytes,
</code_context>
<issue_to_address>
Check for potential overflow when incrementing ping_req.0.
Use `ping_req.0.checked_add(1)` to prevent a potential panic if `ping_req.0` is at `u32::MAX`, and handle the overflow appropriately.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- In README’s Examples section, add similar run instructions for
echo.rs(e.g.cargo run --example echo) to keep consistency with theping_pongexample. - In
ping_pong.rs, you have repeated error‐encoding logic inPongService—consider extracting that into a small helper to reduce duplication and improve readability. - Rather than hardcoding the bind address in
main(), consider making it configurable (via a CLI argument or environment variable) to make the example more flexible.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In README’s Examples section, add similar run instructions for `echo.rs` (e.g. `cargo run --example echo`) to keep consistency with the `ping_pong` example.
- In `ping_pong.rs`, you have repeated error‐encoding logic in `PongService`—consider extracting that into a small helper to reduce duplication and improve readability.
- Rather than hardcoding the bind address in `main()`, consider making it configurable (via a CLI argument or environment variable) to make the example more flexible.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
examples/ping_pong.rs(1 hunks)
🧰 Additional context used
🪛 GitHub Check: build-test
examples/ping_pong.rs
[failure] 120-120:
missing generics for struct wireframe::middleware::HandlerService
[failure] 89-89:
missing generics for struct wireframe::middleware::HandlerService
[failure] 119-119:
missing generics for struct wireframe::middleware::HandlerService
[failure] 122-122:
missing generics for struct wireframe::middleware::HandlerService
[failure] 88-88:
missing generics for struct wireframe::middleware::HandlerService
[failure] 91-91:
missing generics for struct wireframe::middleware::HandlerService
🪛 GitHub Actions: CI
examples/ping_pong.rs
[error] 91-91: Rust compiler error E0107: missing generics for struct wireframe::middleware::HandlerService.
🔇 Additional comments (6)
examples/ping_pong.rs (6)
1-21: Well-structured message types and imports.The imports are appropriate for the framework usage, and the message type definitions with bincode serialization are properly implemented.
23-28: Good documentation for the intentionally empty handler.The documentation clearly explains the purpose of the empty handler and how middleware generates the response. This addresses the educational concern effectively.
43-85: Excellent error handling implementation.The comprehensive error handling addresses all previous concerns, including proper overflow protection with
checked_add()and graceful handling of encoding/decoding failures. This demonstrates good practices for production code.
128-134: Good improvement in error handling.The use of the
?operator for error propagation instead ofunwrap()calls addresses previous concerns and demonstrates proper error handling practices.
136-144: Proper error handling in main function.The address parsing error handling with
map_errproperly addresses previous concerns aboutunwrap()usage, demonstrating good error handling practices.
88-95: Verify the HandlerService generic requirements.The compilation failures indicate that
HandlerServicerequires generic type parameters that aren't provided. This prevents the example from building successfully.#!/bin/bash # Description: Search for HandlerService usage patterns in the codebase to understand required generics echo "=== Searching for HandlerService definitions and usage patterns ===" rg -A 5 -B 5 "struct HandlerService" echo -e "\n=== Searching for Transform implementations with HandlerService ===" rg -A 10 "impl.*Transform.*HandlerService" echo -e "\n=== Searching for other HandlerService generic usage ===" rg "HandlerService<"This will help determine the correct generic parameters needed for
HandlerServicein the middleware implementations.Also applies to: 118-126
| #[async_trait] | ||
| impl Transform<HandlerService> for PongMiddleware { | ||
| type Output = HandlerService; | ||
|
|
||
| async fn transform(&self, service: HandlerService) -> Self::Output { | ||
| let id = service.id(); | ||
| HandlerService::from_service(id, PongService { inner: service }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Missing generic parameters for HandlerService.
The Transform implementation references HandlerService without required generic parameters, causing compilation failures.
Based on the static analysis hints, HandlerService requires generic parameters. You'll need to specify the appropriate type parameters:
#[async_trait]
-impl Transform<HandlerService> for PongMiddleware {
- type Output = HandlerService;
+impl Transform<HandlerService</* appropriate generic params */>> for PongMiddleware {
+ type Output = HandlerService</* appropriate generic params */>;
- async fn transform(&self, service: HandlerService) -> Self::Output {
+ async fn transform(&self, service: HandlerService</* appropriate generic params */>) -> Self::Output {
let id = service.id();
HandlerService::from_service(id, PongService { inner: service })
}
}Please check the wireframe framework documentation to determine the correct generic parameters for HandlerService.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: build-test
[failure] 89-89:
missing generics for struct wireframe::middleware::HandlerService
[failure] 88-88:
missing generics for struct wireframe::middleware::HandlerService
[failure] 91-91:
missing generics for struct wireframe::middleware::HandlerService
🪛 GitHub Actions: CI
[error] 91-91: Rust compiler error E0107: missing generics for struct wireframe::middleware::HandlerService.
🤖 Prompt for AI Agents
In examples/ping_pong.rs around lines 87 to 95, the Transform implementation for
PongMiddleware uses HandlerService without specifying its required generic
parameters, causing compilation errors. Review the wireframe framework
documentation to identify the correct generic parameters for HandlerService and
update the Transform trait implementation to include these parameters
consistently in the type references and method signatures.
| #[async_trait] | ||
| impl Transform<HandlerService> for Logging { | ||
| type Output = HandlerService; | ||
|
|
||
| async fn transform(&self, service: HandlerService) -> Self::Output { | ||
| let id = service.id(); | ||
| HandlerService::from_service(id, LoggingService { inner: service }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Missing generic parameters for HandlerService in Logging middleware.
Similar to PongMiddleware, the Logging middleware has the same compilation issue with missing generic parameters.
Apply the same fix pattern as needed for PongMiddleware:
#[async_trait]
-impl Transform<HandlerService> for Logging {
- type Output = HandlerService;
+impl Transform<HandlerService</* appropriate generic params */>> for Logging {
+ type Output = HandlerService</* appropriate generic params */>;
- async fn transform(&self, service: HandlerService) -> Self::Output {
+ async fn transform(&self, service: HandlerService</* appropriate generic params */>) -> Self::Output {
let id = service.id();
HandlerService::from_service(id, LoggingService { inner: service })
}
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: build-test
[failure] 120-120:
missing generics for struct wireframe::middleware::HandlerService
[failure] 119-119:
missing generics for struct wireframe::middleware::HandlerService
[failure] 122-122:
missing generics for struct wireframe::middleware::HandlerService
🤖 Prompt for AI Agents
In examples/ping_pong.rs around lines 118 to 126, the Logging middleware
implementation for Transform is missing the required generic parameters for
HandlerService, causing compilation errors. Update the Transform trait
implementation and the HandlerService type to include the appropriate generic
parameters, matching the pattern used in PongMiddleware. This involves
specifying the generic types explicitly wherever HandlerService is referenced
within the transform method and the impl block.
|
@coderabbitai review |
|
@sourcery-ai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
examples/ping_pong.rs (1)
116-123: Critical: Same generic parameter issue in Logging middleware.The same
HandlerServicegeneric parameter issue affects theLoggingmiddleware implementation.This needs the same fix as applied to
PongMiddlewareonce the correct generic parameters are determined.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)examples/ping_pong.rs(1 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[typographical] ~209-~209: Consider using an em dash in dialogues and enumerations.
Context: ...the examples/ directory: - echo.rs – minimal echo server using routing - `pa...
(DASH_RULE)
[typographical] ~212-~212: Consider using an em dash in dialogues and enumerations.
Context: ...e HashMap and Vec. - ping_pong.rs – showcases serialization and middleware ...
(DASH_RULE)
🪛 GitHub Check: build-test
examples/ping_pong.rs
[failure] 117-117:
missing generics for struct wireframe::middleware::HandlerService
[failure] 86-86:
missing generics for struct wireframe::middleware::HandlerService
[failure] 116-116:
missing generics for struct wireframe::middleware::HandlerService
[failure] 119-119:
missing generics for struct wireframe::middleware::HandlerService
[failure] 85-85:
missing generics for struct wireframe::middleware::HandlerService
[failure] 88-88:
missing generics for struct wireframe::middleware::HandlerService
🔇 Additional comments (5)
README.md (1)
207-227: Excellent documentation of the new examples!The Examples section clearly describes both example programs and provides helpful instructions for running and testing them. The inclusion of the netcat command and hex dump example is particularly useful for demonstrating the ping-pong protocol.
examples/ping_pong.rs (4)
21-30: Excellent error handling utility function.The
encode_errorfunction demonstrates robust error handling by gracefully handling encoding failures and providing fallback behaviour.
65-70: Great overflow protection implementation.Using
checked_add(1)prevents potential panics and handles the overflow case gracefully with proper error messaging.
125-131: Well-structured application setup and error handling.The
build_appfunction andmainfunction demonstrate good practices with proper error propagation and clean separation of concerns. The address parsing includes proper error handling without usingunwrap().Also applies to: 135-145
85-92: ```shell
#!/bin/bashDisplay Envelope struct definition and its context
sed -n '170,240p' src/app.rs
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| Example programs are available in the `examples/` directory: | ||
|
|
||
| - `echo.rs` – minimal server that echoes incoming frames. | ||
| - `echo.rs` – minimal echo server using routing |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using em dashes for consistency.
The documentation uses regular hyphens in the example descriptions. Consider using em dashes for better typographical consistency.
-- `echo.rs` – minimal echo server using routing
+- `echo.rs` — minimal echo server using routing-- `ping_pong.rs` – showcases serialization and middleware in a ping/pong
+- `ping_pong.rs` — showcases serialization and middleware in a ping/pongAlso applies to: 212-212
🧰 Tools
🪛 LanguageTool
[typographical] ~209-~209: Consider using an em dash in dialogues and enumerations.
Context: ...the examples/ directory: - echo.rs – minimal echo server using routing - `pa...
(DASH_RULE)
🤖 Prompt for AI Agents
In README.md at lines 209 and 212, replace the regular hyphens used in example
descriptions with em dashes to improve typographical consistency. Locate the
hyphens separating the example names and their descriptions and substitute them
with em dashes (—) while preserving spacing around the dash for readability.
1 participant
Summary
Testing
mdformat-all README.md docs/*.md examples/ping_pong.rs examples/echo.rsmarkdownlint --fix AGENTS.md CHANGELOG.md README.md docs/mocking-network-outages-in-rust.md docs/preamble-validator.md docs/roadmap.md docs/rust-binary-router-library-design.md docs/rust-testing-with-rstest-fixtures.mdnixie README.md docs/*.mdmake fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_6857265325588322a829a52b18a77a13
Summary by Sourcery
Introduce a new ping-pong example showcasing middleware chaining and Bincode serialization, update the README to list examples with usage commands, and mark the related roadmap task as done.
New Features:
Documentation:
Summary by CodeRabbit