From e5c94a6d32699f0bc2556263fd6a03ca8a2f7435 Mon Sep 17 00:00:00 2001
From: Lukasz Klimek <842586+lklimek@users.noreply.github.com>
Date: Thu, 10 Oct 2024 15:03:05 +0200
Subject: [PATCH] refactor(proto): add error handling to FromMillis and
 ToMillis

---
 proto/src/error.rs |  5 +++++
 proto/src/time.rs  | 47 +++++++++++++++++++++++++---------------------
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/proto/src/error.rs b/proto/src/error.rs
index 4c1d268..ea92b87 100644
--- a/proto/src/error.rs
+++ b/proto/src/error.rs
@@ -12,6 +12,11 @@ use crate::prelude::*;
 
 define_error! {
     Error {
+        TimeConversion
+            { reason: String }
+            | e | {
+            format!("error converting time: {}", e.reason)
+        },
         TryFromProtobuf
             { reason: String }
             | e | {
diff --git a/proto/src/time.rs b/proto/src/time.rs
index d832311..15c5da5 100644
--- a/proto/src/time.rs
+++ b/proto/src/time.rs
@@ -1,6 +1,6 @@
 //! Time conversion traits and functions
 
-use crate::google::protobuf::Timestamp;
+use crate::{google::protobuf::Timestamp, Error};
 pub trait ToMillis {
     /// Convert protobuf timestamp into milliseconds since epoch
 
@@ -13,27 +13,29 @@ pub trait ToMillis {
     /// # Panics
     ///  
     /// Panics when timestamp doesn't fit `u64` type
-    fn to_millis(&self) -> u64;
+    fn to_millis(&self) -> Result<u64, Error>;
 
     #[deprecated = "use `to_millis` instead"]
     fn to_milis(&self) -> u64 {
         self.to_millis()
+            .expect("cannot convert time to milliseconds")
     }
 }
 
 impl ToMillis for Timestamp {
     /// Convert protobuf timestamp into milliseconds since epoch
-    fn to_millis(&self) -> u64 {
+    fn to_millis(&self) -> Result<u64, Error> {
         chrono::DateTime::from_timestamp(self.seconds, self.nanos as u32)
-            .unwrap()
-            .to_utc()
-            .timestamp_millis()
-            .try_into()
-            .expect("timestamp value out of u64 range")
+            .map(|t| t.to_utc().timestamp_millis())
+            .and_then(|t| t.try_into().ok())
+            .ok_or(Error::time_conversion(format!(
+                "time value {:?} out of range",
+                self
+            )))
     }
 }
 
-pub trait FromMillis {
+pub trait FromMillis: Sized {
     /// Create protobuf timestamp from milliseconds since epoch
     ///
     /// Note there is a resolution difference, as timestamp uses nanoseconds
@@ -41,14 +43,14 @@ pub trait FromMillis {
     /// # Arguments
     ///
     /// * millis - time since epoch, in milliseconds; must fit `i64` type
-    fn from_millis(millis: u64) -> Self;
+    fn from_millis(millis: u64) -> Result<Self, Error>;
 
     #[deprecated = "use `from_millis` instead"]
     fn from_milis(millis: u64) -> Self
     where
         Self: Sized,
     {
-        Self::from_millis(millis)
+        Self::from_millis(millis).expect("conversion from milliseconds should not fail")
     }
 }
 
@@ -60,18 +62,21 @@ impl FromMillis for Timestamp {
     /// # Panics
     ///  
     /// Panics when `millis` don't fit `i64` type
-    fn from_millis(millis: u64) -> Self {
-        let dt = chrono::DateTime::from_timestamp_millis(
-            millis
-                .try_into()
-                .expect("milliseconds timestamp out of i64 range"),
-        )
-        .expect("cannot parse timestamp")
-        .to_utc();
+    fn from_millis(millis: u64) -> Result<Self, Error> {
+        let ts_millis = millis
+            .try_into()
+            .map_err(|e| Error::time_conversion(format!("milliseconds out of range: {:?}", e)))?;
+
+        let dt = chrono::DateTime::from_timestamp_millis(ts_millis)
+            .ok_or(Error::time_conversion(format!(
+                "cannot create date/time from milliseconds {}",
+                ts_millis,
+            )))?
+            .to_utc();
 
-        Self {
+        Ok(Self {
             nanos: dt.timestamp_subsec_nanos() as i32,
             seconds: dt.timestamp(),
-        }
+        })
     }
 }