[MERGE] [FIX] fetchmail: more coherent and robust behavior of fetchmail. Notably, do not discard the whole email stack or stop after first crashed email processing.

Featuring:
- [FORWARD] [FIX] Forward port of revision 6920 of 6.1 addons branch, revid:odo@openerp.com-20120727150051-e1q0m92tyrazz82f. Purpose: some fixes and more robuts handling of incoming emails
- [IMP] mail: mail_thread: message routing: raise exceptions instead of using asserts. Indeed assert are statements meant to be used when developping, for debug purpose. In a production environment it is safer to use real exceptions that can be managed accordingly.
- [FIX] mail: fixed tests, no more AssertionError, but ValueError
- [IMP] fetchmail: try / except each email processing. This allows to print an exception in the logs for every failed email processing, without crashing the whole stack. Any failed or succeeded email is set as seen, to avoid crashing every time the scheduler runs.

bzr revid: tde@openerp.com-20140213115922-33vxgj7nfy86ashw
This commit is contained in:
Thibault Delavallée
2014-02-13 12:59:22 +01:00
3 changed files with 50 additions and 28 deletions
+30 -19
View File
@@ -188,7 +188,7 @@ openerp_mailgate: "|/path/to/openerp-mailgate.py --host=localhost -u %(uid)d -p
for server in self.browse(cr, uid, ids, context=context):
_logger.info('start checking for new emails on %s server %s', server.type, server.name)
context.update({'fetchmail_server_id': server.id, 'server_type': server.type})
count = 0
count, failed = 0, 0
imap_server = False
pop_server = False
if server.type == 'imap':
@@ -197,20 +197,26 @@ openerp_mailgate: "|/path/to/openerp-mailgate.py --host=localhost -u %(uid)d -p
imap_server.select()
result, data = imap_server.search(None, '(UNSEEN)')
for num in data[0].split():
res_id = None
result, data = imap_server.fetch(num, '(RFC822)')
res_id = mail_thread.message_process(cr, uid, server.object_id.model,
data[0][1],
save_original=server.original,
strip_attachments=(not server.attach),
context=context)
imap_server.store(num, '-FLAGS', '\\Seen')
try:
res_id = mail_thread.message_process(cr, uid, server.object_id.model,
data[0][1],
save_original=server.original,
strip_attachments=(not server.attach),
context=context)
except Exception:
_logger.exception('Failed to process mail from %s server %s.', server.type, server.name)
failed += 1
if res_id and server.action_id:
action_pool.run(cr, uid, [server.action_id.id], {'active_id': res_id, 'active_ids':[res_id], 'active_model': context.get("thread_model", server.object_id.model)})
imap_server.store(num, '+FLAGS', '\\Seen')
cr.commit()
action_pool.run(cr, uid, [server.action_id.id], {'active_id': res_id, 'active_ids': [res_id], 'active_model': context.get("thread_model", server.object_id.model)})
imap_server.store(num, '+FLAGS', '\\Seen')
cr.commit()
count += 1
_logger.info("fetched/processed %s email(s) on %s server %s", count, server.type, server.name)
_logger.info("Fetched %d email(s) on %s server %s; %d succeeded, %d failed.", count, server.type, server.name, (count - failed), failed)
except Exception:
_logger.exception("Failed to fetch mail from %s server %s.", server.type, server.name)
_logger.exception("General failure when trying to fetch mail from %s server %s.", server.type, server.name)
finally:
if imap_server:
imap_server.close()
@@ -223,18 +229,23 @@ openerp_mailgate: "|/path/to/openerp-mailgate.py --host=localhost -u %(uid)d -p
for num in range(1, numMsgs + 1):
(header, msges, octets) = pop_server.retr(num)
msg = '\n'.join(msges)
res_id = mail_thread.message_process(cr, uid, server.object_id.model,
msg,
save_original=server.original,
strip_attachments=(not server.attach),
context=context)
res_id = None
try:
res_id = mail_thread.message_process(cr, uid, server.object_id.model,
msg,
save_original=server.original,
strip_attachments=(not server.attach),
context=context)
except Exception:
_logger.exception('Failed to process mail from %s server %s.', server.type, server.name)
failed += 1
if res_id and server.action_id:
action_pool.run(cr, uid, [server.action_id.id], {'active_id': res_id, 'active_ids':[res_id], 'active_model': context.get("thread_model", server.object_id.model)})
action_pool.run(cr, uid, [server.action_id.id], {'active_id': res_id, 'active_ids': [res_id], 'active_model': context.get("thread_model", server.object_id.model)})
pop_server.dele(num)
cr.commit()
_logger.info("fetched/processed %s email(s) on %s server %s", numMsgs, server.type, server.name)
_logger.info("Fetched %d email(s) on %s server %s; %d succeeded, %d failed.", numMsgs, server.type, server.name, (numMsgs - failed), failed)
except Exception:
_logger.exception("Failed to fetch mail from %s server %s.", server.type, server.name)
_logger.exception("General failure when trying to fetch mail from %s server %s.", server.type, server.name)
finally:
if pop_server:
pop_server.quit()
+18 -7
View File
@@ -521,8 +521,11 @@ class mail_thread(osv.AbstractModel):
to which this mail should be attached. Only used if the message
does not reply to an existing thread and does not match any mail alias.
:return: list of [model, thread_id, custom_values, user_id]
:raises: ValueError, TypeError
"""
assert isinstance(message, Message), 'message must be an email.message.Message at this point'
if not isinstance(message, Message):
raise TypeError('message must be an email.message.Message at this point')
message_id = message.get('Message-Id')
email_from = decode_header(message, 'From')
email_to = decode_header(message, 'To')
@@ -593,9 +596,12 @@ class mail_thread(osv.AbstractModel):
thread_id = int(thread_id)
except:
thread_id = False
assert thread_id and hasattr(model_pool, 'message_update') or hasattr(model_pool, 'message_new'), \
"No possible route found for incoming message from %s to %s (Message-Id %s:)." \
"Create an appropriate mail.alias or force the destination model." % (email_from, email_to, message_id)
if not (thread_id and hasattr(model_pool, 'message_update') or hasattr(model_pool, 'message_new')):
raise ValueError(
'No possible route found for incoming message from %s to %s (Message-Id %s:). '
'Create an appropriate mail.alias or force the destination model.' %
(email_from, email_to, message_id)
)
if thread_id and not model_pool.exists(cr, uid, thread_id):
_logger.warning('Received mail reply to missing document %s! Ignoring and creating new document instead for Message-Id %s',
thread_id, message_id)
@@ -637,6 +643,8 @@ class mail_thread(osv.AbstractModel):
to which this mail should be attached. When provided, this
overrides the automatic detection based on the message
headers.
:raises: ValueError, TypeError
"""
if context is None:
context = {}
@@ -679,9 +687,11 @@ class mail_thread(osv.AbstractModel):
context.update({'thread_model': model})
if model:
model_pool = self.pool.get(model)
assert thread_id and hasattr(model_pool, 'message_update') or hasattr(model_pool, 'message_new'), \
"Undeliverable mail with Message-Id %s, model %s does not accept incoming emails" % \
if not (thread_id and hasattr(model_pool, 'message_update') or hasattr(model_pool, 'message_new')):
raise ValueError(
"Undeliverable mail with Message-Id %s, model %s does not accept incoming emails" %
(msg['message_id'], model)
)
# disabled subscriptions during message_new/update to avoid having the system user running the
# email gateway become a follower of all inbound messages
@@ -692,7 +702,8 @@ class mail_thread(osv.AbstractModel):
nosub_ctx = dict(nosub_ctx, mail_create_nolog=True)
thread_id = model_pool.message_new(cr, user_id, msg, custom_values, context=nosub_ctx)
else:
assert thread_id == 0, "Posting a message without model should be with a null res_id, to create a private message."
if thread_id:
raise ValueError("Posting a message without model should be with a null res_id, to create a private message.")
model_pool = self.pool.get('mail.thread')
new_msg_id = model_pool.message_post(cr, uid, [thread_id], context=context, subtype='mail.mt_comment', **msg)
+2 -2
View File
@@ -411,14 +411,14 @@ class TestMailgateway(TestMailBase):
# --------------------------------------------------
# Do: incoming email with model that does not accepts incoming emails must raise
self.assertRaises(AssertionError,
self.assertRaises(ValueError,
format_and_process,
MAIL_TEMPLATE,
to='noone@example.com', subject='spam', extra='', model='res.country',
msg_id='<1198923581.41972151344608186760.JavaMail.new4@agrolait.com>')
# Do: incoming email without model and without alias must raise
self.assertRaises(AssertionError,
self.assertRaises(ValueError,
format_and_process,
MAIL_TEMPLATE,
to='noone@example.com', subject='spam', extra='',