Improve error types

core/Cargo.toml

@@ -15,7 +15,7 @@ repository = "Enet4/dicom-rs"
 [dependencies]
 chrono = "0.4.6"
 itertools = "0.9.0"
-quick-error = "1.2.3"
-smallvec = "1.0.0"
-safe-transmute = "0.11.0"
 num-traits = "0.2.12"
+safe-transmute = "0.11.0"
+smallvec = "1.0.0"
+snafu = "0.6.8"

core/src/dictionary/mod.rs

@@ -7,6 +7,7 @@ pub mod stub;
 use crate::header::{Tag, VR};
 use std::fmt::Debug;
 use std::str::FromStr;
+use snafu::{Backtrace, OptionExt, ResultExt, Snafu, ensure};
 
 /// Specification of a range of tags pertaining to an attribute.
 /// Very often, the dictionary of attributes indicates a unique `(group,elem)`
@@ -35,9 +36,45 @@ impl TagRange {
     }
 }
 
+
 /// An error returned when parsing an invalid tag range.
-#[derive(Debug, Default, Copy, Clone, Eq, PartialEq)]
-pub struct TagRangeParseError(&'static str);
+#[derive(Debug, Snafu)]
+#[non_exhaustive]
+pub enum TagRangeParseError {
+    #[snafu(display("Not enough tag components, expected tag (group, element)"))]
+    MissingTag {
+        backtrace: Backtrace,
+    },
+    #[snafu(display("Not enough tag components, expected tag element"))]
+    MissingTagElement {
+        backtrace: Backtrace,
+    },
+    #[snafu(display("tag component `group` has an invalid length: got {} but must be 4", got))]
+    InvalidGroupLength {
+        got: usize,
+        backtrace: Backtrace,
+    },
+    #[snafu(display("tag component `element` has an invalid length: got {} but must be 4", got))]
+    InvalidElementLength {
+        got: usize,
+        backtrace: Backtrace,
+    },
+    #[snafu(display("unsupported tag range"))]
+    UnsupportedTagRange {
+        backtrace: Backtrace,
+    },
+    #[snafu(display("invalid tag component `group`"))]
+    InvalidTagGroup {
+        backtrace: Backtrace,
+        source: std::num::ParseIntError,
+    },
+    #[snafu(display("invalid tag component `element`"))]
+    InvalidTagElement {
+        backtrace: Backtrace,
+        source: std::num::ParseIntError,
+    },
+}
+
 
 impl FromStr for TagRange {
     type Err = TagRangeParseError;
@@ -47,49 +84,37 @@ impl FromStr for TagRange {
             s = &s[1..s.len() - 1];
         }
         let mut parts = s.split(',');
-        let group = parts.next().ok_or(TagRangeParseError(
-            "not enough tag components, expected `group,element`",
-        ))?;
-        let elem = parts.next().ok_or(TagRangeParseError(
-            "not enough tag components, expected `element`",
-        ))?;
-        if group.len() != 4 {
-            return Err(TagRangeParseError(
-                "tag component `group` has an invalid length, must be 4",
-            ));
-        }
-        if elem.len() != 4 {
-            return Err(TagRangeParseError(
-                "tag component `element` has an invalid length, must be 4",
-            ));
-        }
+        let group = parts.next().context(MissingTag)?;
+        let elem = parts.next().context(MissingTagElement)?;
+        ensure!(group.len() == 4, InvalidGroupLength { got: group.len() });
+        ensure!(elem.len() == 4, InvalidElementLength { got: elem.len() });
 
         match (&group.as_bytes()[2..], &elem.as_bytes()[2..]) {
-            (b"xx", b"xx") => Err(TagRangeParseError("unsupported tag range")),
+            (b"xx", b"xx") => UnsupportedTagRange.fail(),
             (b"xx", _) => {
                 // Group100
                 let group = u16::from_str_radix(&group[..2], 16)
-                    .map_err(|_e| TagRangeParseError("Invalid component `group`"))?
+                    .context(InvalidTagGroup)?
                     << 8;
                 let elem = u16::from_str_radix(elem, 16)
-                    .map_err(|_e| TagRangeParseError("Invalid component `element`"))?;
+                    .context(InvalidTagElement)?;
                 Ok(TagRange::Group100(Tag(group, elem)))
             }
             (_, b"xx") => {
                 // Element100
                 let group = u16::from_str_radix(group, 16)
-                    .map_err(|_e| TagRangeParseError("Invalid component `group`"))?;
+                    .context(InvalidTagGroup)?;
                 let elem = u16::from_str_radix(&elem[..2], 16)
-                    .map_err(|_e| TagRangeParseError("Invalid component `element`"))?
+                    .context(InvalidTagElement)?
                     << 8;
                 Ok(TagRange::Element100(Tag(group, elem)))
             }
             (_, _) => {
                 // single element
                 let group = u16::from_str_radix(group, 16)
-                    .map_err(|_e| TagRangeParseError("Invalid component `group`"))?;
+                    .context(InvalidTagGroup)?;
                 let elem = u16::from_str_radix(elem, 16)
-                    .map_err(|_e| TagRangeParseError("Invalid component `element`"))?;
+                    .context(InvalidTagElement)?;
                 Ok(TagRange::Single(Tag(group, elem)))
             }
         }

core/src/header.rs

@@ -2,12 +2,37 @@
 //! It comprises a variety of basic data types, such as the DICOM attribute tag, the
 //! element header, and element composite types.
 
-use crate::error::{Error, Result};
 use crate::value::{PrimitiveValue, Value};
 use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::fmt;
 use std::str::{from_utf8, FromStr};
+use snafu::{Backtrace, Snafu};
+
+/// Error type for issues constructing a sequence item header.
+#[derive(Debug, Snafu)]
+#[non_exhaustive]
+pub enum SequenceItemHeaderError {
+    /// Unexpected header tag.
+    /// Only Item (0xFFFE, 0xE000),
+    /// Item Delimiter (0xFFFE, 0xE00D),
+    /// or Sequence Delimiter (0xFFFE, 0xE0DD)
+    /// are admitted.
+    #[snafu(display("Unexpected tag {}", tag))]
+    UnexpectedTag {
+        tag: Tag,
+        backtrace: Backtrace,
+    },
+    /// Unexpected delimiter value length.
+    /// Must be zero for item delimiters.
+    #[snafu(display("Unexpected delimiter length {}", len))]
+    UnexpectedDelimiterLength {
+        len: Length,
+        backtrace: Backtrace,
+    }
+}
+
+type Result<T, E = SequenceItemHeaderError> = std::result::Result<T, E>;
 
 /// Trait for any DICOM entity (element or item) which may have a length.
 pub trait HasLength {
@@ -225,8 +250,8 @@ where
     }
 
     /// Retrieve the element's value as a single string.
-    pub fn to_str(&self) -> Result<Cow<str>> {
-        self.value.to_str().map_err(From::from)
+    pub fn to_str(&self) -> Result<Cow<str>, crate::value::CastValueError> {
+        self.value.to_str()
     }
 }
 
@@ -346,7 +371,7 @@ impl SequenceItemHeader {
                 // item delimiter
                 // delimiters should not have a positive length
                 if len != Length(0) {
-                    Err(Error::UnexpectedDataValueLength)
+                    UnexpectedDelimiterLength { len }.fail()
                 } else {
                     Ok(SequenceItemHeader::ItemDelimiter)
                 }
@@ -355,7 +380,7 @@ impl SequenceItemHeader {
                 // sequence delimiter
                 Ok(SequenceItemHeader::SequenceDelimiter)
             }
-            tag => Err(Error::UnexpectedTag(tag)),
+            tag => UnexpectedTag { tag }.fail()
         }
     }
 }
@@ -680,6 +705,7 @@ impl Length {
 
     /// Create a new length value from its internal representation.
     /// This is equivalent to `Length(len)`.
+    #[inline]
     pub fn new(len: u32) -> Self {
         Length(len)
     }
@@ -689,13 +715,15 @@ impl Length {
     /// # Panic
     ///
     /// This function will panic if `len` represents an undefined length.
+    #[inline]
     pub fn defined(len: u32) -> Self {
         assert_ne!(len, UNDEFINED_LEN);
         Length(len)
     }
 }
 
 impl From<u32> for Length {
+    #[inline]
     fn from(o: u32) -> Self {
         Length(o)
     }
@@ -832,6 +860,7 @@ impl Length {
     /// Check whether the length is equally specified as another length.
     /// Unlike the implemented `PartialEq`, two undefined lengths are
     /// considered equivalent by this method.
+    #[inline]
     pub fn inner_eq(self, other: Length) -> bool {
         self.0 == other.0
     }