Improve error types #62
Merged
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
- one crate root error - one meta module error - add Cargo features backtraces
- error types for stateful reader, stateful writer, data set reader and dataset writer - improve dicom-object error accordingly - [encoding] narrow text module result type to Err = TextEncodingError
- use trait OptionExt and selector `.fail()` method instead of backtrace generator
- replace root error with module-level snafu errors - fix writing reserved 43-74 bytes in A-ASSOCIATE-RQ - write zeros instead of the character '0' - statically resolve default text codec - move pdu write tests to inner tests module - move the remaining tests to the tests directory - fmt
- [object] convert experimental pixeldata module to snafu
- error context is on reading a header, not a value
- remove quick-error - remove error module - at text: EncodeTextError and DecodeTextError - at encode/decode::basic, error is std::io::Error - encode-level error + decode-level error types - [parser] propagate error type differences - [parser] move DicomElementMarker to an independent module - [object] propagate error type differences - [ul] propagate error type differences - remove recursion_limit attributes Misc: - remove missing_docs warn lint - very annoying for error types - refactor transfer syntax impls - split encoding and decoding - deprecate transfer_syntax specific modules Fixes: - add missing check on a write_all
- add #[snafu(backtrace)] attributes - otherwise they won't be reachable at top-level - add more backtraces where appropriate - [object] adjust UnsupportedTranserSyntax building - [encoding] optional backtrace @ decode::Error::ReadHeaderTag - since it's also part of the reading logic - [dcmdump] update error printing routine to benefit from backtraces
- ReadHeaderTag must be applied in the context of header reading, otherwise it fails to gracefully detect EOF
shepmaster
reviewed
Aug 21, 2020
shepmaster
left a comment
shepmaster
left a comment
There was a problem hiding this comment.
There's a lot of code, so I skimmed ❤️
- use std::io - remove two-step shuffles - use arrays in encoding tests instead of vectors
- split NoSuchDataElement error variant into two - update Result type alias to admit inference
- split variant InvalidValueCustom into three
- SequenceItemHeaderError at header module - remove annoying missing_docs lint - dictionary: refactor TagRangeParseError to no longer be stringly - Error at value::deserialize - move CastValueError and ConvertValueError to value::primitive - [encoding] update decode::Error accordingly - [parser] update stateful::decode::Error accordingly
will be included as a separate contribution later
- instead rely on error reporters to traverse the error chain - [dcmdump] update accordingly - [dcmdump] return -1 on missing arg or --help - [scpproxy] update accordingly
- [parser] refactor integer comparison into match and .cmp - [parser] remove .clone() in Copy struct - [object] redundant field name in struct init - [dcmdump] redundant import
std::result::Result::Ok to Result::Ok
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.
Resolves #36.
This PR replaces all existing
quick-errortypes and redesigns them with the use ofsnafu. Errors are no longer necessarily crate-level, in many cases they are scoped to a major module (one forencoding::decode, one forencoding::encode, one forobject::meta, etc). This also led to an improved context being passed into the errors, such as the reading/writing position of the stateful encoder/decoder.To serve as an example, these changes turn this error message:
into this message, plus backtrace:
Pending concerns:
dicom_corecratedcmdump, the current error definitions and Cargo features will make it so that backtraces are nearly always generated. On the other hand, the guide suggests the use ofOption<Backtrace>for errors which may take part in control flow.CC @shepmaster Your feedback on the use of SNAFU has so far has been highly appreciated. 🙏