From 35197960b2c5804caefa60421c1f86d36243d26f Mon Sep 17 00:00:00 2001 From: Steven Fackler Date: Fri, 22 May 2015 21:20:56 -0700 Subject: [PATCH] Restructure errors WasNull and BadResponse are gone, and Conversion was added. IoError covers the BadResponse case and Conversion is a more general version of WasNull. --- src/error.rs | 19 +++++++------------ src/io/mod.rs | 3 ++- src/lib.rs | 25 +++++++++++++++---------- src/macros.rs | 2 +- src/priv_io.rs | 2 +- src/types/mod.rs | 30 ++++++++++++++++++++++++------ src/types/rustc_serialize.rs | 6 ++++-- src/types/serde.rs | 6 ++++-- src/types/uuid.rs | 6 +----- tests/test.rs | 2 +- 10 files changed, 60 insertions(+), 41 deletions(-) diff --git a/src/error.rs b/src/error.rs index 1761c9ee..057fc734 100644 --- a/src/error.rs +++ b/src/error.rs @@ -66,14 +66,14 @@ impl DbErrorNew for DbError { fn new_connect(fields: Vec<(u8, String)>) -> result::Result { match DbError::new_raw(fields) { Ok(err) => Err(ConnectError::DbError(err)), - Err(()) => Err(ConnectError::BadResponse), + Err(()) => Err(ConnectError::IoError(::bad_response())), } } fn new(fields: Vec<(u8, String)>) -> Result { match DbError::new_raw(fields) { Ok(err) => Err(Error::DbError(err)), - Err(()) => Err(Error::BadResponse), + Err(()) => Err(Error::IoError(::bad_response())), } } } @@ -205,11 +205,9 @@ pub enum ConnectError { /// The Postgres server does not support SSL encryption. NoSslSupport, /// There was an error initializing the SSL session - SslError(Box), + SslError(Box), /// There was an error communicating with the server. IoError(io::Error), - /// The server sent an unexpected response. - BadResponse, } impl fmt::Display for ConnectError { @@ -235,7 +233,6 @@ impl error::Error for ConnectError { ConnectError::NoSslSupport => "The server does not support SSL", ConnectError::SslError(_) => "Error initiating SSL session", ConnectError::IoError(_) => "Error communicating with server", - ConnectError::BadResponse => "The server returned an unexpected response", } } @@ -296,10 +293,8 @@ pub enum Error { WrongType(Type), /// An attempt was made to read from a column that does not exist. InvalidColumn, - /// A value was NULL but converted to a non-nullable Rust type. - WasNull, - /// The server returned an unexpected response. - BadResponse, + /// An error converting between Postgres and Rust types. + Conversion(Box), } impl fmt::Display for Error { @@ -322,8 +317,7 @@ impl error::Error for Error { } Error::WrongType(_) => "Unexpected type", Error::InvalidColumn => "Invalid column", - Error::WasNull => "The value was NULL", - Error::BadResponse => "The server returned an unexpected response", + Error::Conversion(_) => "An error converting between Postgres and Rust types", } } @@ -331,6 +325,7 @@ impl error::Error for Error { match *self { Error::DbError(ref err) => Some(err), Error::IoError(ref err) => Some(err), + Error::Conversion(ref err) => Some(&**err), _ => None } } diff --git a/src/io/mod.rs b/src/io/mod.rs index 78dfddc2..f3fad41b 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -26,5 +26,6 @@ pub trait NegotiateSsl { /// /// The host portion of the connection parameters is provided for hostname /// verification. - fn negotiate_ssl(&self, host: &str, stream: Stream) -> Result, Box>; + fn negotiate_ssl(&self, host: &str, stream: Stream) + -> Result, Box>; } diff --git a/src/lib.rs b/src/lib.rs index 3f7302d0..5217e5a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -402,6 +402,11 @@ pub fn cancel_query(params: T, ssl: &SslMode, data: CancelData) Ok(()) } +fn bad_response() -> std_io::Error { + std_io::Error::new(std_io::ErrorKind::InvalidInput, + "the server returned an unexpected response") +} + /// An enumeration of transaction isolation levels. /// /// See the [Postgres documentation](http://www.postgresql.org/docs/9.4/static/transaction-iso.html) @@ -451,7 +456,7 @@ impl IsolationLevel { } else if raw.eq_ignore_ascii_case("SERIALIZABLE") { Ok(IsolationLevel::Serializable) } else { - Err(Error::BadResponse) + Err(Error::IoError(bad_response())) } } } @@ -548,7 +553,7 @@ impl InnerConnection { } ReadyForQuery { .. } => break, ErrorResponse { fields } => return DbError::new_connect(fields), - _ => return Err(ConnectError::BadResponse), + _ => return Err(ConnectError::IoError(bad_response())), } } @@ -659,13 +664,13 @@ impl InnerConnection { | AuthenticationGSS | AuthenticationSSPI => return Err(ConnectError::UnsupportedAuthentication), ErrorResponse { fields } => return DbError::new_connect(fields), - _ => return Err(ConnectError::BadResponse) + _ => return Err(ConnectError::IoError(bad_response())) } match try!(self.read_message()) { AuthenticationOk => Ok(()), ErrorResponse { fields } => return DbError::new_connect(fields), - _ => return Err(ConnectError::BadResponse) + _ => return Err(ConnectError::IoError(bad_response())) } } @@ -1469,7 +1474,7 @@ impl<'conn> Statement<'conn> { } _ => { conn.desynchronized = true; - Err(Error::BadResponse) + Err(Error::IoError(bad_response())) } } } @@ -1545,7 +1550,7 @@ impl<'conn> Statement<'conn> { } _ => { conn.desynchronized = true; - return Err(Error::BadResponse); + return Err(Error::IoError(bad_response())); } } } @@ -1693,7 +1698,7 @@ fn read_rows(conn: &mut InnerConnection, buf: &mut VecDeque>> } _ => { conn.desynchronized = true; - return Err(Error::BadResponse); + return Err(Error::IoError(bad_response())); } } } @@ -2136,7 +2141,7 @@ impl<'a> CopyInStatement<'a> { } _ => { conn.desynchronized = true; - return Err(Error::BadResponse); + return Err(Error::IoError(bad_response())); } } @@ -2144,7 +2149,7 @@ impl<'a> CopyInStatement<'a> { CopyInResponse { .. } => {} _ => { conn.desynchronized = true; - return Err(Error::BadResponse); + return Err(Error::IoError(bad_response())); } } @@ -2213,7 +2218,7 @@ impl<'a> CopyInStatement<'a> { } _ => { conn.desynchronized = true; - return Err(Error::BadResponse); + return Err(Error::IoError(bad_response())); } }; diff --git a/src/macros.rs b/src/macros.rs index 191e89d1..a171347e 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -22,6 +22,6 @@ macro_rules! bad_response { ($s:expr) => ({ debug!("Bad response at {}:{}", file!(), line!()); $s.desynchronized = true; - return Err(::Error::BadResponse); + return Err(::Error::IoError(::bad_response())); }) } diff --git a/src/priv_io.rs b/src/priv_io.rs index 6a531936..4ea9c92b 100644 --- a/src/priv_io.rs +++ b/src/priv_io.rs @@ -143,7 +143,7 @@ pub fn initialize_stream(params: &ConnectParams, ssl: &SslMode) let host = match params.target { ConnectTarget::Tcp(ref host) => host, #[cfg(feature = "unix_socket")] - ConnectTarget::Unix(_) => return Err(ConnectError::BadResponse) + ConnectTarget::Unix(_) => return Err(ConnectError::IoError(::bad_response())) }; match negotiator.negotiate_ssl(host, socket) { diff --git a/src/types/mod.rs b/src/types/mod.rs index 44953d6b..69dd382c 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -2,6 +2,7 @@ pub use self::slice::Slice; use std::collections::HashMap; +use std::error; use std::fmt; use std::io::prelude::*; use byteorder::{ReadBytesExt, WriteBytesExt, BigEndian}; @@ -514,6 +515,23 @@ impl Other { } } +/// An error indicating that a `NULL` Postgres value was passed to a `FromSql` +/// implementation that does not support `NULL` values. +#[derive(Debug, Clone, Copy)] +pub struct WasNull; + +impl fmt::Display for WasNull { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.write_str(error::Error::description(self)) + } +} + +impl error::Error for WasNull { + fn description(&self) -> &str { + "a Postgres value was `NULL`" + } +} + /// A trait for types that can be created from a Postgres value. /// /// # Types @@ -566,13 +584,13 @@ pub trait FromSql: Sized { /// is compatible with the Postgres `Type`. /// /// The default implementation calls `FromSql::from_sql` when `raw` is - /// `Some` and returns `Err(Error::WasNull)` when `raw` is `None`. It does - /// not typically need to be overridden. + /// `Some` and returns `Err(Error::Conversion(Box::new(WasNull))` when + /// `raw` is `None`. It does not typically need to be overridden. fn from_sql_nullable(ty: &Type, raw: Option<&mut R>, ctx: &SessionInfo) -> Result { match raw { Some(raw) => FromSql::from_sql(ty, raw, ctx), - None => Err(Error::WasNull), + None => Err(Error::Conversion(Box::new(WasNull))), } } @@ -628,7 +646,7 @@ impl FromSql for String { fn from_sql(_: &Type, raw: &mut R, _: &SessionInfo) -> Result { let mut buf = vec![]; try!(raw.read_to_end(&mut buf)); - String::from_utf8(buf).map_err(|_| Error::BadResponse) + String::from_utf8(buf).map_err(|err| Error::Conversion(Box::new(err))) } fn accepts(ty: &Type) -> bool { @@ -680,7 +698,7 @@ impl FromSql for HashMap> { try!(util::read_all(raw, &mut key)); let key = match String::from_utf8(key) { Ok(key) => key, - Err(_) => return Err(Error::BadResponse), + Err(err) => return Err(Error::Conversion(Box::new(err))), }; let val_len = try!(raw.read_i32::()); @@ -691,7 +709,7 @@ impl FromSql for HashMap> { try!(util::read_all(raw, &mut val)); match String::from_utf8(val) { Ok(val) => Some(val), - Err(_) => return Err(Error::BadResponse), + Err(err) => return Err(Error::Conversion(Box::new(err))), } }; diff --git a/src/types/rustc_serialize.rs b/src/types/rustc_serialize.rs index 61471ef2..8cb50464 100644 --- a/src/types/rustc_serialize.rs +++ b/src/types/rustc_serialize.rs @@ -1,4 +1,5 @@ use serialize::json; +use std::error; use std::io::prelude::*; use byteorder::{ReadBytesExt, WriteBytesExt}; @@ -10,10 +11,11 @@ impl FromSql for json::Json { if let Type::Jsonb = *ty { // We only support version 1 of the jsonb binary format if try!(raw.read_u8()) != 1 { - return Err(Error::BadResponse); + let err: Box = "unsupported JSONB encoding version".into(); + return Err(Error::Conversion(err)); } } - json::Json::from_reader(raw).map_err(|_| Error::BadResponse) + json::Json::from_reader(raw).map_err(|err| Error::Conversion(Box::new(err))) } accepts!(Type::Json, Type::Jsonb); diff --git a/src/types/serde.rs b/src/types/serde.rs index 3b32878f..2bb68ff5 100644 --- a/src/types/serde.rs +++ b/src/types/serde.rs @@ -1,5 +1,6 @@ extern crate serde; +use std::error; use std::io::prelude::*; use byteorder::{ReadBytesExt, WriteBytesExt}; use self::serde::json::{self, Value}; @@ -12,10 +13,11 @@ impl FromSql for Value { if let Type::Jsonb = *ty { // We only support version 1 of the jsonb binary format if try!(raw.read_u8()) != 1 { - return Err(Error::BadResponse); + let err: Box = "unsupported JSONB encoding version".into(); + return Err(Error::Conversion(err)); } } - json::de::from_reader(raw).map_err(|_| Error::BadResponse) + json::de::from_reader(raw).map_err(|err| Error::Conversion(Box::new(err))) } accepts!(Type::Json, Type::Jsonb); diff --git a/src/types/uuid.rs b/src/types/uuid.rs index 3e42286b..4f1c1a57 100644 --- a/src/types/uuid.rs +++ b/src/types/uuid.rs @@ -4,7 +4,6 @@ use std::io::prelude::*; use self::uuid::Uuid; use types::{FromSql, ToSql, Type, IsNull, SessionInfo}; -use Error; use Result; use util; @@ -12,10 +11,7 @@ impl FromSql for Uuid { fn from_sql(_: &Type, raw: &mut R, _: &SessionInfo) -> Result { let mut bytes = [0; 16]; try!(util::read_all(raw, &mut bytes)); - match Uuid::from_bytes(&bytes) { - Some(u) => Ok(u), - None => Err(Error::BadResponse), - } + Ok(Uuid::from_bytes(&bytes).unwrap()) } accepts!(Type::Uuid); diff --git a/tests/test.rs b/tests/test.rs index 4a910f74..010babb4 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -507,7 +507,7 @@ fn test_get_was_null() { let result = or_panic!(stmt.query(&[])); match result.iter().next().unwrap().get_opt::(0) { - Err(Error::WasNull) => {} + Err(Error::Conversion(..)) => {} res => panic!("unexpected result {:?}", res), }; }