From a36fd740af0c5d290d1d5c1dd97aedfb5a5f233a Mon Sep 17 00:00:00 2001
From: Lucien Cartier-Tilet <lucien@phundrak.com>
Date: Tue, 17 Jan 2023 00:58:01 +0100
Subject: [PATCH] Even more error handling

---
 src/db/mod.rs        | 100 +++++++++++++++++++------------------------
 src/graphql/query.rs |  22 +++++-----
 2 files changed, 56 insertions(+), 66 deletions(-)

diff --git a/src/db/mod.rs b/src/db/mod.rs
index 0448a16..20b60b4 100644
--- a/src/db/mod.rs
+++ b/src/db/mod.rs
@@ -7,12 +7,12 @@ use self::models::words::Word;
 
 use diesel::pg::PgConnection;
 use diesel::r2d2::{ConnectionManager, Pool, PooledConnection};
+use diesel::result::Error;
 use diesel::{insert_into, prelude::*};
 
 use dotenvy::dotenv;
 use juniper::{graphql_value, DefaultScalarValue, FieldError, IntoFieldError};
 use std::env;
-use std::error::Error;
 use tracing::info;
 
 #[derive(Debug)]
@@ -36,7 +36,7 @@ impl DatabaseError {
     }
 }
 
-impl Error for DatabaseError {}
+impl std::error::Error for DatabaseError {}
 
 impl std::fmt::Display for DatabaseError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -53,23 +53,6 @@ impl IntoFieldError for DatabaseError {
     }
 }
 
-macro_rules! find_element {
-    ($conn:expr,$dsl:ident,$type:ty,$value:expr,$errmsg:expr) => {
-        if let Ok(val) = $conn {
-            $dsl.find($value).first::<$type>(val).map_or_else(
-                |e| {
-                    info!("{}: {:?}", $errmsg, e);
-                    None
-                },
-                Some,
-            )
-        } else {
-            info!("Failed to obtain connection for the database");
-            None
-        }
-    };
-}
-
 #[derive(Debug, Clone)]
 pub struct Database {
     conn: Pool<ConnectionManager<PgConnection>>,
@@ -170,37 +153,42 @@ impl Database {
             })
     }
 
-    pub fn language(&self, name: &str, owner: &str) -> Option<Language> {
+    pub fn language(
+        &self,
+        name: &str,
+        owner: &str,
+    ) -> Result<Option<Language>, DatabaseError> {
         use self::schema::languages::dsl;
-        match &mut self.conn() {
-            Ok(conn) => match dsl::languages
-                .filter(dsl::name.eq(name))
-                .filter(dsl::owner.eq(owner))
-                .first::<Language>(conn)
-            {
-                Ok(val) => Some(val),
-                Err(e) => {
-                    info!("Could not retrieve language {} of user {} from database: {:?}",
-                          name, owner, e);
-                    None
-                }
-            },
-            Err(e) => {
-                info!("Could not connect to the database: {:?}", e);
-                None
-            }
+        match dsl::languages
+            .filter(dsl::name.eq(name))
+            .filter(dsl::owner.eq(owner))
+            .first(&mut self.conn()?)
+        {
+            Ok(val) => Ok(Some(val)),
+            Err(Error::NotFound) => Ok(None),
+            Err(e) => Err(DatabaseError::new(
+                format!(
+                    "Failed to find language {} belonging to {}: {:?}",
+                    name, owner, e
+                ),
+                "Database error",
+            )),
         }
     }
 
-    pub fn user(&self, id: &str) -> Option<User> {
+    pub fn user(&self, id: &str) -> Result<Option<User>, DatabaseError> {
         use self::schema::users::dsl::users;
-        find_element!(
-            &mut self.conn(),
-            users,
-            User,
-            id.to_string(),
-            format!("Failed to retrieve user {} from database", id)
-        )
+        match users.find(id).first::<User>(&mut self.conn()?) {
+            Ok(val) => Ok(Some(val)),
+            Err(Error::NotFound) => Ok(None),
+            Err(e) => Err(DatabaseError::new(
+                format!(
+                    "Failed to retrieve user {} from database: {:?}",
+                    id, e
+                ),
+                "Database Error",
+            )),
+        }
     }
 
     pub fn insert_user(
@@ -239,18 +227,18 @@ impl Database {
         }
     }
 
-    pub fn word_id(&self, id: &str) -> Option<Word> {
+    pub fn word_id(&self, id: &str) -> Result<Option<Word>, DatabaseError> {
         use self::schema::words::dsl;
-        if let Ok(conn) = &mut self.conn() {
-            match dsl::words.find(id).first::<Word>(conn) {
-                Ok(val) => Some(val),
-                Err(e) => {
-                    info!("Error retrieving {}: {:?}", id, e);
-                    None
-                }
-            }
-        } else {
-            None
+        match dsl::words.find(id).first::<Word>(&mut self.conn()?) {
+            Ok(val) => Ok(Some(val)),
+            Err(Error::NotFound) => Ok(None),
+            Err(e) => Err(DatabaseError::new(
+                format!(
+                    "Failed to retrieve word {} from database: {:?}",
+                    id, e
+                ),
+                "Database Error",
+            )),
         }
     }
 
diff --git a/src/graphql/query.rs b/src/graphql/query.rs
index 1b69a54..e96d34a 100644
--- a/src/graphql/query.rs
+++ b/src/graphql/query.rs
@@ -76,24 +76,27 @@ impl Query {
         context: &Context,
         name: String,
         owner: String,
-    ) -> Option<Language> {
-        context.db.language(name.as_str(), owner.as_str())
+    ) -> FieldResult<Option<Language>> {
+        context
+            .db
+            .language(name.as_str(), owner.as_str())
+            .map_err(Into::into)
     }
 
     #[graphql(
         description = "Retrieve a specific user from its id",
         arguments(id(description = "Appwrite ID of a user"))
     )]
-    fn user(context: &Context, id: String) -> Option<User> {
-        context.db.user(id.as_str())
+    fn user(context: &Context, id: String) -> FieldResult<Option<User>> {
+        context.db.user(id.as_str()).map_err(Into::into)
     }
 
     #[graphql(
         description = "Retrieve a specific word from its id",
         arguments(id(description = "Unique identifier of a word"))
     )]
-    fn word(context: &Context, id: String) -> Option<Word> {
-        context.db.word_id(id.as_str())
+    fn word(context: &Context, id: String) -> FieldResult<Option<Word>> {
+        context.db.word_id(id.as_str()).map_err(Into::into)
     }
 
     #[graphql(
@@ -140,10 +143,9 @@ impl Query {
         word: String,
     ) -> FieldResult<Vec<Word>> {
         match Uuid::from_str(&language) {
-            Ok(uuid) => context
-                .db
-                .words(uuid, word.as_str())
-                .map_err(Into::into),
+            Ok(uuid) => {
+                context.db.words(uuid, word.as_str()).map_err(Into::into)
+            }
             Err(e) => Err(DatabaseError::new(
                 format!("Failed to convert {} to a UUID: {:?}", language, e),
                 "Conversion Error",