fix: Context::execute_sync_items: Ignore all errors (#4817)

An error while executing an item mustn't prevent next items from being executed. There was a comment
that only critical errors like db write failures must be reported upstack, but in fact it's hard to
achieve in the current design, there are no error codes or so, so it's bug-prone. E.g.
`ChatAction::Block` and `Unblock` already reported all errors upstack. So, let's make error handling
the same as everywhere and just ignore any errors in the item execution loop. In the worst case we
just do more unsuccessful db writes f.e.
This commit is contained in:
iequidoo
2023-11-09 05:30:54 -03:00
committed by iequidoo
parent fa61d90115
commit b06a7e7197
3 changed files with 36 additions and 37 deletions

View File

@@ -4274,9 +4274,6 @@ impl Context {
}
ChatAction::SetContacts(addrs) => set_contacts_by_addrs(self, chat_id, addrs).await,
}
.log_err(self)
.ok();
Ok(())
}
}

View File

@@ -291,9 +291,7 @@ pub(crate) async fn receive_imf_inner(
if let Some(ref sync_items) = mime_parser.sync_items {
if from_id == ContactId::SELF {
if mime_parser.was_encrypted() {
if let Err(err) = context.execute_sync_items(sync_items).await {
warn!(context, "receive_imf cannot execute sync items: {err:#}.");
}
context.execute_sync_items(sync_items).await;
} else {
warn!(context, "Sync items are not encrypted.");
}

View File

@@ -10,6 +10,7 @@ use crate::config::Config;
use crate::constants::Blocked;
use crate::contact::ContactId;
use crate::context::Context;
use crate::log::LogExt;
use crate::message::{Message, MsgId, Viewtype};
use crate::mimeparser::SystemMessage;
use crate::param::Param;
@@ -252,41 +253,44 @@ impl Context {
/// as otherwise we would add in a dead-loop between two devices
/// sending message back and forth.
///
/// If an error is returned, the caller shall not try over.
/// Therefore, errors should only be returned on database errors or so.
/// If eg. just an item cannot be deleted,
/// that should not hold off the other items to be executed.
pub(crate) async fn execute_sync_items(&self, items: &SyncItems) -> Result<()> {
/// If an error is returned, the caller shall not try over because some sync items could be
/// already executed. Sync items are considered independent and executed in the given order but
/// regardless of whether executing of the previous items succeeded.
pub(crate) async fn execute_sync_items(&self, items: &SyncItems) {
info!(self, "executing {} sync item(s)", items.items.len());
for item in &items.items {
match &item.data {
AddQrToken(token) => {
let chat_id = if let Some(grpid) = &token.grpid {
if let Some((chat_id, _, _)) =
chat::get_chat_id_by_grpid(self, grpid).await?
{
Some(chat_id)
} else {
warn!(
self,
"Ignoring token for nonexistent/deleted group '{}'.", grpid
);
continue;
}
} else {
None
};
token::save(self, Namespace::InviteNumber, chat_id, &token.invitenumber)
.await?;
token::save(self, Namespace::Auth, chat_id, &token.auth).await?;
}
DeleteQrToken(token) => {
token::delete(self, Namespace::InviteNumber, &token.invitenumber).await?;
token::delete(self, Namespace::Auth, &token.auth).await?;
}
AlterChat { id, action } => self.sync_alter_chat(id, action).await?,
AddQrToken(token) => self.add_qr_token(token).await,
DeleteQrToken(token) => self.delete_qr_token(token).await,
AlterChat { id, action } => self.sync_alter_chat(id, action).await,
}
.log_err(self)
.ok();
}
}
async fn add_qr_token(&self, token: &QrTokenData) -> Result<()> {
let chat_id = if let Some(grpid) = &token.grpid {
if let Some((chat_id, _, _)) = chat::get_chat_id_by_grpid(self, grpid).await? {
Some(chat_id)
} else {
warn!(
self,
"Ignoring token for nonexistent/deleted group '{}'.", grpid
);
return Ok(());
}
} else {
None
};
token::save(self, Namespace::InviteNumber, chat_id, &token.invitenumber).await?;
token::save(self, Namespace::Auth, chat_id, &token.auth).await?;
Ok(())
}
async fn delete_qr_token(&self, token: &QrTokenData) -> Result<()> {
token::delete(self, Namespace::InviteNumber, &token.invitenumber).await?;
token::delete(self, Namespace::Auth, &token.auth).await?;
Ok(())
}
}
@@ -516,7 +520,7 @@ mod tests {
.to_string(),
)
?;
t.execute_sync_items(&sync_items).await?;
t.execute_sync_items(&sync_items).await;
assert!(
Contact::lookup_id_by_addr(&t, "bob@example.net", Origin::Unknown)