From f9cc3cbef034f0024fc4e1a045cd43c9c1fd8d9f Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 16 Sep 2020 13:45:32 +0200 Subject: [PATCH 01/10] Resultify sql.open() --- src/context.rs | 5 +---- src/imex.rs | 48 ++++++++++++++++++++++++++---------------------- src/sql.rs | 20 +++++++++++++------- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/context.rs b/src/context.rs index d281a936f..b070a3a56 100644 --- a/src/context.rs +++ b/src/context.rs @@ -136,10 +136,7 @@ impl Context { let ctx = Context { inner: Arc::new(inner), }; - ensure!( - ctx.sql.open(&ctx, &ctx.dbfile, false).await, - "Failed opening sqlite database" - ); + ctx.sql.open(&ctx, &ctx.dbfile, false).await?; Ok(ctx) } diff --git a/src/imex.rs b/src/imex.rs index 50805707e..93bb958de 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -6,6 +6,7 @@ use std::{ ffi::OsStr, }; +use anyhow::Context as _; use async_std::path::{Path, PathBuf}; use async_std::{ fs::{self, File}, @@ -118,7 +119,11 @@ async fn cleanup_aborted_imex(context: &Context, what: ImexMode) { dc_delete_files_in_dir(context, context.get_blobdir()).await; } if what == ImexMode::ExportBackup || what == ImexMode::ImportBackup { - context.sql.open(context, context.get_dbfile(), false).await; + context + .sql + .open(context, context.get_dbfile(), false) + .await + .ok(); } } @@ -166,7 +171,7 @@ pub async fn has_backup_old(context: &Context, dir_name: impl AsRef) -> Re let name = name.to_string_lossy(); if name.starts_with("delta-chat") && name.ends_with(".bak") { let sql = Sql::new(); - if sql.open(context, &path, true).await { + if sql.open(context, &path, true).await.is_ok() { let curr_backup_time = sql .get_raw_config_int(context, "backup_time") .await @@ -520,13 +525,11 @@ async fn import_backup(context: &Context, backup_to_import: impl AsRef) -> } } - ensure!( - context - .sql - .open(&context, &context.get_dbfile(), false) - .await, - "could not re-open db" - ); + context + .sql + .open(&context, &context.get_dbfile(), false) + .await + .context("Could not re-open db")?; delete_and_reset_all_device_msgs(&context).await?; @@ -558,13 +561,11 @@ async fn import_backup_old(context: &Context, backup_to_import: impl AsRef ); /* error already logged */ /* re-open copied database file */ - ensure!( - context - .sql - .open(&context, &context.get_dbfile(), false) - .await, - "could not re-open db" - ); + context + .sql + .open(&context, &context.get_dbfile(), false) + .await + .context("Could not re-open db")?; delete_and_reset_all_device_msgs(&context).await?; @@ -743,7 +744,7 @@ async fn export_backup_old(context: &Context, dir: impl AsRef) -> Result<( context .sql .open(&context, &context.get_dbfile(), false) - .await; + .await?; if !copied { bail!( @@ -753,11 +754,14 @@ async fn export_backup_old(context: &Context, dir: impl AsRef) -> Result<( ); } let dest_sql = Sql::new(); - ensure!( - dest_sql.open(context, &dest_path_filename, false).await, - "could not open exported database {}", - dest_path_string - ); + dest_sql + .open(context, &dest_path_filename, false) + .await + .context(format!( + "could not open exported database {}", + dest_path_string + ))?; + let res = match add_files_to_export(context, &dest_sql).await { Err(err) => { dc_delete_file(context, &dest_path_filename).await; diff --git a/src/sql.rs b/src/sql.rs index 86400c70e..e5105b39b 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -14,6 +14,7 @@ use crate::constants::{ShowEmails, DC_CHAT_ID_TRASH}; use crate::context::Context; use crate::dc_tools::*; use crate::ephemeral::start_ephemeral_timers; +use crate::error::format_err; use crate::param::*; use crate::peerstate::*; @@ -78,17 +79,22 @@ impl Sql { } // return true on success, false on failure - pub async fn open>(&self, context: &Context, dbfile: T, readonly: bool) -> bool { - match open(context, self, dbfile, readonly).await { - Ok(_) => true, - Err(err) => match err.downcast_ref::() { - Some(Error::SqlAlreadyOpen) => false, + pub async fn open>( + &self, + context: &Context, + dbfile: T, + readonly: bool, + ) -> crate::error::Result<()> { + let res = open(context, self, dbfile, readonly).await; + if let Err(err) = &res { + match err.downcast_ref::() { + Some(Error::SqlAlreadyOpen) => {} _ => { self.close().await; - false } - }, + } } + res.map_err(|e| format_err!("Could not open db: {}", e)) } pub async fn execute>( From 396ccebb5c4eb66b5b566b1bd049947988f84c98 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 16 Sep 2020 15:00:22 +0200 Subject: [PATCH 02/10] Log more --- src/imex.rs | 28 +++++++++++++++++----------- src/sql.rs | 1 - 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/imex.rs b/src/imex.rs index 93bb958de..60ec4740d 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -123,7 +123,7 @@ async fn cleanup_aborted_imex(context: &Context, what: ImexMode) { .sql .open(context, context.get_dbfile(), false) .await - .ok(); + .map_err(|e| warn!(context, "Re-opening db after imex failed: {}", e)); } } @@ -171,17 +171,23 @@ pub async fn has_backup_old(context: &Context, dir_name: impl AsRef) -> Re let name = name.to_string_lossy(); if name.starts_with("delta-chat") && name.ends_with(".bak") { let sql = Sql::new(); - if sql.open(context, &path, true).await.is_ok() { - let curr_backup_time = sql - .get_raw_config_int(context, "backup_time") - .await - .unwrap_or_default(); - if curr_backup_time > newest_backup_time { - newest_backup_path = Some(path); - newest_backup_time = curr_backup_time; + match sql.open(context, &path, true).await { + Ok(_) => { + let curr_backup_time = sql + .get_raw_config_int(context, "backup_time") + .await + .unwrap_or_default(); + if curr_backup_time > newest_backup_time { + newest_backup_path = Some(path); + newest_backup_time = curr_backup_time; + } + info!(context, "backup_time of {} is {}", name, curr_backup_time); + sql.close().await; } - info!(context, "backup_time of {} is {}", name, curr_backup_time); - sql.close().await; + Err(e) => warn!( + context, + "Found backup file {} which could not be opened: {}", name, e + ), } } } diff --git a/src/sql.rs b/src/sql.rs index e5105b39b..137155125 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -78,7 +78,6 @@ impl Sql { // drop closes the connection } - // return true on success, false on failure pub async fn open>( &self, context: &Context, From 27e53ddbff754758dc74a5ead88fcd5565c01307 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 16 Sep 2020 15:00:36 +0200 Subject: [PATCH 03/10] Just for testing, let importing the db always fail - .context() just overwrites the underlying error!!!!! --- src/imex.rs | 4 ++-- src/sql.rs | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/imex.rs b/src/imex.rs index 60ec4740d..bc0423632 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -569,9 +569,9 @@ async fn import_backup_old(context: &Context, backup_to_import: impl AsRef /* re-open copied database file */ context .sql - .open(&context, &context.get_dbfile(), false) + .openh(&context, &context.get_dbfile(), false) .await - .context("Could not re-open db")?; + .context("that's just the context")?; delete_and_reset_all_device_msgs(&context).await?; diff --git a/src/sql.rs b/src/sql.rs index 137155125..451b6de8a 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -96,6 +96,15 @@ impl Sql { res.map_err(|e| format_err!("Could not open db: {}", e)) } + pub async fn openh>( + &self, + context: &Context, + dbfile: T, + readonly: bool, + ) -> crate::error::Result<()> { + Err(format_err!("This is the actual error the user should see")) + } + pub async fn execute>( &self, sql: S, From e870b33e0325b8217846811134cf476b87d43240 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 16 Sep 2020 15:05:05 +0200 Subject: [PATCH 04/10] Revert "Just for testing, let importing the db always fail - .context() just overwrites the underlying error!!!!!" This reverts commit 27e53ddbff754758dc74a5ead88fcd5565c01307. --- src/imex.rs | 4 ++-- src/sql.rs | 9 --------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/imex.rs b/src/imex.rs index bc0423632..60ec4740d 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -569,9 +569,9 @@ async fn import_backup_old(context: &Context, backup_to_import: impl AsRef /* re-open copied database file */ context .sql - .openh(&context, &context.get_dbfile(), false) + .open(&context, &context.get_dbfile(), false) .await - .context("that's just the context")?; + .context("Could not re-open db")?; delete_and_reset_all_device_msgs(&context).await?; diff --git a/src/sql.rs b/src/sql.rs index 451b6de8a..137155125 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -96,15 +96,6 @@ impl Sql { res.map_err(|e| format_err!("Could not open db: {}", e)) } - pub async fn openh>( - &self, - context: &Context, - dbfile: T, - readonly: bool, - ) -> crate::error::Result<()> { - Err(format_err!("This is the actual error the user should see")) - } - pub async fn execute>( &self, sql: S, From b892dafa4921d94d0e8d3cba992a67dc1a821164 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 16 Sep 2020 15:14:37 +0200 Subject: [PATCH 05/10] Clippy --- src/imex.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/imex.rs b/src/imex.rs index 60ec4740d..4e554f00c 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -119,11 +119,9 @@ async fn cleanup_aborted_imex(context: &Context, what: ImexMode) { dc_delete_files_in_dir(context, context.get_blobdir()).await; } if what == ImexMode::ExportBackup || what == ImexMode::ImportBackup { - context - .sql - .open(context, context.get_dbfile(), false) - .await - .map_err(|e| warn!(context, "Re-opening db after imex failed: {}", e)); + if let Err(e) = context.sql.open(context, context.get_dbfile(), false).await { + warn!(context, "Re-opening db after imex failed: {}", e); + } } } From 4ed2638594fb6d3306ea12237d5fecfcb206b31f Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 16 Sep 2020 16:02:25 +0200 Subject: [PATCH 06/10] Also show the inner error --- src/imex.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/imex.rs b/src/imex.rs index 4e554f00c..4972d316c 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -95,7 +95,8 @@ pub async fn imex( } Err(err) => { cleanup_aborted_imex(context, what).await; - error!(context, "{}", err); + // We are using Anyhow's .context() and to show the inner error, too, we need the {:#}: + error!(context, "{:#}", err); context.emit_event(EventType::ImexProgress(0)); bail!("IMEX FAILED to complete: {}", err); } From 6d6ac66f4dff483f8875eb3c04992263e4d0dc03 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Wed, 16 Sep 2020 17:24:04 +0200 Subject: [PATCH 07/10] Show dbfile when opening fails --- src/sql.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sql.rs b/src/sql.rs index 137155125..89e281bcb 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -84,7 +84,7 @@ impl Sql { dbfile: T, readonly: bool, ) -> crate::error::Result<()> { - let res = open(context, self, dbfile, readonly).await; + let res = open(context, self, &dbfile, readonly).await; if let Err(err) = &res { match err.downcast_ref::() { Some(Error::SqlAlreadyOpen) => {} @@ -93,7 +93,7 @@ impl Sql { } } } - res.map_err(|e| format_err!("Could not open db: {}", e)) + res.map_err(|e| format_err!("Could not open db file {}: {}", dbfile.as_ref().to_string_lossy(), e)) } pub async fn execute>( From aee6eb2261d85a7b5d048aad84bf44ae14539176 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 17 Sep 2020 09:39:14 +0200 Subject: [PATCH 08/10] {:#} once more --- src/sql.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/sql.rs b/src/sql.rs index 89e281bcb..81e4a9807 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -93,7 +93,14 @@ impl Sql { } } } - res.map_err(|e| format_err!("Could not open db file {}: {}", dbfile.as_ref().to_string_lossy(), e)) + res.map_err(|e| { + format_err!( + // We are using Anyhow's .context() and to show the inner error, too, we need the {:#}: + "Could not open db file {}: {:#}", + dbfile.as_ref().to_string_lossy(), + e + ) + }) } pub async fn execute>( From 6253a2cef76a65ba366f5fdf53ed8a3e52397dc9 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 17 Sep 2020 12:27:57 +0200 Subject: [PATCH 09/10] . --- src/imex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/imex.rs b/src/imex.rs index 4972d316c..137b62af1 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -762,7 +762,7 @@ async fn export_backup_old(context: &Context, dir: impl AsRef) -> Result<( dest_sql .open(context, &dest_path_filename, false) .await - .context(format!( + .with_context(|| format!( "could not open exported database {}", dest_path_string ))?; From 99a36e8629e8769581ece20a3a5e4f244786fe0b Mon Sep 17 00:00:00 2001 From: Hocuri Date: Thu, 17 Sep 2020 17:29:29 +0200 Subject: [PATCH 10/10] rustfmt --- src/imex.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/imex.rs b/src/imex.rs index 137b62af1..fc01b81b5 100644 --- a/src/imex.rs +++ b/src/imex.rs @@ -762,10 +762,7 @@ async fn export_backup_old(context: &Context, dir: impl AsRef) -> Result<( dest_sql .open(context, &dest_path_filename, false) .await - .with_context(|| format!( - "could not open exported database {}", - dest_path_string - ))?; + .with_context(|| format!("could not open exported database {}", dest_path_string))?; let res = match add_files_to_export(context, &dest_sql).await { Err(err) => {