From 98fc5595363c46ed493d7f24ff51a61b0ff97ba9 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sat, 13 Mar 2021 21:06:37 +0100 Subject: [PATCH] Even nicer logging: Add ok_or_log() and more (#2284) Co-authored-by: Floris Bruynooghe --- deltachat-ffi/src/lib.rs | 22 ++------ examples/repl/cmdline.rs | 2 +- src/imex.rs | 2 +- src/job.rs | 2 +- src/log.rs | 109 +++++++++++++++++++++++++++++---------- 5 files changed, 90 insertions(+), 47 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 839ecb626..0c8703457 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -24,7 +24,6 @@ use std::time::{Duration, SystemTime}; use async_std::task::{block_on, spawn}; use num_traits::{FromPrimitive, ToPrimitive}; -use deltachat::accounts::Accounts; use deltachat::chat::{ChatId, ChatVisibility, MuteDuration, ProtectionStatus}; use deltachat::constants::DC_MSG_ID_LAST_SPECIAL; use deltachat::contact::{Contact, Origin}; @@ -34,6 +33,7 @@ use deltachat::key::DcKey; use deltachat::message::MsgId; use deltachat::stock_str::StockMessage; use deltachat::*; +use deltachat::{accounts::Accounts, log::LogExt}; mod dc_array; @@ -3451,15 +3451,11 @@ pub unsafe extern "C" fn dc_str_unref(s: *mut libc::c_char) { } trait ResultExt { + /// Like `log_err()`, but: + /// - returns the default value instead of an Err value. + /// - emits an error instead of a warning for an [Err] result. This means + /// that the error will be shown to the user in a small pop-up. fn unwrap_or_log_default(self, context: &context::Context, message: &str) -> T; - - /// Log a warning to a [ContextWrapper] for an [Err] result. - /// - /// Does nothing for an [Ok]. - /// - /// You can do this as soon as the wrapper exists, it does not - /// have to be open (which is required for the `warn!()` macro). - fn log_err(self, wrapper: &Context, message: &str) -> Result; } impl ResultExt for Result { @@ -3472,14 +3468,6 @@ impl ResultExt for Result { } } } - - fn log_err(self, ctx: &Context, message: &str) -> Result { - self.map_err(|err| { - // We are using Anyhow's .context() and to show the inner error, too, we need the {:#}: - warn!(ctx, "{}: {:#}", message, err); - err - }) - } } trait ResultNullableExt { diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index fc17b47dd..6b7cea6ba 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -514,7 +514,7 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu context.maybe_network().await; } "housekeeping" => { - sql::housekeeping(&context).await.log(&context); + sql::housekeeping(&context).await.ok_or_log(&context); } "listchats" | "listarchived" | "chats" => { let listflags = if arg0 == "listarchived" { 0x01 } else { 0 }; diff --git a/src/imex.rs b/src/imex.rs index aa42b729c..ad97b5c01 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -676,7 +676,7 @@ async fn export_backup(context: &Context, dir: impl AsRef) -> Result<()> { .sql .set_raw_config_int(context, "backup_time", now as i32) .await?; - sql::housekeeping(context).await.log(context); + sql::housekeeping(context).await.ok_or_log(context); context .sql diff --git a/src/job.rs b/src/job.rs index 35626bf56..8c373b4fb 100644 --- a/src/job.rs +++ b/src/job.rs @@ -1157,7 +1157,7 @@ async fn perform_job_action( Action::MoveMsg => job.move_msg(context, connection.inbox()).await, Action::FetchExistingMsgs => job.fetch_existing_msgs(context, connection.inbox()).await, Action::Housekeeping => { - sql::housekeeping(context).await.log(context); + sql::housekeeping(context).await.ok_or_log(context); Status::Finished(Ok(())) } }; diff --git a/src/log.rs b/src/log.rs index cb5304738..ce76fc9f6 100644 --- a/src/log.rs +++ b/src/log.rs @@ -1,5 +1,4 @@ //! # Logging - use crate::context::Context; #[macro_export] @@ -61,38 +60,94 @@ macro_rules! emit_event { }; } -pub trait LogExt { +pub trait LogExt +where + Self: std::marker::Sized, +{ + #[track_caller] + fn log_err_inner(self, context: &Context, msg: Option<&str>) -> Result; + /// Emits a warning if the receiver contains an Err value. /// - /// Returns an [`Option`] with the `Ok(_)` value, if any: - /// - You won't get any warnings about unused results but can still use the value if you need it - /// - This prevents the same warning from being printed to the log multiple times - /// /// Thanks to the [track_caller](https://blog.rust-lang.org/2020/08/27/Rust-1.46.0.html#track_caller) /// feature, the location of the caller is printed to the log, just like with the warn!() macro. + /// + /// Unfortunately, the track_caller feature does not work on async functions (as of Rust 1.50). + /// Once it is, you can add `#[track_caller]` to helper functions that use one of the log helpers here + /// so that the location of the caller can be seen in the log. (this won't work with the macros, + /// like warn!(), since the file!() and line!() macros don't work with track_caller) + /// See https://github.com/rust-lang/rust/issues/78840 for progress on this. #[track_caller] - fn log(self, context: &Context) -> Option; -} + fn log_err(self, context: &Context, msg: &str) -> Result { + self.log_err_inner(context, Some(msg)) + } -impl LogExt for anyhow::Result { + /// Emits a warning if the receiver contains an Err value and returns an [`Option`]. + /// + /// Example: + /// ```text + /// if let Err(e) = do_something() { + /// warn!(context, "{:#}", e); + /// } + /// ``` + /// is equivalent to: + /// ```text + /// do_something().ok_or_log(context); + /// ``` + /// + /// For a note on the `track_caller` feature, see the doc comment on `log_err()`. #[track_caller] - fn log(self, context: &Context) -> Option { - match self { - Err(e) => { - let location = std::panic::Location::caller(); - // We are using Anyhow's .context() and to show the inner error, too, we need the {:#}: - let full = format!( - "{file}:{line}: {e:#}", - file = location.file(), - line = location.line(), - e = e - ); - // We can't use the warn!() macro here as the file!() and line!() macros - // don't work well with #[track_caller] - emit_event!(context, crate::EventType::Warning(full)); - None - } - Ok(v) => Some(v), - } + fn ok_or_log(self, context: &Context) -> Option { + self.log_err_inner(context, None).ok() + } + + /// Like `ok_or_log()`, but you can pass an extra message that is prepended in the log. + /// + /// Example: + /// ```text + /// if let Err(e) = do_something() { + /// warn!(context, "Something went wrong: {:#}", e); + /// } + /// ``` + /// is equivalent to: + /// ```text + /// do_something().ok_or_log_msg(context, "Something went wrong"); + /// ``` + /// and is also equivalent to: + /// ```text + /// use anyhow::Context as _; + /// do_something().context("Something went wrong").ok_or_log(context); + /// ``` + /// + /// For a note on the `track_caller` feature, see the doc comment on `log_err()`. + #[track_caller] + fn ok_or_log_msg(self, context: &Context, msg: &'static str) -> Option { + self.log_err_inner(context, Some(msg)).ok() + } +} + +impl LogExt for Result { + #[track_caller] + fn log_err_inner(self, context: &Context, msg: Option<&str>) -> Result { + if let Err(e) = &self { + let location = std::panic::Location::caller(); + + let separator = if msg.is_none() { "" } else { ": " }; + let msg = msg.unwrap_or_default(); + + // We are using Anyhow's .context() and to show the inner error, too, we need the {:#}: + let full = format!( + "{file}:{line}: {msg}{separator}{e:#}", + file = location.file(), + line = location.line(), + msg = msg, + separator = separator, + e = e + ); + // We can't use the warn!() macro here as the file!() and line!() macros + // don't work with #[track_caller] + emit_event!(context, crate::EventType::Warning(full)); + }; + self } }