start with some refactoring of the outgoing pipeline

deltachat-ffi/src/lib.rs

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

examples/repl/cmdline.rs

@@ -864,7 +864,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
             let mut msg_ids = [0; 1];
             let chat_id = arg2.parse()?;
             msg_ids[0] = arg1.parse()?;
-            chat::forward_msgs(context, &msg_ids, chat_id);
+            chat::forward_msgs(context, &msg_ids, chat_id)?;
         }
         "markseen" => {
             ensure!(!arg1.is_empty(), "Argument <msg-id> missing.");

src/chat.rs

@@ -784,10 +784,7 @@ pub fn send_msg(context: &Context, chat_id: u32, msg: &mut Message) -> Result<u3
         message::update_msg_state(context, msg.id, MessageState::OutPending);
     }
 
-    ensure!(
+    job_send_msg(context, msg.id)?;
-        job_send_msg(context, msg.id) != 0,
-        "Failed to initiate send job"
-    );
 
     context.call_cb(Event::MsgsChanged {
         chat_id: msg.chat_id,
@@ -1717,10 +1714,12 @@ pub fn set_chat_profile_image(
     bail!("Failed to set profile image");
 }
 
-pub fn forward_msgs(context: &Context, msg_ids: &[u32], chat_id: u32) {
+pub fn forward_msgs(context: &Context, msg_ids: &[u32], chat_id: u32) -> Result<(), Error> {
-    if msg_ids.is_empty() || chat_id <= DC_CHAT_ID_LAST_SPECIAL {
+    ensure!(!msg_ids.is_empty(), "empty msgs_ids: no one to forward to");
-        return;
+    ensure!(
-    }
+        chat_id > DC_CHAT_ID_LAST_SPECIAL,
+        "can not forward to special chat"
+    );
 
     let mut created_db_entries = Vec::new();
     let mut curr_timestamp: i64;
@@ -1790,7 +1789,7 @@ pub fn forward_msgs(context: &Context, msg_ids: &[u32], chat_id: u32) {
                 new_msg_id = chat
                     .prepare_msg_raw(context, &mut msg, fresh10)
                     .unwrap_or_default();
-                job_send_msg(context, new_msg_id);
+                job_send_msg(context, new_msg_id)?;
             }
             created_db_entries.push(chat_id);
             created_db_entries.push(new_msg_id);
@@ -1803,6 +1802,8 @@ pub fn forward_msgs(context: &Context, msg_ids: &[u32], chat_id: u32) {
             msg_id: created_db_entries[i + 1],
         });
     }
+
+    Ok(())
 }
 
 pub fn get_chat_contact_cnt(context: &Context, chat_id: u32) -> usize {

src/job.rs

@@ -8,6 +8,7 @@ use crate::configure::*;
 use crate::constants::*;
 use crate::context::Context;
 use crate::dc_tools::*;
+use crate::error::Error;
 use crate::events::Event;
 use crate::imap::*;
 use crate::imex::*;
@@ -144,7 +145,7 @@ impl Job {
                         .filter_map(|addr| match lettre::EmailAddress::new(addr.to_string()) {
                             Ok(addr) => Some(addr),
                             Err(err) => {
-                                eprintln!("WARNING: invalid recipient: {} {:?}", addr, err);
+                                warn!(context, "invalid recipient: {} {:?}", addr, err);
                                 None
                             }
                         })
@@ -156,21 +157,21 @@ impl Job {
                     if 0 != self.foreign_id && !message::exists(context, self.foreign_id) {
                         warn!(
                             context,
-                            "Message {} for job {} does not exist", self.foreign_id, self.job_id,
+                            "Not sending Message {} as it was deleted", self.foreign_id
                         );
                         return;
                     };
 
                     // hold the smtp lock during sending of a job and
                     // its ok/error response processing. Note that if a message
-                    // was sent we need to mark it in the database as we
+                    // was sent we need to mark it in the database ASAP as we
                     // otherwise might send it twice.
                     let mut sock = context.smtp.lock().unwrap();
-                    if 0 == sock.send(context, recipients_list, body) {
+                    if !sock.send(context, recipients_list, body) {
                         sock.disconnect();
                         self.try_again_later(-1i32, sock.error.clone());
                     } else {
-                        dc_delete_file(context, filename);
+                        // smtp success, update DB as quick as possible
                         if 0 != self.foreign_id {
                             message::update_msg_state(
                                 context,
@@ -190,6 +191,8 @@ impl Job {
                                 msg_id: self.foreign_id,
                             });
                         }
+                        // now also delete the generated file
+                        dc_delete_file(context, filename);
                     }
                 } else {
                     warn!(context, "Missing recipients for job {}", self.job_id,);
@@ -322,7 +325,12 @@ impl Job {
                                 self.try_again_later(3i32, None);
                             }
                             ImapResult::Success => {
-                                send_mdn(context, msg.id);
+                                if let Err(err) = send_mdn(context, msg.id) {
+                                    warn!(
+                                        context,
+                                        "could not send out mdn for {}: {}", msg.id, err
+                                    );
+                                }
                             }
                             ImapResult::Failed | ImapResult::AlreadyDone => {}
                         }
@@ -601,109 +609,101 @@ pub fn job_action_exists(context: &Context, action: Action) -> bool {
 
 /* special case for DC_JOB_SEND_MSG_TO_SMTP */
 #[allow(non_snake_case)]
-pub fn job_send_msg(context: &Context, msg_id: u32) -> libc::c_int {
+pub fn job_send_msg(context: &Context, msg_id: u32) -> Result<(), Error> {
-    let mut success = 0;
+    let mut mimefactory = MimeFactory::load_msg(context, msg_id)?;
 
-    /* load message data */
+    ensure!(!mimefactory.recipients_addr.is_empty(), 
-    let mimefactory = MimeFactory::load_msg(context, msg_id);
+            "msg {} has no recipients, can not smtp-sent", msg_id
-    if mimefactory.is_err() {
+    );
-        warn!(
-            context,
-            "Cannot load data to send, maybe the message is deleted in between.",
-        );
-    } else {
-        let mut mimefactory = mimefactory.unwrap();
-        // no redo, no IMAP. moreover, as the data does not exist, there is no need in calling dc_set_msg_failed()
-        if chat::msgtype_has_file(mimefactory.msg.type_0) {
-            let file_param = mimefactory
-                .msg
-                .param
-                .get(Param::File)
-                .map(|s| s.to_string());
-            if let Some(pathNfilename) = file_param {
-                if (mimefactory.msg.type_0 == Viewtype::Image
-                    || mimefactory.msg.type_0 == Viewtype::Gif)
-                    && !mimefactory.msg.param.exists(Param::Width)
-                {
-                    mimefactory.msg.param.set_int(Param::Width, 0);
-                    mimefactory.msg.param.set_int(Param::Height, 0);
 
-                    if let Ok(buf) = dc_read_file(context, pathNfilename) {
+    if chat::msgtype_has_file(mimefactory.msg.type_0) {
-                        if let Ok((width, height)) = dc_get_filemeta(&buf) {
+        let file_param = mimefactory
-                            mimefactory.msg.param.set_int(Param::Width, width as i32);
+            .msg
-                            mimefactory.msg.param.set_int(Param::Height, height as i32);
+            .param
-                        }
+            .get(Param::File)
-                    }
+            .map(|s| s.to_string());
-                    mimefactory.msg.save_param_to_disk(context);
+        if let Some(pathNfilename) = file_param {
-                }
+            if (mimefactory.msg.type_0 == Viewtype::Image
-            }
+                || mimefactory.msg.type_0 == Viewtype::Gif)
-        }
+                && !mimefactory.msg.param.exists(Param::Width)
-        /* create message */
-        if let Err(msg) = unsafe { mimefactory.render() } {
-            let e = msg.to_string();
-            message::set_msg_failed(context, msg_id, Some(e));
-        } else if 0
-            != mimefactory
-                .msg
-                .param
-                .get_int(Param::GuranteeE2ee)
-                .unwrap_or_default()
-            && !mimefactory.out_encrypted
-        {
-            /* unrecoverable */
-            warn!(
-                context,
-                "e2e encryption unavailable {} - {:?}",
-                msg_id,
-                mimefactory.msg.param.get_int(Param::GuranteeE2ee),
-            );
-            message::set_msg_failed(
-                context,
-                msg_id,
-                Some("End-to-end-encryption unavailable unexpectedly."),
-            );
-        } else {
-            if !vec_contains_lowercase(&mimefactory.recipients_addr, &mimefactory.from_addr) {
-                mimefactory.recipients_names.push("".to_string());
-                mimefactory
-                    .recipients_addr
-                    .push(mimefactory.from_addr.to_string());
-            }
-            if mimefactory.out_gossiped {
-                chat::set_gossiped_timestamp(context, mimefactory.msg.chat_id, time());
-            }
-            if 0 != mimefactory.out_last_added_location_id {
-                if let Err(err) =
-                    location::set_kml_sent_timestamp(context, mimefactory.msg.chat_id, time())
-                {
-                    error!(context, "Failed to set kml sent_timestamp: {:?}", err);
-                }
-                if !mimefactory.msg.hidden {
-                    if let Err(err) = location::set_msg_location_id(
-                        context,
-                        mimefactory.msg.id,
-                        mimefactory.out_last_added_location_id,
-                    ) {
-                        error!(context, "Failed to set msg_location_id: {:?}", err);
-                    }
-                }
-            }
-            if mimefactory.out_encrypted
-                && mimefactory
-                    .msg
-                    .param
-                    .get_int(Param::GuranteeE2ee)
-                    .unwrap_or_default()
-                    == 0
             {
-                mimefactory.msg.param.set_int(Param::GuranteeE2ee, 1);
+                mimefactory.msg.param.set_int(Param::Width, 0);
+                mimefactory.msg.param.set_int(Param::Height, 0);
+
+                if let Ok(buf) = dc_read_file(context, pathNfilename) {
+                    if let Ok((width, height)) = dc_get_filemeta(&buf) {
+                        mimefactory.msg.param.set_int(Param::Width, width as i32);
+                        mimefactory.msg.param.set_int(Param::Height, height as i32);
+                    }
+                }
                 mimefactory.msg.save_param_to_disk(context);
             }
-            success = add_smtp_job(context, Action::SendMsgToSmtp, &mut mimefactory);
         }
     }
 
-    success
+    /* create message */
+    if let Err(msg) = unsafe { mimefactory.render() } {
+        let e = msg.to_string();
+        message::set_msg_failed(context, msg_id, Some(e));
+        return Err(msg);
+    }
+    if 0 != mimefactory
+        .msg
+        .param
+        .get_int(Param::GuranteeE2ee)
+        .unwrap_or_default()
+        && !mimefactory.out_encrypted
+    {
+        /* unrecoverable */
+        message::set_msg_failed(
+            context,
+            msg_id,
+            Some("End-to-end-encryption unavailable unexpectedly."),
+        );
+        bail!(
+            "e2e encryption unavailable {} - {:?}",
+            msg_id,
+            mimefactory.msg.param.get_int(Param::GuranteeE2ee),
+        );
+    }
+    if !vec_contains_lowercase(&mimefactory.recipients_addr, &mimefactory.from_addr) {
+        mimefactory.recipients_names.push("".to_string());
+        mimefactory
+            .recipients_addr
+            .push(mimefactory.from_addr.to_string());
+    }
+    if mimefactory.out_gossiped {
+        chat::set_gossiped_timestamp(context, mimefactory.msg.chat_id, time());
+    }
+    if 0 != mimefactory.out_last_added_location_id {
+        if let Err(err) = location::set_kml_sent_timestamp(context, mimefactory.msg.chat_id, time())
+        {
+            error!(context, "Failed to set kml sent_timestamp: {:?}", err);
+        }
+        if !mimefactory.msg.hidden {
+            if let Err(err) = location::set_msg_location_id(
+                context,
+                mimefactory.msg.id,
+                mimefactory.out_last_added_location_id,
+            ) {
+                error!(context, "Failed to set msg_location_id: {:?}", err);
+            }
+        }
+    }
+    if mimefactory.out_encrypted
+        && mimefactory
+            .msg
+            .param
+            .get_int(Param::GuranteeE2ee)
+            .unwrap_or_default()
+            == 0
+    {
+        mimefactory.msg.param.set_int(Param::GuranteeE2ee, 1);
+        mimefactory.msg.save_param_to_disk(context);
+    }
+    add_smtp_job(context, Action::SendMsgToSmtp, &mut mimefactory)?;
+
+    Ok(())
 }
 
 pub fn perform_imap_jobs(context: &Context) {
@@ -955,16 +955,20 @@ fn connect_to_inbox(context: &Context, inbox: &Imap) -> libc::c_int {
     ret_connected
 }
 
-fn send_mdn(context: &Context, msg_id: u32) {
+fn send_mdn(context: &Context, msg_id: u32) -> Result<(), Error> {
-    if let Ok(mut mimefactory) = MimeFactory::load_mdn(context, msg_id) {
+    let mut mimefactory = MimeFactory::load_mdn(context, msg_id)?;
-        if unsafe { mimefactory.render() }.is_ok() {
+    unsafe { mimefactory.render()? };
-            add_smtp_job(context, Action::SendMdn, &mut mimefactory);
+    add_smtp_job(context, Action::SendMdn, &mut mimefactory)?;
-        }
+
-    }
+    Ok(())
 }
 
 #[allow(non_snake_case)]
-fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> libc::c_int {
+fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) -> Result<(), Error> {
+    ensure!(
+        !mimefactory.recipients_addr.is_empty(),
+        "no recipients for smtp job set"
+    );
     let mut param = Params::new();
     let bytes = unsafe {
         std::slice::from_raw_parts(
@@ -972,16 +976,7 @@ fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) ->
             (*mimefactory.out).len,
         )
     };
-    let bpath = match context.new_blob_file(&mimefactory.rfc724_mid, bytes) {
+    let bpath = context.new_blob_file(&mimefactory.rfc724_mid, bytes)?;
-        Ok(path) => path,
-        Err(err) => {
-            error!(
-                context,
-                "Could not write {} smtp-message, error {}", mimefactory.rfc724_mid, err
-            );
-            return 0;
-        }
-    };
     info!(context, "add_smtp_job file written: {:?}", bpath);
     let recipients = mimefactory.recipients_addr.join("\x1e");
     param.set(Param::File, &bpath);
@@ -997,7 +992,8 @@ fn add_smtp_job(context: &Context, action: Action, mimefactory: &MimeFactory) ->
         param,
         0,
     );
-    1
+
+    Ok(())
 }
 
 pub fn job_add(

src/mimefactory.rs

@@ -141,52 +141,40 @@ impl<'a> MimeFactory<'a> {
     }
 
     /*******************************************************************************
-     * Render
+     * Render a basic email
      ******************************************************************************/
     // restrict unsafe to parts, introduce wrapmime helpers where appropriate
     pub unsafe fn render(&mut self) -> Result<(), Error> {
         if self.loaded == Loaded::Nothing || !self.out.is_null() {
             bail!("Invalid use of mimefactory-object.");
         }
-        let context = &self.context;
+        ensure!(
-
+            !self.recipients_names.is_empty() && !self.recipients_addr.is_empty(),
-        /* create basic mail
+            "message has no recipients"
-         *************************************************************************/
-
-        let from: *mut mailimf_mailbox_list = mailimf_mailbox_list_new_empty();
-        mailimf_mailbox_list_add(
-            from,
-            mailimf_mailbox_new(
-                if !self.from_displayname.is_empty() {
-                    dc_encode_header_words(&self.from_displayname).strdup()
-                } else {
-                    ptr::null_mut()
-                },
-                self.from_addr.strdup(),
-            ),
         );
-        let mut to: *mut mailimf_address_list = ptr::null_mut();
+
-        if !self.recipients_names.is_empty() && !self.recipients_addr.is_empty() {
+        let context = &self.context;
-            to = mailimf_address_list_new_empty();
+        let from = wrapmime::new_mailbox_list(&self.from_displayname, &self.from_addr);
-            let name_iter = self.recipients_names.iter();
+
-            let addr_iter = self.recipients_addr.iter();
+        let to = mailimf_address_list_new_empty();
-            for (name, addr) in name_iter.zip(addr_iter) {
+        let name_iter = self.recipients_names.iter();
-                mailimf_address_list_add(
+        let addr_iter = self.recipients_addr.iter();
-                    to,
+        for (name, addr) in name_iter.zip(addr_iter) {
-                    mailimf_address_new(
+            mailimf_address_list_add(
-                        MAILIMF_ADDRESS_MAILBOX as libc::c_int,
+                to,
-                        mailimf_mailbox_new(
+                mailimf_address_new(
-                            if !name.is_empty() {
+                    MAILIMF_ADDRESS_MAILBOX as libc::c_int,
-                                dc_encode_header_words(&name).strdup()
+                    mailimf_mailbox_new(
-                            } else {
+                        if !name.is_empty() {
-                                ptr::null_mut()
+                            dc_encode_header_words(&name).strdup()
-                            },
+                        } else {
-                            addr.strdup(),
+                            ptr::null_mut()
-                        ),
+                        },
-                        ptr::null_mut(),
+                        addr.strdup(),
                     ),
-                );
+                    ptr::null_mut(),
-            }
+                ),
+            );
         }
         let references_list = if !self.references.is_empty() {
             dc_str_to_clist(&self.references, " ")
@@ -198,6 +186,7 @@ impl<'a> MimeFactory<'a> {
         } else {
             ptr::null_mut()
         };
+
         let imf_fields = mailimf_fields_new_with_data_all(
             mailimf_get_date(self.timestamp as i64),
             from,

src/smtp.rs

@@ -125,18 +125,22 @@ impl Smtp {
         }
     }
 
+    /// SMTP-Send a prepared mail to recipients.
+    /// returns boolean whether send was successful.
     pub fn send<'a>(
         &mut self,
         context: &Context,
         recipients: Vec<EmailAddress>,
-        body: Vec<u8>,
+        message: Vec<u8>,
-    ) -> usize {
+    ) -> bool {
+        let message_len = message.len();
+
         if let Some(ref mut transport) = self.transport {
             let envelope = Envelope::new(self.from.clone(), recipients).expect("invalid envelope");
             let mail = SendableEmail::new(
                 envelope,
                 "mail-id".into(), // TODO: random id
-                body,
+                message,
             );
 
             match transport.send(mail) {
@@ -145,17 +149,20 @@ impl Smtp {
                         "Message was sent to SMTP server".into(),
                     ));
                     self.transport_connected = true;
-                    1
+                    true
                 }
                 Err(err) => {
-                    warn!(context, "SMTP failed to send message: {}", err);
+                    warn!(
+                        context,
+                        "SMTP failed message size {}, error: {}", message_len, err
+                    );
                     self.error = Some(format!("{}", err));
-                    0
+                    false
                 }
             }
         } else {
-            // TODO: log error
+            warn!(context, "SMTP Failed to send message to {:?}", recipients);
-            0
+            false
         }
     }
 }

src/wrapmime.rs

@@ -3,6 +3,7 @@ use std::ffi::CString;
 use std::ptr;
 
 use crate::contact::addr_normalize;
+use crate::dc_strencode::*;
 use crate::dc_tools::*;
 use crate::error::Error;
 use mmime::clist::*;
@@ -480,6 +481,24 @@ pub fn content_type_needs_encoding(content: *const mailmime_content) -> bool {
     }
 }
 
+pub fn new_mailbox_list(displayname: &str, addr: &str) -> *mut mailimf_mailbox_list {
+    let mbox: *mut mailimf_mailbox_list = unsafe { mailimf_mailbox_list_new_empty() };
+    unsafe {
+        mailimf_mailbox_list_add(
+            mbox,
+            mailimf_mailbox_new(
+                if !displayname.is_empty() {
+                    dc_encode_header_words(&displayname).strdup()
+                } else {
+                    ptr::null_mut()
+                },
+                addr.strdup(),
+            ),
+        );
+    }
+    mbox
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;