From 2b0fe0e868a47f231029f39bab5890d4830a9c79 Mon Sep 17 00:00:00 2001 From: Jonathan Krebs Date: Fri, 13 Dec 2024 14:55:57 +0100 Subject: [PATCH] add some error handling, mostly to quiet warnings --- bffhd/capnp/user.rs | 17 +++++++++-------- bffhd/capnp/user_system.rs | 4 ++-- bffhd/db/mod.rs | 9 +++++++++ bffhd/resources/mod.rs | 7 +++++-- bffhd/resources/state/db.rs | 8 ++++---- bffhd/users/db.rs | 5 ++--- build.rs | 2 +- 7 files changed, 32 insertions(+), 20 deletions(-) diff --git a/bffhd/capnp/user.rs b/bffhd/capnp/user.rs index 484125f..3ef26d9 100644 --- a/bffhd/capnp/user.rs +++ b/bffhd/capnp/user.rs @@ -109,7 +109,7 @@ impl manage::Server for User { if let Some(mut user) = self.session.users.get_user(uid) { if let Ok(true) = user.check_password(old_pw.as_bytes()) { user.set_pw(new_pw.as_bytes()); - self.session.users.put_user(uid, &user); + pry!(self.session.users.put_user(uid, &user)); } } Promise::ok(()) @@ -143,9 +143,9 @@ impl admin::Server for User { // Only update if needed if !target.userdata.roles.iter().any(|r| r.as_str() == rolename) { target.userdata.roles.push(rolename.to_string()); - self.session + pry!(self.session .users - .put_user(self.user.get_username(), &target); + .put_user(self.user.get_username(), &target)); } } @@ -168,9 +168,9 @@ impl admin::Server for User { // Only update if needed if target.userdata.roles.iter().any(|r| r.as_str() == rolename) { target.userdata.roles.retain(|r| r.as_str() != rolename); - self.session + pry!(self.session .users - .put_user(self.user.get_username(), &target); + .put_user(self.user.get_username(), &target)); } } @@ -185,7 +185,7 @@ impl admin::Server for User { let uid = self.user.get_username(); if let Some(mut user) = self.session.users.get_user(uid) { user.set_pw(new_pw.as_bytes()); - self.session.users.put_user(uid, &user); + pry!(self.session.users.put_user(uid, &user)); } Promise::ok(()) } @@ -299,7 +299,8 @@ impl card_d_e_s_fire_e_v2::Server for User { .insert("cardtoken".to_string(), token.to_string()); user.userdata.kv.insert("cardkey".to_string(), card_key); - self.session.users.put_user(self.user.get_username(), &user); + pry!(self.session.users.put_user(self.user.get_username(), &user)); + Promise::ok(()) } @@ -338,7 +339,7 @@ impl card_d_e_s_fire_e_v2::Server for User { } } - self.session.users.put_user(self.user.get_username(), &user); + pry!(self.session.users.put_user(self.user.get_username(), &user)); Promise::ok(()) } diff --git a/bffhd/capnp/user_system.rs b/bffhd/capnp/user_system.rs index 2d40a44..b52366f 100644 --- a/bffhd/capnp/user_system.rs +++ b/bffhd/capnp/user_system.rs @@ -89,8 +89,8 @@ impl manage::Server for Users { if !username.is_empty() && !password.is_empty() { if self.session.users.get_user(username).is_none() { let user = db::User::new_with_plain_pw(username, password); - self.session.users.put_user(username, &user); - let mut builder = builder.init_successful(); + pry!(self.session.users.put_user(username, &user)); + let builder = builder.init_successful(); User::fill(&self.session, user, builder); } else { let mut builder = builder.init_failed(); diff --git a/bffhd/db/mod.rs b/bffhd/db/mod.rs index 23701aa..a01cff6 100644 --- a/bffhd/db/mod.rs +++ b/bffhd/db/mod.rs @@ -1,5 +1,8 @@ use thiserror::Error; +// for converting a database error into a failed promise +use capnp; + mod raw; use miette::{Diagnostic, Severity}; @@ -79,3 +82,9 @@ impl Diagnostic for Error { None } } + +impl From for capnp::Error { + fn from(dberr: Error) -> capnp::Error { + capnp::Error::failed(format!("database error: {}", dberr.to_string())) + } +} diff --git a/bffhd/resources/mod.rs b/bffhd/resources/mod.rs index 369efe3..56dccf2 100644 --- a/bffhd/resources/mod.rs +++ b/bffhd/resources/mod.rs @@ -85,10 +85,13 @@ impl Inner { self.db.put(&self.id.as_bytes(), &state).unwrap(); tracing::trace!("Updated DB, sending update signal"); - AUDIT + let res = AUDIT .get() .unwrap() .log(self.id.as_str(), &format!("{}", state)); + if let Err(e) = res { + tracing::error!("Writing to the audit log failed for {} {}: {e}", self.id.as_str(), state); + } self.signal.set(state); tracing::trace!("Sent update signal"); @@ -161,7 +164,7 @@ impl Resource { fn set_state(&self, state: MachineState) { let mut serializer = AllocSerializer::<1024>::default(); - serializer.serialize_value(&state); + serializer.serialize_value(&state).expect("serializing a MachineState shoud be infallible"); let archived = ArchivedValue::new(serializer.into_serializer().into_inner()); self.inner.set_state(archived) } diff --git a/bffhd/resources/state/db.rs b/bffhd/resources/state/db.rs index 05a9658..e8ad67c 100644 --- a/bffhd/resources/state/db.rs +++ b/bffhd/resources/state/db.rs @@ -52,8 +52,8 @@ impl StateDB { } pub fn open_with_env(env: Arc) -> Result { - let db = unsafe { RawDB::open(&env, Some("state")) }; - let db = db.map_err(|e| StateDBError::Open(e.into()))?; + let db = RawDB::open(&env, Some("state")) + .map_err(|e| StateDBError::Open(e.into()))?; Ok(Self::new(env, db)) } @@ -64,8 +64,8 @@ impl StateDB { pub fn create_with_env(env: Arc) -> Result { let flags = DatabaseFlags::empty(); - let db = unsafe { RawDB::create(&env, Some("state"), flags) }; - let db = db.map_err(|e| StateDBError::Create(e.into()))?; + let db = RawDB::create(&env, Some("state"), flags) + .map_err(|e| StateDBError::Create(e.into()))?; Ok(Self::new(env, db)) } diff --git a/bffhd/users/db.rs b/bffhd/users/db.rs index df92444..817f45a 100644 --- a/bffhd/users/db.rs +++ b/bffhd/users/db.rs @@ -182,9 +182,8 @@ impl UserDB { } pub fn clear_txn(&self, txn: &mut RwTransaction) -> Result<(), db::Error> { - // TODO: why is the result ignored here? - self.db.clear(txn); - Ok(()) + // TODO: why was the result ignored here? + self.db.clear(txn) } pub fn get_all(&self) -> Result, db::Error> { diff --git a/build.rs b/build.rs index 37f84e1..e23aab1 100644 --- a/build.rs +++ b/build.rs @@ -1,4 +1,4 @@ fn main() { // Extract build-time information using the `shadow-rs` crate - shadow_rs::new(); + shadow_rs::new().unwrap(); }