Refactor errors, buffer sizing, tests, and docs; add fixtures#456
Merged
Refactor errors, buffer sizing, tests, and docs; add fixtures#456
Conversation
- Introduces v0.1.0 to v0.2.0 migration guide detailing breaking changes - Notes removal of deprecated payload() methods in favor of into_payload() - Updates existing docs for minor formatting improvements - Removes deprecated payload accessor methods from PacketParts and FragmentParts These changes improve developer experience by documenting required migration steps and cleaning up deprecated APIs. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Move preamble tests into grouped modules under tests/preamble/\nwith shared helpers to keep file sizes under the limit.\n\nAdd a Makefile typecheck target to match the commit gates.
Address review feedback by clarifying the migration guide intro and\nkeeping payload-accessor documentation neutral.\n\nRefactor preamble tests to avoid excessive nesting and add helper\nRustdoc examples for shared test utilities.
Propagate response send errors and tidy logging output. Update client builder docs and buffer sizing constant. Refactor extractor errors/docs and server config tests to align with fixture guidance. Refresh migration guide wording and payload accessor example.
Contributor
Reviewer's GuideRefactors error handling, buffer sizing, and tests across framing, extractor, client, and server components, tightening invariants, improving logging, and consolidating test coverage while making small documentation and API ergonomics tweaks. Updated class diagram for extractor error handling and state accessclassDiagram
class MessageRequest {
+Option<SocketAddr> peer_addr
+Option<StreamingBody> body
+HashMap<TypeId, Arc<dyn Any+Send+Sync>> app_data
+new() MessageRequest
+with_peer_addr(peer_addr: Option<SocketAddr>) MessageRequest
+state<T>() Option<SharedState<T>>
+insert_state<T>(state: T)
}
class SharedState~T~ {
-Arc<T> inner
+deref() &T
+into_inner(self) T
}
class ExtractError {
<<enum>>
MissingState(ty: &'static str)
InvalidPayload(source: bincode::error::DecodeError)
MissingBodyStream
}
class ConnectionInfo {
-Option<SocketAddr> peer_addr
+peer_addr(&self) Option<SocketAddr>
+from_message_request(req: &MessageRequest, payload: &mut Payload) Result<ConnectionInfo, ExtractError>
}
class Payload {
+default() Payload
}
MessageRequest "1" o-- "*" SharedState : stores
SharedState "1" --> "1" ExtractError : may produce
ConnectionInfo --> MessageRequest : extracts_from
ConnectionInfo --> Payload : uses
ConnectionInfo --> ExtractError : error_type
Updated class diagram for app and client builders with timeouts and buffer limitsclassDiagram
class WireframeApp~S,C,E,F~ {
+HashMap<RouteKey, Handler> handlers
+OnceCell<Router> routes
+Vec<Middleware> middleware
+S serializer
+HashMap<TypeId, Arc<dyn Any+Send+Sync>> app_data
+Option<Arc<ConnectionSetup<C>>> on_connect
+Option<Arc<ConnectionTeardown<C>>> on_disconnect
+Option<ProtocolConfig> protocol
+Option<PushDlq> push_dlq
+F codec
+u64 read_timeout_ms
+Option<FragmentationConfig> fragmentation
+Option<MessageAssembler> message_assembler
+rebuild_with_connection_type~C2~(self, on_connect: Option<Arc<ConnectionSetup<C2>>>, on_disconnect: Option<Arc<ConnectionTeardown<C2>>>) WireframeApp~S,C2,E,F~
}
class ConnectionSetup~C~ {
<<type_alias>>
+call() Pin<Box<dyn Future<Output=C>+Send>>
}
class ConnectionTeardown~C~ {
<<type_alias>>
+call(ctx: C) Pin<Box<dyn Future<Output=()>+Send>>
}
class BuilderDefaults {
<<module_consts>>
+u64 MIN_READ_TIMEOUT_MS = 1
+u64 MAX_READ_TIMEOUT_MS = 86400000
+u64 DEFAULT_READ_TIMEOUT_MS = 100
}
class WireframeClientBuilder~S,Ctx,E,Codec~ {
+CodecConfig codec_config
+on_error(handler: fn(ClientError) -> impl Future<Output=()>+Send) WireframeClientBuilder~S,Ctx,E,Codec~
+build_with_stream(stream: TlsOrTcpStream, leftover: Bytes) WireframeClient~S,Ctx,E,Codec~
}
class CodecConfig {
+max_frame_length_value() usize
+build_codec() Codec
}
class Framed~IO,Codec~ {
+new(io: IO, codec: Codec) Framed~IO,Codec~
+read_buffer_mut() &mut BytesMut
}
class RewindStream~IO~ {
+new(leftover: Bytes, stream: IO) RewindStream~IO~
}
class ClientConsts {
<<module_consts>>
+usize INITIAL_READ_BUFFER_CAPACITY_LIMIT = 64*1024
}
BuilderDefaults <.. WireframeApp : uses_defaults
WireframeApp --> ConnectionSetup : on_connect
WireframeApp --> ConnectionTeardown : on_disconnect
WireframeClientBuilder --> CodecConfig : has
WireframeClientBuilder --> ClientConsts : uses
WireframeClientBuilder --> Framed : constructs
WireframeClientBuilder --> RewindStream : wraps
CodecConfig --> Framed : configures
Updated class diagram for codec recovery configuration and policy hooksclassDiagram
class RecoveryPolicy {
<<enum>>
Drop
Disconnect
Quarantine(Duration)
}
class CodecErrorContext {
+Option<SocketAddr> peer_address
+new() CodecErrorContext
+set_peer_address(addr: SocketAddr)
}
class CodecError {
<<enum>>
Framing(FramingError)
Io(io::Error)
Eof(EofError)
+default_recovery_policy(&self) RecoveryPolicy
}
class RecoveryPolicyHook {
<<trait>>
+recovery_policy(&self, error: &CodecError, ctx: &CodecErrorContext) RecoveryPolicy
+quarantine_duration(&self, error: &CodecError, ctx: &CodecErrorContext) Duration
}
class DefaultRecoveryPolicy {
}
class RecoveryConfig {
<<data>>
+u32 max_consecutive_drops
+Duration quarantine_duration
+new() RecoveryConfig
}
RecoveryPolicyHook <|.. DefaultRecoveryPolicy : implements
DefaultRecoveryPolicy --> CodecError : delegates_policy
DefaultRecoveryPolicy --> CodecErrorContext : uses
RecoveryConfig --> RecoveryPolicy : configures
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
Summary by CodeRabbitRelease Notes
WalkthroughThis pull request consolidates internal refactoring, extends error handling, introduces new extractor types for message handling, refactors test infrastructure with fixtures and parameterised scenarios, and updates documentation to clarify API constraints and naming conventions across the codebase. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
Reuse a shared helper when changing connection state types. This keeps lifecycle builder reconstruction aligned with the core builder and removes the ad hoc OnceCell instantiation.
…e scenarios Expanded and diversified the backoff configuration tests for WireframeServer to cover multiple edge cases and delay configurations. Added parameterized tests using rstest, verifying initial and max delay clamping, swapping, and defaults for better test coverage and robustness. Also: - Cleaned up unused test utility functions and redundant assertions. - Enhanced diagnostics in test assertions with scenario descriptions. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Ensure rstest fixtures avoid clippy lint issues by adding\nexplicit harness assertions and applying rustfmt-friendly layout\nfor test setup helpers.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
changed the title
Adjust response logging and public doc comments in frame handling,\nand reuse a shared framed harness with client/server endpoints\nfor response tests. Also normalize message docs to use\nOxford -ize spelling.
Combine send_response_payload cases with a shared harness that\nincludes both client and server framed endpoints, reducing\nduplicate setup while preserving payload assertions.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: Code Duplicationsrc/server/config/tests.rs: What lead to degradation?The module contains 2 functions with similar structure: test_bind_success,test_local_addr_after_bind Why does this problem occur?Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health. How to fix it?A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More |
This comment was marked as resolved.
This comment was marked as resolved.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
default_hook,context, andio_errorfixtures incodec/recovery.rscontainlet () = ();statements that are no-ops and can be removed to reduce noise without changing behavior. - In
frame_handling.rstests,build_harnessconstructs both server and clientCombinedCodecinstances from the sameTestCodecvalue; ifTestCodec’s internal state is meant to model per-endpoint behavior, consider cloning or constructing separate codec instances to avoid shared mutable state across the two sides.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `default_hook`, `context`, and `io_error` fixtures in `codec/recovery.rs` contain `let () = ();` statements that are no-ops and can be removed to reduce noise without changing behavior.
- In `frame_handling.rs` tests, `build_harness` constructs both server and client `CombinedCodec` instances from the same `TestCodec` value; if `TestCodec`’s internal state is meant to model per-endpoint behavior, consider cloning or constructing separate codec instances to avoid shared mutable state across the two sides.
## Individual Comments
### Comment 1
<location> `src/codec/recovery.rs:338` </location>
<code_context>
+ }
+ }
+
+ #[fixture]
+ fn default_harness() -> FramedHarness {
+ let harness = build_harness(64);
</code_context>
<issue_to_address>
**issue (complexity):** Consider inlining or replacing the trivial `rstest` fixtures with direct construction or simple helpers to reduce indirection in these tests.
The new `rstest` fixtures do add indirection without pulling their weight. You can keep the new behavior and `rstest` usage while simplifying the tests and removing the no-op abstraction.
### 1. Remove trivial fixtures and construct values inline
The fixtures:
```rust
#[fixture]
fn default_hook() -> DefaultRecoveryPolicy {
let () = ();
DefaultRecoveryPolicy
}
#[fixture]
fn context() -> CodecErrorContext {
let () = ();
CodecErrorContext::new()
}
#[fixture]
fn io_error() -> CodecError {
let () = ();
CodecError::Io(io::Error::other("test"))
}
```
are effectively just `Default::default()`/`new`/direct constructors, used only once each. You can inline them in the tests while still using `rstest`:
```rust
#[rstest]
fn default_recovery_policy_delegates_to_error() {
use super::super::error::{EofError, FramingError};
let default_hook = DefaultRecoveryPolicy;
let context = CodecErrorContext::new();
let err = CodecError::Framing(FramingError::OversizedFrame { size: 100, max: 50 });
assert_eq!(
default_hook.recovery_policy(&err, &context),
RecoveryPolicy::Drop
);
let err = CodecError::Io(io::Error::other("test"));
assert_eq!(
default_hook.recovery_policy(&err, &context),
RecoveryPolicy::Disconnect
);
let err = CodecError::Eof(EofError::CleanClose);
assert_eq!(
default_hook.recovery_policy(&err, &context),
RecoveryPolicy::Disconnect
);
}
#[rstest]
fn default_quarantine_duration_is_30_seconds() {
let default_hook = DefaultRecoveryPolicy;
let context = CodecErrorContext::new();
let io_error = CodecError::Io(io::Error::other("test"));
assert_eq!(
default_hook.quarantine_duration(&io_error, &context),
Duration::from_secs(30)
);
}
```
This keeps:
- All current behavior and assertions.
- The `rstest` attribute (so you can later add parameterization if needed).
But removes:
- The three separate fixtures and their `let () = ();` no-ops.
- The cognitive overhead of chasing fixtures for very simple constructors.
### 2. If you do want shared construction logic, use plain helpers
If you expect these constructors to evolve and want a single point of change, simple helper functions are enough and avoid the fixture machinery:
```rust
fn make_default_hook() -> DefaultRecoveryPolicy {
DefaultRecoveryPolicy
}
fn make_context() -> CodecErrorContext {
CodecErrorContext::new()
}
fn make_io_error() -> CodecError {
CodecError::Io(io::Error::other("test"))
}
```
Then in tests:
```rust
#[rstest]
fn default_quarantine_duration_is_30_seconds() {
let default_hook = make_default_hook();
let context = make_context();
let io_error = make_io_error();
assert_eq!(
default_hook.quarantine_duration(&io_error, &context),
Duration::from_secs(30)
);
}
```
Either approach reduces complexity while preserving the new test behavior and keeping the door open for future `rstest` parameterization.
</issue_to_address>
### Comment 2
<location> `docs/v0-1-0-to-v0-2-0-migration-guide.md:3` </location>
<code_context>
-This guide summarizes the breaking changes required when migrating from
-wireframe v0.1.0 to v0.2.0.
+This guide summarizes the breaking changes you need to address when migrating
+from wireframe v0.1.0 to v0.2.0.
</code_context>
<issue_to_address>
**issue (review_instructions):** The new sentence uses the 2nd person pronoun "you", which the style guide forbids.
Consider rephrasing to avoid direct 2nd person, for example: "This guide summarizes the breaking changes that must be addressed when migrating from wireframe v0.1.0 to v0.2.0."
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.md`
**Instructions:**
Avoid 2nd person or 1st person pronouns ("I", "you", "we").
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2842e57ebc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/v0-1-0-to-v0-2-0-migration-guide.md`:
- Around line 3-4: The phrase "breaking changes you need to address when
migrating" uses a second-person pronoun; change it to a neutral, impersonal
phrasing by replacing "you need to address" with something like "to address" or
"that must be addressed" (i.e., update the sentence "This guide summarizes the
breaking changes you need to address when migrating from wireframe v0.1.0 to
v0.2.0." to "This guide summarizes the breaking changes to address when
migrating from wireframe v0.1.0 to v0.2.0." or "This guide summarizes breaking
changes that must be addressed when migrating from wireframe v0.1.0 to
v0.2.0.").
In `@src/app/frame_handling.rs`:
- Around line 255-260: Change the fixture to return a Result instead of
panicking: update fn default_harness() -> FramedHarness to something like fn
default_harness() -> Result<FramedHarness, Box<dyn std::error::Error>> (or your
crate's error type), call let harness = build_harness(64); then validate with if
harness.codec.max_frame_length() != 64 { return Err(format!("unexpected
max_frame_length: {}", harness.codec.max_frame_length()).into()); } and finally
Ok(harness); also update tests that consume default_harness to return Result and
call the fixture with ? so failures propagate rather than panic; reference
symbols: default_harness, FramedHarness, build_harness,
codec.max_frame_length().
In `@src/codec/recovery.rs`:
- Around line 338-355: The three fixture functions default_hook, context, and
io_error contain redundant no-op unit bindings (`let () = ();`) — remove those
lines from each fixture so the functions simply return DefaultRecoveryPolicy,
CodecErrorContext::new(), and CodecError::Io(io::Error::other("test"))
respectively; update only the bodies of the functions (default_hook, context,
io_error) to drop the `let () = ();` statements.
…fig tests - Remove unused rstest fixtures in codec recovery tests and initialize variables directly within test functions - Add helper assert_local_addr_matches_listener function in server config tests to reduce duplication - Refactor server config tests to use the new helper for clearer, more concise assertions Additionally: - Fix client codec initialization in frame_handling tests for clarity - Minor wording improvement in migration guide docs Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Have the following now been resolved? |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/codec/recovery.rs (2)
330-420: 🛠️ Refactor suggestion | 🟠 MajorSplit this module to keep it under 400 lines.
Move the test module into a dedicated file to bring the module below the 400-line limit.
As per coding guidelines, "Files must not exceed 400 lines in length."
334-398: 🧹 Nitpick | 🔵 TrivialRefactor tests to use rstest fixtures for shared setup.
Both
default_recovery_policy_delegates_to_erroranddefault_quarantine_duration_is_30_secondsduplicate identical initialisation ofDefaultRecoveryPolicyandCodecErrorContext. Introduce fixtures to centralise this setup:Suggested refactor
- use rstest::rstest; + use rstest::{fixture, rstest}; + + #[fixture] + fn default_hook() -> DefaultRecoveryPolicy { + DefaultRecoveryPolicy + } + + #[fixture] + fn context() -> CodecErrorContext { + CodecErrorContext::new() + } @@ - #[rstest] - fn default_recovery_policy_delegates_to_error() { + #[test] + fn default_recovery_policy_delegates_to_error( + default_hook: DefaultRecoveryPolicy, + context: CodecErrorContext, + ) { use super::super::error::{EofError, FramingError}; - let default_hook = DefaultRecoveryPolicy; - let context = CodecErrorContext::new(); @@ - #[rstest] - fn default_quarantine_duration_is_30_seconds() { - let default_hook = DefaultRecoveryPolicy; - let context = CodecErrorContext::new(); + #[test] + fn default_quarantine_duration_is_30_seconds( + default_hook: DefaultRecoveryPolicy, + context: CodecErrorContext, + ) { let io_error = CodecError::Io(io::Error::other("test"));
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 2817 🏁 Script executed: Length of output: 353 🏁 Script executed: Length of output: 882 🏁 Script executed: Length of output: 51
✅ All Issues Resolved1. Trivial fixtures in
|
* docs(protocol): add docs clarifying that ProtocolError must be () Added detailed comments to `WireframeApp::with_protocol` and the `WireframeProtocol` trait explaining that currently `ProtocolError` must be `()`. This ensures a uniform interface for dynamic dispatch and prevents leaking application-specific errors. Also added unit test module documentation for fragmentation and reassembly. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(hooks): simplify example by removing unused error variable In the WireframeProtocol trait example, changed the handle_error function to remove the unused `error` parameter and updated the logging statement accordingly to avoid unused variable warnings and improve clarity. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Restore non-fatal handling for response fragmentation and codec encoding errors in frame handling so handler payload mistakes do not disconnect healthy client connections. Keep transport failures fatal by propagating non-encoding send errors. Also restore public visibility for MessageRequest.app_data to avoid a breaking API change and remove the panicking rstest fixture pattern in frame-handling tests.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/extractor.rs (1)
1-6:⚠️ Potential issue | 🟠 MajorDecompose this module—file exceeds 400 lines (currently 507).
Split the extractors into a submodule structure. A sensible decomposition:
src/extractor/ ├── mod.rs // re-exports, MessageRequest, Payload, FromMessageRequest trait, ExtractError ├── shared_state.rs // SharedState extractor ├── message.rs // Message<T> extractor ├── streaming.rs // StreamingBody extractor └── connection.rs // ConnectionInfo extractorThis keeps related types collocated whilst satisfying the 400-line limit. As per coding guidelines: "Files must not exceed 400 lines in length" and "Large modules must be decomposed".
src/app/frame_handling.rs (1)
237-239:⚠️ Potential issue | 🟡 MinorAdd a module-level doc comment inside
mod tests.Insert a
//!comment at Line 237 so the inner module meets the module-doc requirement.As per coding guidelines: "Every Rust module must begin with a module level (`//!`) comment explaining the module's purpose and utility."Suggested fix
#[cfg(test)] mod tests { + //! Tests for frame handling helpers and response sending. use bytes::Bytes; use futures::StreamExt; use rstest::rstest;
🤖 Fix all issues with AI agents
In `@src/app/frame_handling.rs`:
- Around line 250-277: Tests duplicate harness setup; convert build_harness into
an rstest fixture and inject it into tests. Add use rstest::{fixture, rstest};
create a #[fixture] fn harness() -> FramedHarness { build_harness(64) }; then
update send_response_payload_behaviour and response_context_holds_references
signatures to accept mut harness: FramedHarness (and keep their #[case] params)
and remove the local let mut harness = build_harness(64) calls; keep
FramedHarness, build_harness, send_response_payload_behaviour, and
response_context_holds_references names unchanged.
Move recovery tests out of src/codec/recovery.rs into src/codec/recovery/tests.rs so the production module stays under the 400-line file-size limit. Add rstest fixtures for DefaultRecoveryPolicy and CodecErrorContext and reuse them in tests that previously duplicated setup.
Decompose extractor into a submodule layout by moving concrete extractor implementations into src/extractor/extractors.rs and converting extractor to directory module layout. This keeps src/extractor/mod.rs below the 400-line threshold while preserving the public extractor API via re-exports. Update frame-handling tests to add the required module-level //! docs, introduce an rstest harness fixture, and inject it into both tests to remove duplicated setup.
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Absence of Expected Change Pattern
- wireframe/src/message.rs is usually changed with: wireframe/src/preamble.rs
Quality Gate Profile: Pay Down Tech Debt
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Changes
Documentation
Core framework
Client builder
Codec recovery tests
Extractor error handling
Server tests and fixtures
Miscellaneous
Test helpers and fixtures
Test plan
Review notes
If you want me to adjust the PR description tone or add a short changelog-like bullet list, I can update accordingly.
📎 Task: https://www.devboxer.com/task/f39504b6-9fcc-4590-9bd4-ea21171e9d02
Summary by Sourcery
Refine server configuration defaults, error handling, and tests while tightening client read buffer sizing and improving extractor ergonomics and documentation.
Bug Fixes:
Enhancements:
io::Resultinstead of swallowing them, with clearer structured logging for handler, fragmentation, and serialization failures.rstest, reducing duplication and improving coverage of backoff scenarios.PartialEqandEqfor recovery configuration to allow direct comparison in tests and make extractor errorsthiserror::Errorwith better display messages and sources.Documentation:
Tests:
connected_streamsfixture and expand backoff tests into table-driven scenarios covering clamping and parameter swapping.📎 Task: https://www.devboxer.com/task/51a5833d-284f-4bfe-9a6b-012cec4035fb