Refactor: preserve EofError in io::Error via type-safe downcast#454
Merged
Refactor: preserve EofError in io::Error via type-safe downcast#454
Conversation
…classification - Remove special case mapping of `EofError::CleanClose` to a generic IO error, preserving the original EOF error. - Add test to ensure `EofError::CleanClose` is preserved when converting to `io::Error`. - Update EOF error classification in decoder to downcast the inner error instead of relying on fragile string matching. - Improve robustness in EOF error inference logic by inspecting inner errors and buffer content. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
Reviewer's GuideRefactors EOF handling so that CodecError::Eof is wrapped in io::Error while preserving the concrete EofError as the inner source, and updates decoder test utilities and tests to classify EOF via type-safe downcasting instead of brittle string inspection. Sequence diagram for EOF error propagation and classificationsequenceDiagram
actor Client
participant Codec
participant FromCodecErrorForIoError
participant IoLayer
participant DecoderClassifier
Client->>Codec: read_frame()
Codec-->>Client: error CodecError::Eof(EofError::CleanClose)
Client->>FromCodecErrorForIoError: from(CodecError::Eof(CleanClose))
FromCodecErrorForIoError->>IoLayer: create io_Error(kind=UnexpectedEof, source=EofError::CleanClose)
IoLayer-->>Client: io_Error
Client->>DecoderClassifier: classify_eof(io_Error, buffer_state)
DecoderClassifier->>IoLayer: source()
IoLayer-->>DecoderClassifier: EofError::CleanClose
DecoderClassifier-->>Client: EofClassification(clean_close)
Class diagram for CodecError to IoError EOF mapping refactorclassDiagram
class CodecError {
<<enum>>
Io(io_Error)
Framing(FramingError)
Protocol(ProtocolError)
Eof(EofError)
}
class EofError {
<<enum>>
CleanClose
MidHeader
MidFrame
Other
}
class io_Error {
+kind: io_ErrorKind
+source: Error
+new(kind: io_ErrorKind, source: Error)
}
class io_ErrorKind {
<<enum>>
UnexpectedEof
InvalidData
Other
}
class FromCodecErrorForIoError {
+from(error: CodecError) io_Error
}
CodecError --> EofError : contains
CodecError --> io_Error : converted_via_From
FromCodecErrorForIoError --> CodecError : input
FromCodecErrorForIoError --> io_Error : output
io_Error ..> EofError : source_preserves_EofError
io_Error --> io_ErrorKind : has_kind
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 Summary by CodeRabbit
WalkthroughRemove special-case handling for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Add a test verifying EOF variants preserve UnexpectedEof kind on conversion to io::Error - Add tests for classify_eof_error method covering inner error detection, buffer fallback, and ignoring unrelated errors - Add tests for infer_eof_from_buffer covering MidHeader and MidFrame EOF error cases - Refactor decoder_ops to improve EOF error detection by searching error chain - Verify io_error_from_clean_close_preserves_eof_error with assert_eq on error kind Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Owner
Author
|
@coderabbitai have the following now been resolved? |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the current state of the codebase to see if these review comments have been addressed. 🏁 Script executed: Length of output: 17519
✅ Overall Comment - ResolvedThe code now walks the
✅ Comment 1 - ResolvedThree comprehensive tests for
✅ Comment 2 - ResolvedTwo focused tests for
✅ Comment 3 - ResolvedThe test coverage has been extended in
All suggestions have been implemented with appropriate test coverage. The refactoring is complete and well-tested! 🎉 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/codec/error_tests.rs`:
- Around line 117-157: Merge the two tests
io_error_from_clean_close_preserves_eof_error and
io_error_from_eof_variants_preserves_eof_error into a single
rstest-parameterised test that iterates over all EofError variants (include
EofError::CleanClose plus the MidFrame and MidHeader variants) and for each
converts CodecError::Eof(variant) into an io::Error and asserts io_err.kind() ==
io::ErrorKind::UnexpectedEof; when extracting the inner error use
io_err.get_ref().expect(...) and inner.downcast_ref::<EofError>().expect(...)
(replace the current unwrap_or_else(|| panic!(...)) calls), and assert_eq!(*eof,
variant); ensure the new test is annotated with #[rstest] and uses a parameter
name like variant to match the pattern used elsewhere.
In `@tests/fixtures/codec_error/decoder_ops.rs`:
- Around line 234-342: The tests module currently uses a hand-rolled helper
make_world_with_buffer and a manual loop in
classify_eof_error_uses_inner_eof_error_variant; refactor to use rstest fixtures
and parameterised cases: replace make_world_with_buffer with a #[fixture] fn
returning CodecErrorWorld (populate world.buffer in the fixture or via helper
called in the fixture), convert classify_eof_error_uses_inner_eof_error_variant
into an #[rstest] test with #[case] for each EofError variant, and inline simple
buffer setup directly in tests (or add another #[fixture] for prefilled buffers)
while keeping calls to world.classify_eof_error and assertions against
world.detected_eof and world.infer_eof_from_buffer unchanged; reference symbols:
make_world_with_buffer, CodecErrorWorld,
classify_eof_error_uses_inner_eof_error_variant, infer_eof_from_buffer, and
EofError.
…ts for EOF variants Refactor multiple manual loops and separate test functions into concise parameterized tests using the rstest crate. This simplifies and clarifies tests around EOF error variant conversions and EOF classification logic in codec error handling. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@tests/fixtures/codec_error/decoder_ops.rs`:
- Around line 234-239: Add a module-level doc comment to the test module by
inserting a `//!` comment as the first item inside `mod tests` so it explains
the purpose of the tests in this module (e.g., what behavior `decoder_ops`
fixtures validate); ensure the `//!` comment appears before any `use` statements
or items within `mod tests` and is concise and descriptive to satisfy the
module-level documentation guideline.
…coder ops tests Add documentation comment describing tests for decoder ops EOF classification and buffer inference behaviour. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
devboxerhub Bot
added a commit
that referenced
this pull request
Feb 18, 2026
* refactor(codec,error): Refactor EOF error handling and improve error classification - Remove special case mapping of `EofError::CleanClose` to a generic IO error, preserving the original EOF error. - Add test to ensure `EofError::CleanClose` is preserved when converting to `io::Error`. - Update EOF error classification in decoder to downcast the inner error instead of relying on fragile string matching. - Improve robustness in EOF error inference logic by inspecting inner errors and buffer content. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * test(codec error): add tests for EOF error classification and conversion - Add a test verifying EOF variants preserve UnexpectedEof kind on conversion to io::Error - Add tests for classify_eof_error method covering inner error detection, buffer fallback, and ignoring unrelated errors - Add tests for infer_eof_from_buffer covering MidHeader and MidFrame EOF error cases - Refactor decoder_ops to improve EOF error detection by searching error chain - Verify io_error_from_clean_close_preserves_eof_error with assert_eq on error kind Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * test(codec_error): replace manual loops with rstest parameterized tests for EOF variants Refactor multiple manual loops and separate test functions into concise parameterized tests using the rstest crate. This simplifies and clarifies tests around EOF error variant conversions and EOF classification logic in codec error handling. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(tests/fixtures/codec_error): add module-level doc comment for decoder ops tests Add documentation comment describing tests for decoder ops EOF classification and buffer inference behaviour. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
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
Core Functionality
Tests
Notes
Test plan
◳ Generated by DevBoxer ◰
ℹ️ Tag @devboxerhub to ask questions and address PR feedback
📎 Task: https://www.devboxer.com/task/b9b4b861-a655-4fd1-9ebf-b15f6a002be4
📝 Closes #434
Summary by Sourcery
Preserve precise EOF error variants when converting
CodecErrorintoio::Errorand update EOF classification in tests to use type-safe downcasting instead of string matching.Enhancements:
CodecError::Eof(EofError::CleanClose)soio::Errorconsistently wrapsEofErroras the inner source forUnexpectedEof.EofErrorvia downcasting fromio::Errorand fall back to buffer-based inference when no inner error is present.Tests:
CodecError::Eof(EofError::CleanClose)is preserved inside theio::Errorsource.