Complete.Org: Mailing Lists: Archives: offlineimap: August 2009:
[PATCH] This patch attempts to introduce a little more error handling -
Home

[PATCH] This patch attempts to introduce a little more error handling -

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
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.
From: mike <mike@mikelaptop.(none)>
Date: Mon, 24 Aug 2009 19:37:59 +0430

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
---
 offlineimap/accounts.py        |  185 +++++++++++++++++++++-------------------
 offlineimap/folder/Base.py     |   69 ++++++++++-----
 offlineimap/imapserver.py      |  138 +++++++++++++++++-------------
 offlineimap/repository/Base.py |   13 ++-
 4 files changed, 231 insertions(+), 174 deletions(-)

diff --git a/offlineimap/accounts.py b/offlineimap/accounts.py
index e7f1324..5975332 100644
--- a/offlineimap/accounts.py
+++ b/offlineimap/accounts.py
@@ -23,6 +23,7 @@ from subprocess import Popen, PIPE
 from threading import Event, Lock
 import os
 from Queue import Queue, Empty
+import sys
 
 class SigListener(Queue):
     def __init__(self):
@@ -181,18 +182,27 @@ class AccountSynchronizationMixin:
 
         #might need changes here to ensure that one account sync does not 
crash others...
         if not self.refreshperiod:
-            
-            self.sync(siglistener)
-            self.ui.acctdone(self.name)
+            try:
+                self.sync(siglistener)
+            except:
+                self.ui.warn("Error occured attempting to sync account " + 
self.name \
+                    + ": " + str(sys.exc_info()[1]))
+            finally:
+                self.ui.acctdone(self.name)
 
             return
 
 
         looping = 1
         while looping:
-            self.sync(siglistener)
-            looping = self.sleeper(siglistener) != 2
-            self.ui.acctdone(self.name)
+            try:
+                self.sync(siglistener)
+            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):
@@ -276,83 +286,86 @@ def syncfolder(accountname, remoterepos, remotefolder, 
localrepos,
     global mailboxes
     ui = UIBase.getglobalui()
     ui.registerthread(accountname)
-    # Load local folder.
-    localfolder = localrepos.\
-                  getfolder(remotefolder.getvisiblename().\
-                            replace(remoterepos.getsep(), localrepos.getsep()))
-    # Write the mailboxes
-    mbnames.add(accountname, localfolder.getvisiblename())
-
-    # Load status folder.
-    statusfolder = statusrepos.getfolder(remotefolder.getvisiblename().\
-                                         replace(remoterepos.getsep(),
-                                                 statusrepos.getsep()))
-    if localfolder.getuidvalidity() == None:
-        # This is a new folder, so delete the status cache to be sure
-        # we don't have a conflict.
-        statusfolder.deletemessagelist()
-        
-    statusfolder.cachemessagelist()
-
-    if quick:
-        if not localfolder.quickchanged(statusfolder) \
-               and not remotefolder.quickchanged(statusfolder):
-            ui.skippingfolder(remotefolder)
-            localrepos.restore_atime()
-            return
-
-    # Load local folder
-    ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder)
-    ui.loadmessagelist(localrepos, localfolder)
-    localfolder.cachemessagelist()
-    ui.messagelistloaded(localrepos, localfolder, 
len(localfolder.getmessagelist().keys()))
-
-    # If either the local or the status folder has messages and there is a UID
-    # validity problem, warn and abort.  If there are no messages, UW IMAPd
-    # loses UIDVALIDITY.  But we don't really need it if both local folders are
-    # empty.  So, in that case, just save it off.
-    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():
-        # Delete local copies of remote messages.  This way,
-        # if a message's flag is modified locally but it has been
-        # deleted remotely, we'll delete it locally.  Otherwise, we
-        # try to modify a deleted message's flags!  This step
-        # need only be taken if a statusfolder is present; otherwise,
-        # there is no action taken *to* the remote repository.
-
-        remotefolder.syncmessagesto_delete(localfolder, [localfolder,
-                                                         statusfolder])
-        ui.syncingmessages(localrepos, localfolder, remoterepos, remotefolder)
-        localfolder.syncmessagesto(statusfolder, [remotefolder, statusfolder])
-
-    # Synchronize remote changes.
-    ui.syncingmessages(remoterepos, remotefolder, localrepos, localfolder)
-    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()
-
+    try:
+        # Load local folder.
+        localfolder = localrepos.\
+                      getfolder(remotefolder.getvisiblename().\
+                                replace(remoterepos.getsep(), 
localrepos.getsep()))
+        # Write the mailboxes
+        mbnames.add(accountname, localfolder.getvisiblename())
+
+        # Load status folder.
+        statusfolder = statusrepos.getfolder(remotefolder.getvisiblename().\
+                                             replace(remoterepos.getsep(),
+                                                     statusrepos.getsep()))
+        if localfolder.getuidvalidity() == None:
+            # This is a new folder, so delete the status cache to be sure
+            # we don't have a conflict.
+            statusfolder.deletemessagelist()
+
+        statusfolder.cachemessagelist()
+
+        if quick:
+            if not localfolder.quickchanged(statusfolder) \
+                   and not remotefolder.quickchanged(statusfolder):
+                ui.skippingfolder(remotefolder)
+                localrepos.restore_atime()
+                return
+
+        # Load local folder
+        ui.syncingfolder(remoterepos, remotefolder, localrepos, localfolder)
+        ui.loadmessagelist(localrepos, localfolder)
+        localfolder.cachemessagelist()
+        ui.messagelistloaded(localrepos, localfolder, 
len(localfolder.getmessagelist().keys()))
+
+        # If either the local or the status folder has messages and there is a 
UID
+        # validity problem, warn and abort.  If there are no messages, UW IMAPd
+        # loses UIDVALIDITY.  But we don't really need it if both local 
folders are
+        # empty.  So, in that case, just save it off.
+        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():
+            # Delete local copies of remote messages.  This way,
+            # if a message's flag is modified locally but it has been
+            # deleted remotely, we'll delete it locally.  Otherwise, we
+            # try to modify a deleted message's flags!  This step
+            # need only be taken if a statusfolder is present; otherwise,
+            # there is no action taken *to* the remote repository.
+
+            remotefolder.syncmessagesto_delete(localfolder, [localfolder,
+                                                             statusfolder])
+            ui.syncingmessages(localrepos, localfolder, remoterepos, 
remotefolder)
+            localfolder.syncmessagesto(statusfolder, [remotefolder, 
statusfolder])
+
+        # Synchronize remote changes.
+        ui.syncingmessages(remoterepos, remotefolder, localrepos, localfolder)
+        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()
+    except:
+        ui.warn("ERROR in syncfolder for " + accountname + " folder  " + \
+        remotefolder.getvisiblename() +" : " +str(sys.exc_info()[1]))
diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py
index 9b29ab3..0f38bc3 100644
--- a/offlineimap/folder/Base.py
+++ b/offlineimap/folder/Base.py
@@ -21,6 +21,7 @@ from offlineimap import threadutil
 from offlineimap.threadutil import InstanceLimitedThread
 from offlineimap.ui import UIBase
 import os.path, re
+import sys
 
 class BaseFolder:
     def __init__(self):
@@ -266,25 +267,30 @@ class BaseFolder:
         # synced to the status cache.  This is only a problem with
         # self.getmessage().  So, don't call self.getmessage unless
         # really needed.
-        if register:
-            UIBase.getglobalui().registerthread(self.getaccountname())
-        UIBase.getglobalui().copyingmessage(uid, self, applyto)
-        message = ''
-        # If any of the destinations actually stores the message body,
-        # load it up.
-        for object in applyto:
-            if object.storesmessages():
-                message = self.getmessage(uid)
-                break
-        flags = self.getmessageflags(uid)
-        rtime = self.getmessagetime(uid)
-        for object in applyto:
-            newuid = object.savemessage(uid, message, flags, rtime)
-            if newuid > 0 and newuid != uid:
-                # Change the local uid.
-                self.savemessage(newuid, message, flags, rtime)
-                self.deletemessage(uid)
-                uid = newuid
+        try:
+            if register:
+                UIBase.getglobalui().registerthread(self.getaccountname())
+            UIBase.getglobalui().copyingmessage(uid, self, applyto)
+            message = ''
+            # If any of the destinations actually stores the message body,
+            # load it up.
+            
+            for object in applyto:
+                if object.storesmessages():
+                    message = self.getmessage(uid)
+                    break
+            flags = self.getmessageflags(uid)
+            rtime = self.getmessagetime(uid)
+            for object in applyto:
+                newuid = object.savemessage(uid, message, flags, rtime)
+                if newuid > 0 and newuid != uid:
+                    # Change the local uid.
+                    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):
@@ -384,15 +390,30 @@ class BaseFolder:
         applyto ones should be the ones that "copy" the main action."""
         if applyto == None:
             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_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.
         # (except for potential negative uids that couldn't be placed
         # 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]))
         
             
diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py
index 29e3232..d9dd6ba 100644
--- a/offlineimap/imapserver.py
+++ b/offlineimap/imapserver.py
@@ -246,72 +246,88 @@ class IMAPServer:
         
         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
-        while not success:
-            # Generate a new connection.
-            if self.tunnel:
-                UIBase.getglobalui().connecting('tunnel', self.tunnel)
-                imapobj = UsefulIMAP4_Tunnel(self.tunnel)
-                success = 1
-            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:
+        try:
+            while not success:
+                # Generate a new connection.
+                if self.tunnel:
+                    UIBase.getglobalui().connecting('tunnel', self.tunnel)
+                    imapobj = UsefulIMAP4_Tunnel(self.tunnel)
+                    success = 1
+                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 CRAM-MD5 
authentication')
+                                'Attempting GSSAPI authentication')
                             try:
-                                imapobj.authenticate('CRAM-MD5', 
self.md5handler)
+                                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)
-                        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)
-                    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)
+                        # 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
+            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):
         """Waits until there is a connection available.  Note that between
diff --git a/offlineimap/repository/Base.py b/offlineimap/repository/Base.py
index ea1fd50..45a20cc 100644
--- a/offlineimap/repository/Base.py
+++ b/offlineimap/repository/Base.py
@@ -17,7 +17,9 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
 from offlineimap import CustomConfig
+from offlineimap.ui import UIBase
 import os.path
+import sys
 
 def LoadRepository(name, account, reqtype):
     from offlineimap.repository.Gmail import GmailRepository
@@ -152,9 +154,14 @@ class BaseRepository(CustomConfig.ConfigHelperMixin):
         
         for key in srchash.keys():
             if not key in desthash:
-                dest.makefolder(key)
-                for copyfolder in copyfolders:
-                    copyfolder.makefolder(key.replace(dest.getsep(), 
copyfolder.getsep()))
+                try:
+                    dest.makefolder(key)
+                    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.
-- 
1.6.0.4





[Prev in Thread] Current Thread [Next in Thread]
  • [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., mike <=