Simplify error handling

ohsayan · ohsayan · commit f7623e98c9f8 · 2021-08-17T23:54:48.000-07:00
diff --git a/README.md b/README.md
@@ -43,12 +43,12 @@ fn main() -> std::io::Result<()> {
 Now let's run a `Query`! Change the previous code block to:
 
 ```rust
-use skytable::{Connection, Query, Response, Element};
-fn main() -> std::io::Result<()> {
+use skytable::{error, Connection, Query, Element};
+fn main() -> Result<(), error::Error> {
     let mut con = Connection::new("127.0.0.1", 2003)?;
     let query = Query::from("heya");
     let res = con.run_simple_query(&query)?;
-    assert_eq!(res, Response::Item(Element::String("HEY!".to_owned())));
+    assert_eq!(res, Element::String("HEY!".to_owned()));
     Ok(())
 }
 ```
diff --git a/src/actions.rs b/src/actions.rs
@@ -37,6 +37,7 @@
 //!
 //! ```
 
+use crate::error::{Error, SkyhashError};
 use crate::types::Array;
 use crate::types::FlatElement;
 use crate::types::SnapshotResult;
@@ -47,54 +48,34 @@ use crate::IntoSkyhashAction;
 use crate::IntoSkyhashBytes;
 use crate::Query;
 use crate::RespCode;
-use crate::Response;
+use crate::SkyResult;
 
 cfg_async!(
     use core::{future::Future, pin::Pin};
 );
-use std::io::ErrorKind;
 
 /// The error string returned when the snapshot engine is busy
 pub const ERR_SNAPSHOT_BUSY: &str = "err-snapshot-busy";
 /// The error string returned when periodic snapshots are busy
 pub const ERR_SNAPSHOT_DISABLED: &str = "err-snapshot-disabled";
 
-/// Errors while running actions
-#[derive(Debug)]
-#[non_exhaustive]
-pub enum ActionError {
-    /// The server sent data but we failed to parse it
-    ParseError,
-    /// The server sent an unexpected data type for this action
-    UnexpectedDataType,
-    /// The server sent an unknown data type that we cannot parse
-    UnknownDataType,
-    /// The server sent an invalid response
-    InvalidResponse,
-    /// An I/O error occurred while running this action
-    IoError(ErrorKind),
-    /// The server returned a response code **other than the one that should have been returned
-    /// for this action** (if any)
-    Code(RespCode),
-}
-
 cfg_async!(
     /// A special result that is returned when running actions (async)
     pub type AsyncResult<'s, T> = Pin<Box<dyn Future<Output = T> + Send + Sync + 's>>;
     #[doc(hidden)]
     pub trait AsyncSocket: Send + Sync {
-        fn run(&mut self, q: Query) -> AsyncResult<std::io::Result<Response>>;
+        fn run(&mut self, q: Query) -> AsyncResult<SkyResult>;
     }
     impl<T> AsyncActions for T where T: AsyncSocket {}
 );
 
 /// A special result that is returned when running actions
-pub type ActionResult<T> = Result<T, ActionError>;
+pub type ActionResult<T> = Result<T, Error>;
 
 cfg_sync!(
     #[doc(hidden)]
     pub trait SyncSocket {
-        fn run(&mut self, q: Query) -> std::io::Result<Response>;
+        fn run(&mut self, q: Query) -> SkyResult;
     }
     impl<T> Actions for T where T: SyncSocket {}
 );
@@ -103,12 +84,8 @@ macro_rules! gen_match {
     ($ret:expr, $($($mtch:pat)+ $(if $exp:expr)*, $expect:expr),*) => {
         match $ret {
             $($(Ok($mtch))|* $(if $exp:expr)* => Ok($expect),)*
-            Ok(Response::InvalidResponse) => Err(ActionError::InvalidResponse),
-            Ok(Response::ParseError) => Err(ActionError::ParseError),
-            Ok(Response::UnsupportedDataType) => Err(ActionError::UnknownDataType),
-            Ok(Response::Item(Element::RespCode(code))) => Err(ActionError::Code(code)),
-            Ok(Response::Item(_)) => Err(ActionError::UnexpectedDataType),
-            Err(e) => Err(ActionError::IoError(e.kind())),
+            Ok(_) => Err(SkyhashError::UnexpectedDataType.into()),
+            Err(e) => Err(e),
         }
     };
 }
@@ -155,52 +132,52 @@ implement_actions!(
     /// Get the number of keys present in the database
     fn dbsize() -> usize {
         { Query::from("dbsize") }
-        Response::Item(Element::UnsignedInt(int)) => int as usize
+        Element::UnsignedInt(int) => int as usize
     }
     /// Deletes a single or a number of keys
     ///
     /// This will return the number of keys that were deleted
     fn del(key: impl IntoSkyhashAction + 's) -> usize {
         { Query::from("del").arg(key) }
-        Response::Item(Element::UnsignedInt(int)) => int as usize
+        Element::UnsignedInt(int) => int as usize
     }
     /// Checks if a key (or keys) exist(s)
     ///
     /// This will return the number of keys that do exist
     fn exists(key: impl IntoSkyhashAction + 's) -> usize {
         { Query::from("exists").arg(key) }
-        Response::Item(Element::UnsignedInt(int)) => int as usize
+        Element::UnsignedInt(int) => int as usize
     }
     /// Removes all the keys present in the database
     fn flushdb() -> () {
         { Query::from("flushdb") }
-        Response::Item(Element::RespCode(RespCode::Okay)) => {}
+        Element::RespCode(RespCode::Okay) => {}
     }
     /// Get the value of a key
     fn get(key: impl IntoSkyhashBytes + 's) -> String {
         { Query::from("get").arg(key)}
-        Response::Item(Element::Str(st)) => st
+        Element::String(st) => st
     }
     /// Get the length of a key
     fn keylen(key: impl IntoSkyhashBytes + 's) -> usize {
         { Query::from("keylen").arg(key)}
-        Response::Item(Element::UnsignedInt(int)) => int as usize
+        Element::UnsignedInt(int) => int as usize
     }
     /// Returns a vector of keys
     ///
     /// Do note that the order might be completely meaningless
     fn lskeys(count: usize) -> Vec<FlatElement> {
         { Query::from("lskeys").arg(count)}
-        Response::Item(Element::Array(Array::Flat(arr))) => arr
+        Element::Array(Array::Flat(arr)) => arr
     }
     /// Get multiple keys
     ///
     /// This returns a vector of [`Element`]s which either contain the values
     /// as strings or contains `Not Found (Code: 1)` response codes
     fn mget(keys: impl IntoSkyhashAction+ 's) -> Array {
         { Query::from("mget").arg(keys)}
-        Response::Item(Element::Array(Array::Bin(brr))) => Array::Bin(brr),
-        Response::Item(Element::Array(Array::Str(srr))) => Array::Str(srr)
+        Element::Array(Array::Bin(brr)) => Array::Bin(brr),
+        Element::Array(Array::Str(srr)) => Array::Str(srr)
     }
     /// Creates a snapshot
     ///
@@ -209,15 +186,16 @@ implement_actions!(
     /// which is normal behavior and _not an inherent error_
     fn mksnap() -> SnapshotResult {
        { Query::from("mksnap")}
-       Response::Item(Element::RespCode(RespCode::Okay)) => SnapshotResult::Okay,
-       Response::Item(Element::RespCode(RespCode::ErrorString(er))) => {
+       Element::RespCode(RespCode::Okay) => SnapshotResult::Okay,
+       Element::RespCode(RespCode::ErrorString(er)) => {
            match er.as_str() {
                ERR_SNAPSHOT_BUSY => SnapshotResult::Busy,
                ERR_SNAPSHOT_DISABLED => SnapshotResult::Disabled,
-               _ => return Err(ActionError::InvalidResponse)
+               _ => return Err(SkyhashError::InvalidResponse.into())
            }
        }
     }
+
     /// Sets the value of multiple keys and values and returns the number of keys that were set
     ///
     /// ## Panics
@@ -231,7 +209,7 @@ implement_actions!(
             assert!(keys.incr_len_by() == values.incr_len_by(), "The number of keys and values for mset must be equal");
             Query::from("mset")._push_alt_iter(keys, values)
         }
-        Response::Item(Element::UnsignedInt(int)) => int as usize
+        Element::UnsignedInt(int) => int as usize
     }
     /// Updates the value of multiple keys and values and returns the number of keys that were updated
     ///
@@ -246,34 +224,34 @@ implement_actions!(
             assert!(keys.incr_len_by() == values.incr_len_by(), "The number of keys and values for mupdate must be equal");
             Query::from("mset")._push_alt_iter(keys, values)
         }
-        Response::Item(Element::UnsignedInt(int)) => int as usize
+        Element::UnsignedInt(int) => int as usize
     }
     /// Consumes a key if it exists
     ///
     /// This should return either the corresponding values of the provided keys or `Not Found`
     /// error codes
     fn pop(keys: impl IntoSkyhashBytes + 's) -> Str {
         { Query::from("POP").arg(keys) }
-        Response::Item(Element::Str(st)) => Str::Unicode(st),
-        Response::Item(Element::Binstr(bstr)) => Str::Binary(bstr)
+        Element::String(st) => Str::Unicode(st),
+        Element::Binstr(bstr) => Str::Binary(bstr)
     }
     /// Consumes the provided keys if they exist
     fn mpop(keys: impl IntoSkyhashAction + 's) -> Array {
         { Query::from("mpop").arg(keys)}
-        Response::Item(Element::Array(Array::Bin(brr))) => Array::Bin(brr),
-        Response::Item(Element::Array(Array::Str(srr))) => Array::Str(srr)
+        Element::Array(Array::Bin(brr)) => Array::Bin(brr),
+        Element::Array(Array::Str(srr)) => Array::Str(srr)
     }
     /// Deletes all the provided keys if they exist or doesn't do anything at all. This method
     /// will return true if all the provided keys were deleted, else it will return false
     fn sdel(keys: impl IntoSkyhashAction + 's) -> bool {
         { Query::from("sdel").arg(keys) }
-        Response::Item(Element::RespCode(RespCode::Okay)) => true,
-        Response::Item(Element::RespCode(RespCode::NotFound)) => false
+        Element::RespCode(RespCode::Okay) => true,
+        Element::RespCode(RespCode::NotFound) => false
     }
     /// Set the value of a key
     fn set(key: impl IntoSkyhashBytes + 's, value: impl IntoSkyhashBytes + 's) -> () {
         { Query::from("set").arg(key).arg(value) }
-        Response::Item(Element::RespCode(RespCode::Okay)) => {}
+        Element::RespCode(RespCode::Okay) => {}
     }
     /// Sets the value of all the provided keys or does nothing. This method will return true if all the keys
     /// were set or will return false if none were set
@@ -292,8 +270,8 @@ implement_actions!(
             );
             Query::from("sset")._push_alt_iter(keys, values)
         }
-        Response::Item(Element::RespCode(RespCode::Okay)) => true,
-        Response::Item(Element::RespCode(RespCode::OverwriteError)) => false
+        Element::RespCode(RespCode::Okay) => true,
+        Element::RespCode(RespCode::OverwriteError) => false
     }
     /// Updates the value of all the provided keys or does nothing. This method will return true if all the keys
     /// were updated or will return false if none were updated
@@ -312,13 +290,13 @@ implement_actions!(
             );
             Query::from("supdate")._push_alt_iter(keys, values)
         }
-        Response::Item(Element::RespCode(RespCode::Okay)) => true,
-        Response::Item(Element::RespCode(RespCode::NotFound)) => false
+        Element::RespCode(RespCode::Okay) => true,
+        Element::RespCode(RespCode::NotFound) => false
     }
     /// Update the value of a key
     fn update(key: impl IntoSkyhashBytes + 's, value: impl IntoSkyhashBytes + 's) -> () {
         { Query::from("update").arg(key).arg(value) }
-        Response::Item(Element::RespCode(RespCode::Okay)) => {}
+        Element::RespCode(RespCode::Okay) => {}
     }
     /// Updates or sets all the provided keys and returns the number of keys that were set
     ///
@@ -336,6 +314,6 @@ implement_actions!(
             );
             Query::from("uset")._push_alt_iter(keys, values)
         }
-        Response::Item(Element::UnsignedInt(int)) => int as usize
+        Element::UnsignedInt(int) => int as usize
     }
 );
diff --git a/src/aio.rs b/src/aio.rs
@@ -25,10 +25,12 @@
 //!
 
 use crate::deserializer::{ParseError, Parser, RawResponse};
+use crate::error::SkyhashError;
 use crate::IoResult;
-use crate::{Query, Response};
+use crate::Query;
+use crate::SkyResult;
 use bytes::{Buf, BytesMut};
-use std::io::{Error, ErrorKind};
+use std::io::{Error as IoError, ErrorKind};
 use tokio::io::{AsyncReadExt, AsyncWriteExt, BufWriter};
 use tokio::net::TcpStream;
 
@@ -40,24 +42,24 @@ macro_rules! impl_async_methods {
         impl $ty {
             /// This function will write a [`Query`] to the stream and read the response from the
             /// server. It will then determine if the returned response is complete or incomplete
-            /// or invalid and return an appropriate variant of [`Response`] wrapped in [`IoResult`]
+            /// or invalid and return an appropriate variant of [`Error`](crate::error::Error) wrapped in [`IoResult`]
             /// for any I/O errors that may occur
             ///
             /// ## Panics
             /// This method will panic if the [`Query`] supplied is empty (i.e has no arguments)
-            pub async fn run_simple_query(&mut self, query: &Query) -> IoResult<Response> {
+            pub async fn run_simple_query(&mut self, query: &Query) -> SkyResult {
                 assert!(query.len() != 0, "A `Query` cannot be of zero length!");
                 query.write_query_to(&mut self.stream).await?;
                 self.stream.flush().await?;
                 loop {
                     if 0usize == self.stream.read_buf(&mut self.buffer).await? {
-                        return Err(Error::from(ErrorKind::ConnectionReset));
+                        return Err(IoError::from(ErrorKind::ConnectionReset).into());
                     }
                     match self.try_response() {
                         Ok((query, forward_by)) => {
                             self.buffer.advance(forward_by);
                             match query {
-                                RawResponse::SimpleQuery(s) => return Ok(Response::Item(s)),
+                                RawResponse::SimpleQuery(s) => return Ok(s),
                                 RawResponse::PipelinedQuery(_) => {
                                     unimplemented!("Pipelined queries aren't implemented yet")
                                 }
@@ -67,14 +69,16 @@ macro_rules! impl_async_methods {
                             ParseError::NotEnough => (),
                             ParseError::BadPacket | ParseError::UnexpectedByte => {
                                 self.buffer.clear();
-                                return Ok(Response::InvalidResponse);
+                                return Err(SkyhashError::InvalidResponse.into());
+                            }
+                            ParseError::DataTypeError => {
+                                return Err(SkyhashError::ParseError.into())
                             }
-                            ParseError::DataTypeError => return Ok(Response::ParseError),
                             ParseError::Empty => {
-                                return Err(Error::from(ErrorKind::ConnectionReset))
+                                return Err(IoError::from(ErrorKind::ConnectionReset).into())
                             }
                             ParseError::UnknownDatatype => {
-                                return Ok(Response::UnsupportedDataType)
+                                return Err(SkyhashError::UnknownDataType.into())
                             }
                         },
                     }
@@ -90,7 +94,7 @@ macro_rules! impl_async_methods {
             }
         }
         impl crate::actions::AsyncSocket for $ty {
-            fn run(&mut self, q: Query) -> crate::actions::AsyncResult<std::io::Result<Response>> {
+            fn run(&mut self, q: Query) -> crate::actions::AsyncResult<SkyResult> {
                 Box::pin(async move { self.run_simple_query(&q).await })
             }
         }
diff --git a/src/deserializer.rs b/src/deserializer.rs
@@ -65,10 +65,10 @@ pub(super) struct Parser<'a> {
 ///
 /// This enum represents the data types supported by the Skyhash Protocol
 pub enum Element {
-    /// Arrays can be nested! Their `<tsymbol>` is `&`
+    /// Array types
     Array(Array),
     /// An unicode string value; `<tsymbol>` is `+`
-    Str(String),
+    String(String),
     /// A binary string (`?`)
     Binstr(Vec<u8>),
     /// An unsigned integer value; `<tsymbol>` is `:`
@@ -369,7 +369,7 @@ impl<'a> Parser<'a> {
             self.incr_cursor();
             let ret = match *tsymbol {
                 b'?' => Element::Binstr(self.parse_next_binstr()?),
-                b'+' => Element::Str(self.parse_next_string()?),
+                b'+' => Element::String(self.parse_next_string()?),
                 b':' => Element::UnsignedInt(self.parse_next_u64()?),
                 b'&' => Element::Array(Array::Recursive(self.parse_next_array()?)),
                 b'!' => Element::RespCode(self.parse_next_respcode()?),
diff --git a/src/lib.rs b/src/lib.rs
diff --git a/src/sync.rs b/src/sync.rs
