Error Handling fixes and improvements

kitchen/src/web/auth.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 use std::str::FromStr;
+use std::sync::Arc;
 
 use async_session::{Session, SessionStore};
 use axum::{
@@ -22,46 +23,70 @@ use axum::{
 use axum_auth::AuthBasic;
 use cookie::{Cookie, SameSite};
 use secrecy::Secret;
-use tracing::{debug, info, instrument};
+use tracing::{debug, error, info, instrument};
 
 use super::storage::{self, AuthStore};
 
 #[instrument(skip_all, fields(user=%auth.0.0))]
 pub async fn handler(
     auth: AuthBasic,
-    Extension(session_store): Extension<storage::SqliteStore>,
+    Extension(session_store): Extension<Arc<storage::SqliteStore>>,
 ) -> impl IntoResponse {
     // NOTE(jwall): It is very important that you do **not** log the password
     // here. We convert the AuthBasic into UserCreds immediately to help prevent
     // that. Do not circumvent that protection.
     let auth = storage::UserCreds::from(auth);
     info!("Handling authentication request");
+    let mut headers = HeaderMap::new();
     if let Ok(true) = session_store.check_user_creds(&auth).await {
         debug!("successfully authenticated user");
         // 1. Create a session identifier.
         let mut session = Session::new();
-        session
+        if let Err(err) = session.insert("user_id", auth.user_id()) {
-            .insert("user_id", auth.user_id())
+            error!(?err, "Unable to insert user id into session");
-            .expect("Unable to insert user id into session");
+            return (
+                StatusCode::INTERNAL_SERVER_ERROR,
+                headers,
+                "Unable to insert user id into session",
+            );
+        }
         // 2. Store the session in the store.
-        let cookie_value = session_store
+        let cookie_value = match session_store.store_session(session).await {
-            .store_session(session)
+            Err(err) => {
-            .await
+                error!(?err, "Unable to store session in session store");
-            .expect("Unable to store session in session store")
+                return (
-            .expect("No session cookie created");
+                    StatusCode::INTERNAL_SERVER_ERROR,
-        let mut headers = HeaderMap::new();
+                    headers,
+                    "Unable to store session in session store",
+                );
+            }
+            Ok(None) => {
+                error!("Unable to create session cookie");
+                return (
+                    StatusCode::INTERNAL_SERVER_ERROR,
+                    headers,
+                    "Unable to create session cookie",
+                );
+            }
+            Ok(Some(value)) => value,
+        };
         // 3. Construct the Session Cookie.
         let cookie = Cookie::build(storage::AXUM_SESSION_COOKIE_NAME, cookie_value)
             .same_site(SameSite::Strict)
             .secure(true)
             .finish();
-        headers.insert(
+        let parsed_cookie = match cookie.to_string().parse() {
-            header::SET_COOKIE,
+            Err(err) => {
-            cookie
+                error!(?err, "Unable to parse session cookie");
-                .to_string()
+                return (
-                .parse()
+                    StatusCode::INTERNAL_SERVER_ERROR,
-                .expect("Unable to parse session cookie"),
+                    headers,
+                    "Unable to parse session cookie",
                 );
+            }
+            Ok(parsed_cookie) => parsed_cookie,
+        };
+        headers.insert(header::SET_COOKIE, parsed_cookie);
         // Respond with 200 OK
         (StatusCode::OK, headers, "Login Successful")
     } else {
@@ -76,15 +101,24 @@ pub async fn handler(
 }
 
 impl From<AuthBasic> for storage::UserCreds {
+    #[instrument(skip_all)]
     fn from(AuthBasic((id, pass)): AuthBasic) -> Self {
+        debug!(user = id, "Authorizing user");
         Self {
-            id: storage::UserId(id.clone()),
+            id: storage::UserId(id),
-            pass: Secret::from_str(
+            pass: match Secret::from_str(match pass {
-                pass.clone()
+                None => {
-                    .expect("No password provided in BasicAuth")
+                    error!("No password provided in BasicAuth");
-                    .as_str(),
+                    panic!("No password provided in BasicAuth");
-            )
+                }
-            .expect("Unable to store pass in secret"),
+                Some(ref pass) => pass.as_str(),
+            }) {
+                Err(err) => {
+                    error!("Unable to store pass in secret");
+                    panic!("{}", err);
+                }
+                Ok(secret) => secret,
+            },
         }
     }
 }

web/src/components/recipe_selector.rs

@@ -24,7 +24,10 @@ pub fn RecipeSelector<G: Html>(cx: Scope) -> View<G> {
     let app_service = get_appservice_from_context(cx).clone();
     let rows = create_memo(cx, move || {
         let mut rows = Vec::new();
-        if let (_, Some(bt)) = app_service.fetch_recipes_from_storage().unwrap() {
+        if let (_, Some(bt)) = app_service
+            .fetch_recipes_from_storage()
+            .expect("Unable to fetch recipes from storage")
+        {
             for row in bt
                 .iter()
                 .map(|(k, v)| create_signal(cx, (k.clone(), v.clone())))

web/src/service.rs

@@ -59,6 +59,7 @@ impl AppService {
             .get()
             .iter()
             .map(|(k, v)| (k.clone(), *v))
+            .filter(|(_, v)| *v != 0)
             .collect()
     }
 
@@ -66,7 +67,9 @@ impl AppService {
     pub async fn synchronize(&mut self) -> Result<(), String> {
         info!("Synchronizing Recipes");
         // TODO(jwall): Make our caching logic using storage more robust.
-        let storage = self.get_storage()?.unwrap();
+        let storage = self
+            .get_storage()?
+            .expect("Unable to get storage for browser session");
         let recipes = self
             .store
             .get_recipes()
@@ -129,7 +132,12 @@ impl AppService {
         let recipe_counts = self.get_menu_list();
         for (idx, count) in recipe_counts.iter() {
             for _ in 0..*count {
-                acc.accumulate_from(self.recipes.get().get(idx).unwrap());
+                acc.accumulate_from(
+                    self.recipes
+                        .get()
+                        .get(idx)
+                        .expect(&format!("No such recipe id exists: {}", idx)),
+                );
             }
         }
         if show_staples {
@@ -158,7 +166,9 @@ impl AppService {
     }
 
     pub fn get_category_text(&self) -> Result<Option<String>, String> {
-        let storage = self.get_storage()?.unwrap();
+        let storage = self
+            .get_storage()?
+            .expect("Unable to get storage for browser session");
         storage
             .get_item("categories")
             .map_err(|e| format!("{:?}", e))
@@ -199,7 +209,9 @@ impl AppService {
     }
 
     pub fn fetch_recipe_text(&self, id: &str) -> Result<Option<RecipeEntry>, String> {
-        let storage = self.get_storage()?.unwrap();
+        let storage = self
+            .get_storage()?
+            .expect("Unable to get storage for browser session");
         if let Some(s) = storage
             .get_item("recipes")
             .map_err(|e| format!("{:?}", e))?