Make Encode return a result (launchbadge#3126)

* Make encode and encode_by_ref fallible

This only changes the trait for now and makes it compile, calling .expect() on all users. Those will be removed in a later commit.

* PgNumeric: Turn TryFrom Decimal to an infallible From

* Turn panics in Encode implementations into errors

* Add Encode error analogous to the Decode error

* Propagate decode errors through Arguments::add

This pushes the panics one level further to mostly bind calls. Those will also be removed later.

* Only check argument encoding at the end

* Use Result in Query internally

* Implement query_with functions in terms of _with_result

* Surface encode errors when executing a query.

* Remove remaining panics in AnyConnectionBackend implementations

* PostgreSQL BigDecimal: Return encode error immediately

* Arguments: Add len method to report how many arguments were added

* Query::bind: Report which argument failed to encode

* IsNull: Add is_null method

* MySqlArguments: Replace manual bitmap code with NullBitMap helper type

* Roll back buffer in MySqlArguments if encoding fails

* Roll back buffer in SqliteArguments if encoding fails

* Roll back PgArgumentBuffer if encoding fails

Author: FSMaxB

sqlx-core/src/any/arguments.rs

@@ -1,7 +1,8 @@
 use crate::any::value::AnyValueKind;
 use crate::any::Any;
 use crate::arguments::Arguments;
-use crate::encode::Encode;
+use crate::encode::{Encode, IsNull};
+use crate::error::BoxDynError;
 use crate::types::Type;
 
 pub struct AnyArguments<'q> {
@@ -16,11 +17,16 @@ impl<'q> Arguments<'q> for AnyArguments<'q> {
         self.values.0.reserve(additional);
     }
 
-    fn add<T>(&mut self, value: T)
+    fn add<T>(&mut self, value: T) -> Result<(), BoxDynError>
     where
         T: 'q + Encode<'q, Self::Database> + Type<Self::Database>,
     {
-        let _ = value.encode(&mut self.values);
+        let _: IsNull = value.encode(&mut self.values)?;
+        Ok(())
+    }
+
+    fn len(&self) -> usize {
+        self.values.0.len()
     }
 }
 
@@ -36,7 +42,7 @@ impl<'q> Default for AnyArguments<'q> {
 
 impl<'q> AnyArguments<'q> {
     #[doc(hidden)]
-    pub fn convert_to<'a, A: Arguments<'a>>(&'a self) -> A
+    pub fn convert_to<'a, A: Arguments<'a>>(&'a self) -> Result<A, BoxDynError>
     where
         'q: 'a,
         Option<i32>: Type<A::Database> + Encode<'a, A::Database>,
@@ -62,9 +68,9 @@ impl<'q> AnyArguments<'q> {
                 AnyValueKind::Double(d) => out.add(d),
                 AnyValueKind::Text(t) => out.add(&**t),
                 AnyValueKind::Blob(b) => out.add(&**b),
-            }
+            }?
         }
 
-        out
+        Ok(out)
     }
 }

sqlx-core/src/any/connection/executor.rs

@@ -5,6 +5,8 @@ use crate::executor::{Execute, Executor};
 use either::Either;
 use futures_core::future::BoxFuture;
 use futures_core::stream::BoxStream;
+use futures_util::{stream, FutureExt, StreamExt};
+use std::future;
 
 impl<'c> Executor<'c> for &'c mut AnyConnection {
     type Database = Any;
@@ -17,7 +19,10 @@ impl<'c> Executor<'c> for &'c mut AnyConnection {
         'c: 'e,
         E: Execute<'q, Any>,
     {
-        let arguments = query.take_arguments();
+        let arguments = match query.take_arguments().map_err(Error::Encode) {
+            Ok(arguments) => arguments,
+            Err(error) => return stream::once(future::ready(Err(error))).boxed(),
+        };
         self.backend.fetch_many(query.sql(), arguments)
     }
 
@@ -29,7 +34,10 @@ impl<'c> Executor<'c> for &'c mut AnyConnection {
         'c: 'e,
         E: Execute<'q, Self::Database>,
     {
-        let arguments = query.take_arguments();
+        let arguments = match query.take_arguments().map_err(Error::Encode) {
+            Ok(arguments) => arguments,
+            Err(error) => return future::ready(Err(error)).boxed(),
+        };
         self.backend.fetch_optional(query.sql(), arguments)
     }
 

sqlx-core/src/any/mod.rs

@@ -67,12 +67,15 @@ impl<'q, T> Encode<'q, Any> for Option<T>
 where
     T: Encode<'q, Any> + 'q,
 {
-    fn encode_by_ref(&self, buf: &mut AnyArgumentBuffer<'q>) -> crate::encode::IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut AnyArgumentBuffer<'q>,
+    ) -> Result<crate::encode::IsNull, crate::error::BoxDynError> {
         if let Some(value) = self {
             value.encode_by_ref(buf)
         } else {
             buf.0.push(AnyValueKind::Null);
-            crate::encode::IsNull::Yes
+            Ok(crate::encode::IsNull::Yes)
         }
     }
 }

sqlx-core/src/any/types/blob.rs

@@ -15,9 +15,12 @@ impl Type<Any> for [u8] {
 }
 
 impl<'q> Encode<'q, Any> for &'q [u8] {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::Blob((*self).into()));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 
@@ -42,9 +45,12 @@ impl Type<Any> for Vec<u8> {
 }
 
 impl<'q> Encode<'q, Any> for Vec<u8> {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::Blob(Cow::Owned(self.clone())));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 

sqlx-core/src/any/types/bool.rs

@@ -14,9 +14,12 @@ impl Type<Any> for bool {
 }
 
 impl<'q> Encode<'q, Any> for bool {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::Bool(*self));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 

sqlx-core/src/any/types/float.rs

@@ -14,9 +14,9 @@ impl Type<Any> for f32 {
 }
 
 impl<'q> Encode<'q, Any> for f32 {
-    fn encode_by_ref(&self, buf: &mut AnyArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(&self, buf: &mut AnyArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::Real(*self));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 
@@ -38,9 +38,12 @@ impl Type<Any> for f64 {
 }
 
 impl<'q> Encode<'q, Any> for f64 {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::Double(*self));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 

sqlx-core/src/any/types/int.rs

@@ -18,9 +18,12 @@ impl Type<Any> for i16 {
 }
 
 impl<'q> Encode<'q, Any> for i16 {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::SmallInt(*self));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 
@@ -43,9 +46,12 @@ impl Type<Any> for i32 {
 }
 
 impl<'q> Encode<'q, Any> for i32 {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::Integer(*self));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 
@@ -68,9 +74,12 @@ impl Type<Any> for i64 {
 }
 
 impl<'q> Encode<'q, Any> for i64 {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::BigInt(*self));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 

sqlx-core/src/any/types/str.rs

@@ -16,15 +16,18 @@ impl Type<Any> for str {
 }
 
 impl<'a> Encode<'a, Any> for &'a str {
-    fn encode(self, buf: &mut <Any as Database>::ArgumentBuffer<'a>) -> IsNull
+    fn encode(self, buf: &mut <Any as Database>::ArgumentBuffer<'a>) -> Result<IsNull, BoxDynError>
     where
         Self: Sized,
     {
         buf.0.push(AnyValueKind::Text(self.into()));
-        IsNull::No
+        Ok(IsNull::No)
     }
 
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'a>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'a>,
+    ) -> Result<IsNull, BoxDynError> {
         (*self).encode(buf)
     }
 }
@@ -50,9 +53,12 @@ impl Type<Any> for String {
 }
 
 impl<'q> Encode<'q, Any> for String {
-    fn encode_by_ref(&self, buf: &mut <Any as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <Any as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         buf.0.push(AnyValueKind::Text(Cow::Owned(self.clone())));
-        IsNull::No
+        Ok(IsNull::No)
     }
 }
 

sqlx-core/src/arguments.rs

@@ -2,6 +2,7 @@
 
 use crate::database::Database;
 use crate::encode::Encode;
+use crate::error::BoxDynError;
 use crate::types::Type;
 use std::fmt::{self, Write};
 
@@ -14,10 +15,13 @@ pub trait Arguments<'q>: Send + Sized + Default {
     fn reserve(&mut self, additional: usize, size: usize);
 
     /// Add the value to the end of the arguments.
-    fn add<T>(&mut self, value: T)
+    fn add<T>(&mut self, value: T) -> Result<(), BoxDynError>
     where
         T: 'q + Encode<'q, Self::Database> + Type<Self::Database>;
 
+    /// The number of arguments that were already added.
+    fn len(&self) -> usize;
+
     fn format_placeholder<W: Write>(&self, writer: &mut W) -> fmt::Result {
         writer.write_str("?")
     }

sqlx-core/src/encode.rs

@@ -3,8 +3,10 @@
 use std::mem;
 
 use crate::database::Database;
+use crate::error::BoxDynError;
 
 /// The return type of [Encode::encode].
+#[must_use]
 pub enum IsNull {
     /// The value is null; no data was written.
     Yes,
@@ -15,11 +17,16 @@ pub enum IsNull {
     No,
 }
 
+impl IsNull {
+    pub fn is_null(&self) -> bool {
+        matches!(self, IsNull::Yes)
+    }
+}
+
 /// Encode a single value to be sent to the database.
 pub trait Encode<'q, DB: Database> {
     /// Writes the value of `self` into `buf` in the expected format for the database.
-    #[must_use]
-    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull
+    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError>
     where
         Self: Sized,
     {
@@ -30,8 +37,10 @@ pub trait Encode<'q, DB: Database> {
     ///
     /// Where possible, make use of `encode` instead as it can take advantage of re-using
     /// memory.
-    #[must_use]
-    fn encode_by_ref(&self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull;
+    fn encode_by_ref(
+        &self,
+        buf: &mut <DB as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError>;
 
     fn produces(&self) -> Option<DB::TypeInfo> {
         // `produces` is inherently a hook to allow database drivers to produce value-dependent
@@ -50,12 +59,15 @@ where
     T: Encode<'q, DB>,
 {
     #[inline]
-    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
         <T as Encode<DB>>::encode_by_ref(self, buf)
     }
 
     #[inline]
-    fn encode_by_ref(&self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> IsNull {
+    fn encode_by_ref(
+        &self,
+        buf: &mut <DB as Database>::ArgumentBuffer<'q>,
+    ) -> Result<IsNull, BoxDynError> {
         <&T as Encode<DB>>::encode(self, buf)
     }
 
@@ -90,23 +102,23 @@ macro_rules! impl_encode_for_option {
             fn encode(
                 self,
                 buf: &mut <$DB as $crate::database::Database>::ArgumentBuffer<'q>,
-            ) -> $crate::encode::IsNull {
+            ) -> Result<$crate::encode::IsNull, $crate::error::BoxDynError> {
                 if let Some(v) = self {
                     v.encode(buf)
                 } else {
-                    $crate::encode::IsNull::Yes
+                    Ok($crate::encode::IsNull::Yes)
                 }
             }
 
             #[inline]
             fn encode_by_ref(
                 &self,
                 buf: &mut <$DB as $crate::database::Database>::ArgumentBuffer<'q>,
-            ) -> $crate::encode::IsNull {
+            ) -> Result<$crate::encode::IsNull, $crate::error::BoxDynError> {
                 if let Some(v) = self {
                     v.encode_by_ref(buf)
                 } else {
-                    $crate::encode::IsNull::Yes
+                    Ok($crate::encode::IsNull::Yes)
                 }
             }
 

sqlx-core/src/error.rs

@@ -78,6 +78,10 @@ pub enum Error {
         source: BoxDynError,
     },
 
+    /// Error occured while encoding a value.
+    #[error("error occured while encoding a value: {0}")]
+    Encode(#[source] BoxDynError),
+
     /// Error occurred while decoding a value.
     #[error("error occurred while decoding: {0}")]
     Decode(#[source] BoxDynError),