- resultify send-out pipeline for better error reporting

- early ignore sending out smtp messages with no recipients
This commit is contained in:
holger krekel
2019-10-02 18:45:31 +02:00
parent 3f7995a7ea
commit 489cdd1b24
4 changed files with 70 additions and 54 deletions

View File

@@ -1245,8 +1245,11 @@ pub unsafe extern "C" fn dc_forward_msgs(
let ffi_context = &*context; let ffi_context = &*context;
ffi_context ffi_context
.with_inner(|ctx| chat::forward_msgs(ctx, ids, chat_id)) .with_inner(|ctx| {
.unwrap_or_log_default(ctx, "Failed to forard message") chat::forward_msgs(ctx, ids, chat_id)
.unwrap_or_log_default(ctx, "Failed to forward message")
})
.unwrap_or_default()
} }
#[no_mangle] #[no_mangle]

View File

@@ -805,8 +805,7 @@ pub fn send_msg(context: &Context, chat_id: u32, msg: &mut Message) -> Result<u3
break; break;
} else { } else {
if let Ok(mut copy) = Message::load_from_db(context, id as u32) { if let Ok(mut copy) = Message::load_from_db(context, id as u32) {
// TODO: handle cleanup and return early instead send_msg(context, 0, &mut copy)?;
send_msg(context, 0, &mut copy).unwrap();
} }
} }
} }
@@ -1421,7 +1420,7 @@ pub(crate) fn add_contact_to_chat_ex(
msg.param.set_int(Param::Cmd, 4); msg.param.set_int(Param::Cmd, 4);
msg.param.set(Param::Arg, contact.get_addr()); msg.param.set(Param::Arg, contact.get_addr());
msg.param.set_int(Param::Arg2, from_handshake.into()); msg.param.set_int(Param::Arg2, from_handshake.into());
msg.id = send_msg(context, chat_id, &mut msg).unwrap_or_default(); msg.id = send_msg(context, chat_id, &mut msg)?;
context.call_cb(Event::MsgsChanged { context.call_cb(Event::MsgsChanged {
chat_id, chat_id,
msg_id: msg.id, msg_id: msg.id,
@@ -1532,7 +1531,7 @@ pub fn remove_contact_from_chat(
} }
msg.param.set_int(Param::Cmd, 5); msg.param.set_int(Param::Cmd, 5);
msg.param.set(Param::Arg, contact.get_addr()); msg.param.set(Param::Arg, contact.get_addr());
msg.id = send_msg(context, chat_id, &mut msg).unwrap_or_default(); msg.id = send_msg(context, chat_id, &mut msg)?;
context.call_cb(Event::MsgsChanged { context.call_cb(Event::MsgsChanged {
chat_id, chat_id,
msg_id: msg.id, msg_id: msg.id,
@@ -1629,7 +1628,7 @@ pub fn set_chat_name(
if !chat.name.is_empty() { if !chat.name.is_empty() {
msg.param.set(Param::Arg, &chat.name); msg.param.set(Param::Arg, &chat.name);
} }
msg.id = send_msg(context, chat_id, &mut msg).unwrap_or_default(); msg.id = send_msg(context, chat_id, &mut msg)?;
context.call_cb(Event::MsgsChanged { context.call_cb(Event::MsgsChanged {
chat_id, chat_id,
msg_id: msg.id, msg_id: msg.id,
@@ -1698,7 +1697,7 @@ pub fn set_chat_profile_image(
"", "",
DC_CONTACT_ID_SELF, DC_CONTACT_ID_SELF,
)); ));
msg.id = send_msg(context, chat_id, &mut msg).unwrap_or_default(); msg.id = send_msg(context, chat_id, &mut msg)?;
emit_event!( emit_event!(
context, context,
Event::MsgsChanged { Event::MsgsChanged {

View File

@@ -167,32 +167,36 @@ impl Job {
// was sent we need to mark it in the database ASAP as we // was sent we need to mark it in the database ASAP as we
// otherwise might send it twice. // otherwise might send it twice.
let mut sock = context.smtp.lock().unwrap(); let mut sock = context.smtp.lock().unwrap();
if !sock.send(context, recipients_list, body) { match sock.send(context, recipients_list, body) {
sock.disconnect(); Err(err) => {
self.try_again_later(-1i32, sock.error.clone()); sock.disconnect();
} else { warn!(context, "smtp failed: {}", err);
// smtp success, update DB as quick as possible self.try_again_later(-1i32, Some(err.to_string()));
if 0 != self.foreign_id { }
message::update_msg_state( Ok(()) => {
context, // smtp success, update db ASAP, then delete smtp file
self.foreign_id, if 0 != self.foreign_id {
MessageState::OutDelivered, message::update_msg_state(
); context,
let chat_id: i32 = context self.foreign_id,
.sql MessageState::OutDelivered,
.query_get_value( );
context, let chat_id: i32 = context
"SELECT chat_id FROM msgs WHERE id=?", .sql
params![self.foreign_id as i32], .query_get_value(
) context,
.unwrap_or_default(); "SELECT chat_id FROM msgs WHERE id=?",
context.call_cb(Event::MsgDelivered { params![self.foreign_id as i32],
chat_id: chat_id as u32, )
msg_id: self.foreign_id, .unwrap_or_default();
}); context.call_cb(Event::MsgDelivered {
chat_id: chat_id as u32,
msg_id: self.foreign_id,
});
}
// now also delete the generated file
dc_delete_file(context, filename);
} }
// now also delete the generated file
dc_delete_file(context, filename);
} }
} else { } else {
warn!(context, "Missing recipients for job {}", self.job_id,); warn!(context, "Missing recipients for job {}", self.job_id,);
@@ -214,7 +218,7 @@ impl Job {
if !inbox.is_connected() { if !inbox.is_connected() {
connect_to_inbox(context, &inbox); connect_to_inbox(context, &inbox);
if !inbox.is_connected() { if !inbox.is_connected() {
self.try_again_later(3, None); self.try_again_later(3, Some("could not connect".into()));
return; return;
} }
} }
@@ -612,10 +616,6 @@ pub fn job_action_exists(context: &Context, action: Action) -> bool {
pub fn job_send_msg(context: &Context, msg_id: u32) -> Result<(), Error> { pub fn job_send_msg(context: &Context, msg_id: u32) -> Result<(), Error> {
let mut mimefactory = MimeFactory::load_msg(context, msg_id)?; let mut mimefactory = MimeFactory::load_msg(context, msg_id)?;
ensure!(!mimefactory.recipients_addr.is_empty(),
"msg {} has no recipients, can not smtp-sent", msg_id
);
if chat::msgtype_has_file(mimefactory.msg.type_0) { if chat::msgtype_has_file(mimefactory.msg.type_0) {
let file_param = mimefactory let file_param = mimefactory
.msg .msg
@@ -672,6 +672,15 @@ pub fn job_send_msg(context: &Context, msg_id: u32) -> Result<(), Error> {
.recipients_addr .recipients_addr
.push(mimefactory.from_addr.to_string()); .push(mimefactory.from_addr.to_string());
} }
if mimefactory.recipients_addr.is_empty() {
warn!(
context,
"message {} has no recipient, skipping smtp-send", msg_id
);
return Ok(());
}
if mimefactory.out_gossiped { if mimefactory.out_gossiped {
chat::set_gossiped_timestamp(context, mimefactory.msg.chat_id, time()); chat::set_gossiped_timestamp(context, mimefactory.msg.chat_id, time());
} }

View File

@@ -3,6 +3,7 @@ use lettre::*;
use crate::constants::*; use crate::constants::*;
use crate::context::Context; use crate::context::Context;
use crate::error::Error;
use crate::events::Event; use crate::events::Event;
use crate::login_param::LoginParam; use crate::login_param::LoginParam;
use crate::oauth2::*; use crate::oauth2::*;
@@ -14,7 +15,6 @@ pub struct Smtp {
transport_connected: bool, transport_connected: bool,
/// Email address we are sending from. /// Email address we are sending from.
from: Option<EmailAddress>, from: Option<EmailAddress>,
pub error: Option<String>,
} }
impl Smtp { impl Smtp {
@@ -24,7 +24,6 @@ impl Smtp {
transport: None, transport: None,
transport_connected: false, transport_connected: false,
from: None, from: None,
error: None,
} }
} }
@@ -132,11 +131,19 @@ impl Smtp {
context: &Context, context: &Context,
recipients: Vec<EmailAddress>, recipients: Vec<EmailAddress>,
message: Vec<u8>, message: Vec<u8>,
) -> bool { ) -> Result<(), Error> {
let message_len = message.len(); let message_len = message.len();
let recipients_display = recipients
.iter()
.map(|x| format!("{}", x))
.collect::<Vec<String>>()
.join(",");
if let Some(ref mut transport) = self.transport { if let Some(ref mut transport) = self.transport {
let envelope = Envelope::new(self.from.clone(), recipients).expect("invalid envelope"); let envelope = Envelope::new(self.from.clone(), recipients);
ensure!(envelope.is_ok(), "internal smtp-message construction fail");
let envelope = envelope.unwrap();
let mail = SendableEmail::new( let mail = SendableEmail::new(
envelope, envelope,
"mail-id".into(), // TODO: random id "mail-id".into(), // TODO: random id
@@ -145,24 +152,22 @@ impl Smtp {
match transport.send(mail) { match transport.send(mail) {
Ok(_) => { Ok(_) => {
context.call_cb(Event::SmtpMessageSent( context.call_cb(Event::SmtpMessageSent(format!(
"Message was sent to SMTP server".into(), "Message len={} was smtp-sent to {}",
)); message_len, recipients_display
)));
self.transport_connected = true; self.transport_connected = true;
true return Ok(());
} }
Err(err) => { Err(err) => {
warn!( bail!("SMTP failed len={}: error: {}", message_len, err);
context,
"SMTP failed message size {}, error: {}", message_len, err
);
self.error = Some(format!("{}", err));
false
} }
} }
} else { } else {
warn!(context, "SMTP Failed to send message to {:?}", recipients); bail!(
false "uh? SMTP has no transport, failed to send to {:?}",
recipients_display
);
} }
} }
} }