Patch for error handling / separation of accounts etc.

Dear All,
I have made the attached patch to try and make offlineimap a bit more
stable in challenging situations.  It's extremely useful in slow
connection environments - but sometimes if one account had the wrong
password or the connection went down then unfortunately the whole
program would crash.

I have tested this on our connection and tried throwing at it just about
every situation - connection, up down, up, down again, change password,
error whilst copying one message, etc.  I have been running this patch
for the last 5 days or so syncing 6 accounts at the moment...  It seems
to work and stay alive nicely (even if your connection does not)...

Hope that this can go in for the next release... Please let me know if
anyone notices any problems with this...

Regards,

-Mike

-- Attached file included as plaintext by Ecartis --
-- File: submit

From 1d6777cab23637eb830031c7cab0ae9b8589afd6 Mon Sep 17 00:00:00 2001
From: mike <mike@mikelaptop.(none)>
Date: Mon, 24 Aug 2009 19:37:59 +0430
Subject: [PATCH] This patch attempts to introduce a little more error handling - e.g.
 if one account has an error because of a changed password or something
 that should not affect the other accounts.

Specifically:
If one sync run has an issue this is in a try-except clause - if it
has an auto refresh period the thread will sleep and try again - this
could be quite useful in the event of the connection going down for a
little while, changed password etc.

If one folder cannot be created an error message will be displayed through
the UI and the program will continue (e.g. permission denied to create a folder)

If one message does not want to copy for whatever resaon an error message
will be displayed through the UI and at least the other messages will
be copied

If one folder run has an exception then the others will still run
This commit is contained in:
Mike Dawson 2009-08-24 19:47:57 +04:30 committed by John Goerzen
parent 43ead072a1
commit 30344587d9
4 changed files with 221 additions and 164 deletions

View File

@ -23,6 +23,7 @@ from subprocess import Popen, PIPE
from threading import Event, Lock from threading import Event, Lock
import os import os
from Queue import Queue, Empty from Queue import Queue, Empty
import sys
class SigListener(Queue): class SigListener(Queue):
def __init__(self): def __init__(self):
@ -181,18 +182,27 @@ class AccountSynchronizationMixin:
#might need changes here to ensure that one account sync does not crash others... #might need changes here to ensure that one account sync does not crash others...
if not self.refreshperiod: if not self.refreshperiod:
try:
self.sync(siglistener) self.sync(siglistener)
self.ui.acctdone(self.name) except:
self.ui.warn("Error occured attempting to sync account " + self.name \
+ ": " + str(sys.exc_info()[1]))
finally:
self.ui.acctdone(self.name)
return return
looping = 1 looping = 1
while looping: while looping:
self.sync(siglistener) try:
looping = self.sleeper(siglistener) != 2 self.sync(siglistener)
self.ui.acctdone(self.name) except:
self.ui.warn("Error occured attempting to sync account " + self.name \
+ ": " + str(sys.exc_info()[1]))
finally:
looping = self.sleeper(siglistener) != 2
self.ui.acctdone(self.name)
def getaccountmeta(self): def getaccountmeta(self):
@ -276,83 +286,86 @@ def syncfolder(accountname, remoterepos, remotefolder, localrepos,
global mailboxes global mailboxes
ui = UIBase.getglobalui() ui = UIBase.getglobalui()
ui.registerthread(accountname) ui.registerthread(accountname)
# Load local folder. try:
localfolder = localrepos.\ # Load local folder.
getfolder(remotefolder.getvisiblename().\ localfolder = localrepos.\
replace(remoterepos.getsep(), localrepos.getsep())) getfolder(remotefolder.getvisiblename().\
# Write the mailboxes replace(remoterepos.getsep(), localrepos.getsep()))
mbnames.add(accountname, localfolder.getvisiblename()) # Write the mailboxes
mbnames.add(accountname, localfolder.getvisiblename())
# Load status folder. # Load status folder.
statusfolder = statusrepos.getfolder(remotefolder.getvisiblename().\ statusfolder = statusrepos.getfolder(remotefolder.getvisiblename().\
replace(remoterepos.getsep(), replace(remoterepos.getsep(),
statusrepos.getsep())) statusrepos.getsep()))
if localfolder.getuidvalidity() == None: if localfolder.getuidvalidity() == None:
# This is a new folder, so delete the status cache to be sure # This is a new folder, so delete the status cache to be sure
# we don't have a conflict. # we don't have a conflict.
statusfolder.deletemessagelist() statusfolder.deletemessagelist()
statusfolder.cachemessagelist()
if quick: statusfolder.cachemessagelist()
if not localfolder.quickchanged(statusfolder) \
and not remotefolder.quickchanged(statusfolder):
ui.skippingfolder(remotefolder)
localrepos.restore_atime()
return
# Load local folder if quick:
ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder) if not localfolder.quickchanged(statusfolder) \
ui.loadmessagelist(localrepos, localfolder) and not remotefolder.quickchanged(statusfolder):
localfolder.cachemessagelist() ui.skippingfolder(remotefolder)
ui.messagelistloaded(localrepos, localfolder, len(localfolder.getmessagelist().keys())) localrepos.restore_atime()
return
# If either the local or the status folder has messages and there is a UID # Load local folder
# validity problem, warn and abort. If there are no messages, UW IMAPd ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder)
# loses UIDVALIDITY. But we don't really need it if both local folders are ui.loadmessagelist(localrepos, localfolder)
# empty. So, in that case, just save it off. localfolder.cachemessagelist()
if len(localfolder.getmessagelist()) or len(statusfolder.getmessagelist()): ui.messagelistloaded(localrepos, localfolder, len(localfolder.getmessagelist().keys()))
if not localfolder.isuidvalidityok():
ui.validityproblem(localfolder)
localrepos.restore_atime()
return
if not remotefolder.isuidvalidityok():
ui.validityproblem(remotefolder)
localrepos.restore_atime()
return
else:
localfolder.saveuidvalidity()
remotefolder.saveuidvalidity()
# Load remote folder. # If either the local or the status folder has messages and there is a UID
ui.loadmessagelist(remoterepos, remotefolder) # validity problem, warn and abort. If there are no messages, UW IMAPd
remotefolder.cachemessagelist() # loses UIDVALIDITY. But we don't really need it if both local folders are
ui.messagelistloaded(remoterepos, remotefolder, # empty. So, in that case, just save it off.
len(remotefolder.getmessagelist().keys())) if len(localfolder.getmessagelist()) or len(statusfolder.getmessagelist()):
if not localfolder.isuidvalidityok():
ui.validityproblem(localfolder)
localrepos.restore_atime()
return
if not remotefolder.isuidvalidityok():
ui.validityproblem(remotefolder)
localrepos.restore_atime()
return
else:
localfolder.saveuidvalidity()
remotefolder.saveuidvalidity()
# Load remote folder.
ui.loadmessagelist(remoterepos, remotefolder)
remotefolder.cachemessagelist()
ui.messagelistloaded(remoterepos, remotefolder,
len(remotefolder.getmessagelist().keys()))
# #
if not statusfolder.isnewfolder(): if not statusfolder.isnewfolder():
# Delete local copies of remote messages. This way, # Delete local copies of remote messages. This way,
# if a message's flag is modified locally but it has been # if a message's flag is modified locally but it has been
# deleted remotely, we'll delete it locally. Otherwise, we # deleted remotely, we'll delete it locally. Otherwise, we
# try to modify a deleted message's flags! This step # try to modify a deleted message's flags! This step
# need only be taken if a statusfolder is present; otherwise, # need only be taken if a statusfolder is present; otherwise,
# there is no action taken *to* the remote repository. # there is no action taken *to* the remote repository.
remotefolder.syncmessagesto_delete(localfolder, [localfolder, remotefolder.syncmessagesto_delete(localfolder, [localfolder,
statusfolder]) statusfolder])
ui.syncingmessages(localrepos, localfolder, remoterepos, remotefolder) ui.syncingmessages(localrepos, localfolder, remoterepos, remotefolder)
localfolder.syncmessagesto(statusfolder, [remotefolder, statusfolder]) localfolder.syncmessagesto(statusfolder, [remotefolder, statusfolder])
# Synchronize remote changes. # Synchronize remote changes.
ui.syncingmessages(remoterepos, remotefolder, localrepos, localfolder) ui.syncingmessages(remoterepos, remotefolder, localrepos, localfolder)
remotefolder.syncmessagesto(localfolder, [localfolder, statusfolder]) remotefolder.syncmessagesto(localfolder, [localfolder, statusfolder])
# Make sure the status folder is up-to-date.
ui.syncingmessages(localrepos, localfolder, statusrepos, statusfolder)
localfolder.syncmessagesto(statusfolder)
statusfolder.save()
localrepos.restore_atime()
# Make sure the status folder is up-to-date.
ui.syncingmessages(localrepos, localfolder, statusrepos, statusfolder)
localfolder.syncmessagesto(statusfolder)
statusfolder.save()
localrepos.restore_atime()
except:
ui.warn("ERROR in syncfolder for " + accountname + " folder " + \
remotefolder.getvisiblename() +" : " +str(sys.exc_info()[1]))

View File

@ -21,6 +21,7 @@ from offlineimap import threadutil
from offlineimap.threadutil import InstanceLimitedThread from offlineimap.threadutil import InstanceLimitedThread
from offlineimap.ui import UIBase from offlineimap.ui import UIBase
import os.path, re import os.path, re
import sys
class BaseFolder: class BaseFolder:
def __init__(self): def __init__(self):
@ -266,25 +267,30 @@ class BaseFolder:
# synced to the status cache. This is only a problem with # synced to the status cache. This is only a problem with
# self.getmessage(). So, don't call self.getmessage unless # self.getmessage(). So, don't call self.getmessage unless
# really needed. # really needed.
if register: try:
UIBase.getglobalui().registerthread(self.getaccountname()) if register:
UIBase.getglobalui().copyingmessage(uid, self, applyto) UIBase.getglobalui().registerthread(self.getaccountname())
message = '' UIBase.getglobalui().copyingmessage(uid, self, applyto)
# If any of the destinations actually stores the message body, message = ''
# load it up. # If any of the destinations actually stores the message body,
for object in applyto: # load it up.
if object.storesmessages():
message = self.getmessage(uid) for object in applyto:
break if object.storesmessages():
flags = self.getmessageflags(uid) message = self.getmessage(uid)
rtime = self.getmessagetime(uid) break
for object in applyto: flags = self.getmessageflags(uid)
newuid = object.savemessage(uid, message, flags, rtime) rtime = self.getmessagetime(uid)
if newuid > 0 and newuid != uid: for object in applyto:
# Change the local uid. newuid = object.savemessage(uid, message, flags, rtime)
self.savemessage(newuid, message, flags, rtime) if newuid > 0 and newuid != uid:
self.deletemessage(uid) # Change the local uid.
uid = newuid self.savemessage(newuid, message, flags, rtime)
self.deletemessage(uid)
uid = newuid
except:
UIBase.getglobalui().warn("ERROR attempting to copy message " + str(uid) \
+ " for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))
def syncmessagesto_copy(self, dest, applyto): def syncmessagesto_copy(self, dest, applyto):
@ -384,15 +390,30 @@ class BaseFolder:
applyto ones should be the ones that "copy" the main action.""" applyto ones should be the ones that "copy" the main action."""
if applyto == None: if applyto == None:
applyto = [dest] applyto = [dest]
self.syncmessagesto_neguid(dest, applyto) try:
self.syncmessagesto_neguid(dest, applyto)
except:
UIBase.getglobalui().warn("ERROR attempting to handle negative uids " \
+ "for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))
#all threads launched here are in try / except clauses when they copy anyway...
self.syncmessagesto_copy(dest, applyto) self.syncmessagesto_copy(dest, applyto)
self.syncmessagesto_delete(dest, applyto)
try:
self.syncmessagesto_delete(dest, applyto)
except:
UIBase.getglobalui().warn("ERROR attempting to delete messages " \
+ "for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))
# Now, the message lists should be identical wrt the uids present. # Now, the message lists should be identical wrt the uids present.
# (except for potential negative uids that couldn't be placed # (except for potential negative uids that couldn't be placed
# anywhere) # anywhere)
self.syncmessagesto_flags(dest, applyto) try:
self.syncmessagesto_flags(dest, applyto)
except:
UIBase.getglobalui().warn("ERROR attempting to sync flags " \
+ "for account " + self.getaccountname() + ":" + str(sys.exc_info()[1]))

View File

@ -246,72 +246,88 @@ class IMAPServer:
self.connectionlock.release() # Release until need to modify data self.connectionlock.release() # Release until need to modify data
""" Must be careful here that if we fail we should bail out gracefully
and release locks / threads so that the next attempt can try...
"""
success = 0 success = 0
while not success: try:
# Generate a new connection. while not success:
if self.tunnel: # Generate a new connection.
UIBase.getglobalui().connecting('tunnel', self.tunnel) if self.tunnel:
imapobj = UsefulIMAP4_Tunnel(self.tunnel) UIBase.getglobalui().connecting('tunnel', self.tunnel)
success = 1 imapobj = UsefulIMAP4_Tunnel(self.tunnel)
elif self.usessl:
UIBase.getglobalui().connecting(self.hostname, self.port)
imapobj = UsefulIMAP4_SSL(self.hostname, self.port,
self.sslclientkey, self.sslclientcert)
else:
UIBase.getglobalui().connecting(self.hostname, self.port)
imapobj = UsefulIMAP4(self.hostname, self.port)
imapobj.mustquote = imaplibutil.mustquote
if not self.tunnel:
try:
# Try GSSAPI and continue if it fails
if 'AUTH=GSSAPI' in imapobj.capabilities and have_gss:
UIBase.getglobalui().debug('imap',
'Attempting GSSAPI authentication')
try:
imapobj.authenticate('GSSAPI', self.gssauth)
except imapobj.error, val:
self.gssapi = False
UIBase.getglobalui().debug('imap',
'GSSAPI Authentication failed')
else:
self.gssapi = True
self.password = None
if not self.gssapi:
if 'AUTH=CRAM-MD5' in imapobj.capabilities:
UIBase.getglobalui().debug('imap',
'Attempting CRAM-MD5 authentication')
try:
imapobj.authenticate('CRAM-MD5', self.md5handler)
except imapobj.error, val:
self.plainauth(imapobj)
else:
self.plainauth(imapobj)
# Would bail by here if there was a failure.
success = 1 success = 1
self.goodpassword = self.password elif self.usessl:
except imapobj.error, val: UIBase.getglobalui().connecting(self.hostname, self.port)
self.passworderror = str(val) imapobj = UsefulIMAP4_SSL(self.hostname, self.port,
self.password = None self.sslclientkey, self.sslclientcert)
else:
UIBase.getglobalui().connecting(self.hostname, self.port)
imapobj = UsefulIMAP4(self.hostname, self.port)
if self.delim == None: imapobj.mustquote = imaplibutil.mustquote
listres = imapobj.list(self.reference, '""')[1]
if listres == [None] or listres == None:
# Some buggy IMAP servers do not respond well to LIST "" ""
# Work around them.
listres = imapobj.list(self.reference, '"*"')[1]
self.delim, self.root = \
imaputil.imapsplit(listres[0])[1:]
self.delim = imaputil.dequote(self.delim)
self.root = imaputil.dequote(self.root)
self.connectionlock.acquire() if not self.tunnel:
self.assignedconnections.append(imapobj) try:
self.lastowner[imapobj] = thread.get_ident() # Try GSSAPI and continue if it fails
self.connectionlock.release() if 'AUTH=GSSAPI' in imapobj.capabilities and have_gss:
return imapobj UIBase.getglobalui().debug('imap',
'Attempting GSSAPI authentication')
try:
imapobj.authenticate('GSSAPI', self.gssauth)
except imapobj.error, val:
self.gssapi = False
UIBase.getglobalui().debug('imap',
'GSSAPI Authentication failed')
else:
self.gssapi = True
#if we do self.password = None then the next attempt cannot try...
#self.password = None
if not self.gssapi:
if 'AUTH=CRAM-MD5' in imapobj.capabilities:
UIBase.getglobalui().debug('imap',
'Attempting CRAM-MD5 authentication')
try:
imapobj.authenticate('CRAM-MD5', self.md5handler)
except imapobj.error, val:
self.plainauth(imapobj)
else:
self.plainauth(imapobj)
# Would bail by here if there was a failure.
success = 1
self.goodpassword = self.password
except imapobj.error, val:
self.passworderror = str(val)
raise
#self.password = None
if self.delim == None:
listres = imapobj.list(self.reference, '""')[1]
if listres == [None] or listres == None:
# Some buggy IMAP servers do not respond well to LIST "" ""
# Work around them.
listres = imapobj.list(self.reference, '"*"')[1]
self.delim, self.root = \
imaputil.imapsplit(listres[0])[1:]
self.delim = imaputil.dequote(self.delim)
self.root = imaputil.dequote(self.root)
self.connectionlock.acquire()
self.assignedconnections.append(imapobj)
self.lastowner[imapobj] = thread.get_ident()
self.connectionlock.release()
return imapobj
except:
"""If we are here then we did not succeed in getting a connection -
we should clean up and then re-raise the error..."""
self.semaphore.release()
#Make sure that this can be retried the next time...
self.passworderror = None
if(self.connectionlock.locked()):
self.connectionlock.release()
raise
def connectionwait(self): def connectionwait(self):
"""Waits until there is a connection available. Note that between """Waits until there is a connection available. Note that between

View File

@ -17,7 +17,9 @@
# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
from offlineimap import CustomConfig from offlineimap import CustomConfig
from offlineimap.ui import UIBase
import os.path import os.path
import sys
def LoadRepository(name, account, reqtype): def LoadRepository(name, account, reqtype):
from offlineimap.repository.Gmail import GmailRepository from offlineimap.repository.Gmail import GmailRepository
@ -152,9 +154,14 @@ class BaseRepository(CustomConfig.ConfigHelperMixin):
for key in srchash.keys(): for key in srchash.keys():
if not key in desthash: if not key in desthash:
dest.makefolder(key) try:
for copyfolder in copyfolders: dest.makefolder(key)
copyfolder.makefolder(key.replace(dest.getsep(), copyfolder.getsep())) for copyfolder in copyfolders:
copyfolder.makefolder(key.replace(dest.getsep(), copyfolder.getsep()))
except:
UIBase.getglobalui().warn("ERROR Attempting to make folder " \
+ key + ":" +str(sys.exc_info()[1]))
# #
# Find deleted folders. # Find deleted folders.