Improve error messages for option getters and add null checks

apache · Apr 29, 2024 · 91b8b76 · 91b8b76
1 parent 6420b4e
commit 91b8b76
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 8 deletions.
diff --git a/rust/core/src/driver_exporter.rs b/rust/core/src/driver_exporter.rs
@@ -297,6 +297,7 @@ unsafe fn copy_bytes(src: &[u8], dst: *mut u8, length: *mut usize) {
     *length = n;
 }
 
+// SAFETY: Will panic if `key` is null.
 unsafe fn get_option_int<'a, OptionType, Object>(
     object: Option<&mut Object>,
     options: Option<&mut HashMap<OptionType, OptionValue>>,
@@ -306,6 +307,7 @@ where
     OptionType: Hash + Eq + From<&'a str>,
     Object: Optionable<Option = OptionType>,
 {
+    assert!(!key.is_null());
     let key = CStr::from_ptr(key).to_str()?;
 
     if let Some(options) = options {
@@ -319,7 +321,10 @@ where
             Ok(*optvalue)
         } else {
             let err = Error::with_message_and_status(
-                format!("Option value for key {key:?} has wrong type"),
+                format!(
+                    "Option value for key {key:?} has wrong type (got={}, expected=Int)",
+                    optvalue.get_type()
+                ),
                 Status::InvalidState,
             );
             Err(err)
@@ -331,6 +336,7 @@ where
     }
 }
 
+// SAFETY: Will panic if `key` is null.
 unsafe fn get_option_double<'a, OptionType, Object>(
     object: Option<&mut Object>,
     options: Option<&mut HashMap<OptionType, OptionValue>>,
@@ -340,6 +346,7 @@ where
     OptionType: Hash + Eq + From<&'a str>,
     Object: Optionable<Option = OptionType>,
 {
+    assert!(!key.is_null());
     let key = CStr::from_ptr(key).to_str()?;
 
     if let Some(options) = options {
@@ -353,7 +360,10 @@ where
             Ok(*optvalue)
         } else {
             let err = Error::with_message_and_status(
-                format!("Option value for key {:?} has wrong type", key),
+                format!(
+                    "Option value for key {key:?} has wrong type (got={}, expected=Double)",
+                    optvalue.get_type()
+                ),
                 Status::InvalidState,
             );
             Err(err)
@@ -365,6 +375,7 @@ where
     }
 }
 
+// SAFETY: Will panic if `key` is null.
 unsafe fn get_option<'a, OptionType, Object>(
     object: Option<&mut Object>,
     options: Option<&mut HashMap<OptionType, OptionValue>>,
@@ -374,6 +385,7 @@ where
     OptionType: Hash + Eq + From<&'a str>,
     Object: Optionable<Option = OptionType>,
 {
+    assert!(!key.is_null());
     let key = CStr::from_ptr(key).to_str()?;
 
     if let Some(options) = options {
@@ -387,18 +399,22 @@ where
             Ok(optvalue.clone())
         } else {
             let err = Error::with_message_and_status(
-                format!("Option value for key {key:?} has wrong type"),
+                format!(
+                    "Option value for key {key:?} has wrong type (got={}, expected=String)",
+                    optvalue.get_type()
+                ),
                 Status::InvalidState,
             );
             Err(err)
         }
     } else {
-        let database = object.expect("Broken invariant");
-        let optvalue = database.get_option_string(key.into())?;
+        let object = object.expect("Broken invariant");
+        let optvalue = object.get_option_string(key.into())?;
         Ok(optvalue)
     }
 }
 
+// SAFETY: Will panic if `key` is null.
 unsafe fn get_option_bytes<'a, OptionType, Object>(
     object: Option<&mut Object>,
     options: Option<&mut HashMap<OptionType, OptionValue>>,
@@ -408,6 +424,7 @@ where
     OptionType: Hash + Eq + From<&'a str>,
     Object: Optionable<Option = OptionType>,
 {
+    assert!(!key.is_null());
     let key = CStr::from_ptr(key).to_str()?;
 
     if let Some(options) = options {
@@ -421,14 +438,17 @@ where
             Ok(optvalue.clone())
         } else {
             let err = Error::with_message_and_status(
-                format!("Option value for key {key:?} has wrong type"),
+                format!(
+                    "Option value for key {key:?} has wrong type (got={}, expected=Bytes)",
+                    optvalue.get_type()
+                ),
                 Status::InvalidState,
             );
             Err(err)
         }
     } else {
-        let connection = object.expect("Broken invariant");
-        let optvalue = connection.get_option_bytes(key.into())?;
+        let object = object.expect("Broken invariant");
+        let optvalue = object.get_option_bytes(key.into())?;
         Ok(optvalue)
     }
 }

diff --git a/rust/core/src/options.rs b/rust/core/src/options.rs
@@ -35,6 +35,18 @@ pub enum OptionValue {
     Double(f64),
 }
 
+impl OptionValue {
+    /// Gets the data type of the option's value.
+    pub fn get_type(&self) -> &str {
+        match self {
+            Self::String(_) => "String",
+            Self::Bytes(_) => "Bytes",
+            Self::Int(_) => "Int",
+            Self::Double(_) => "Double",
+        }
+    }
+}
+
 impl From<String> for OptionValue {
     fn from(value: String) -> Self {
         Self::String(value)