Properly re-raise exception to save original tracebacks

We usually mutate some exceptions to OfflineImapError() and it is
a whole lot better if such exception will show up with the original
traceback, so all valid occurrences of such mutations were transformed
to the 3-tuple form of "raise".  Had also added coding guidelines
document where this re-raise strategy is documented.

Signed-off-by: Eygene Ryabinkin <rea@codelabs.ru>

docs/CodingGuidelines.rst

@@ -0,0 +1,102 @@
+.. -*- coding: utf-8 -*-
+.. _OfflineIMAP: https://github.com/OfflineIMAP/offlineimap
+.. _OLI_git_repo: git://github.com/OfflineIMAP/offlineimap.git
+
+=================================
+Coding guidelines for OfflineIMAP
+=================================
+
+.. contents::
+.. .. sectnum::
+
+This document contains assorted guidelines for programmers that want
+to hack OfflineIMAP.
+
+
+------------------
+Exception handling
+------------------
+
+OfflineIMAP on many occasions re-raises various exceptions and often
+changes exception type to `OfflineImapError`.  This is not a problem
+per se, but you must always remember that we need to preserve original
+tracebacks.  This is not hard if you follow these simple rules.
+
+For re-raising original exceptions, just use::
+
+  raise
+
+from inside your exception handling code.
+
+If you need to change exception type, or its argument, or whatever,
+use this three-argument form::
+
+  raise YourExceptionClass(argum, ents), None, sys.exc_info()[2]
+
+In this form, you're creating an instance of new exception, so ``raise``
+will deduce its ``type`` and ``value`` parameters from the first argument,
+thus the second expression passed to ``raise`` is always ``None``.
+And the third one is the traceback object obtained from the thread-safe
+``exc_info()`` function.
+
+In fact, if you hadn't already imported the whole ``sys`` module, it will
+be better to import just ``exc_info()``::
+
+  from sys import exc_info
+
+and raise like this::
+
+  raise YourExceptionClass(argum, ents), None, exc_info()[2]
+
+since this is the historically-preferred style in the OfflineIMAP code.
+.. -*- coding: utf-8 -*-
+.. _OfflineIMAP: https://github.com/OfflineIMAP/offlineimap
+.. _OLI_git_repo: git://github.com/OfflineIMAP/offlineimap.git
+
+=================================
+Coding guidelines for OfflineIMAP
+=================================
+
+.. contents::
+.. .. sectnum::
+
+This document contains assorted guidelines for programmers that want
+to hack OfflineIMAP.
+
+
+------------------
+Exception handling
+------------------
+
+OfflineIMAP on many occasions re-raises various exceptions and often
+changes exception type to `OfflineImapError`.  This is not a problem
+per se, but you must always remember that we need to preserve original
+tracebacks.  This is not hard if you follow these simple rules.
+
+For re-raising original exceptions, just use::
+
+  raise
+
+from inside your exception handling code.
+
+If you need to change exception type, or its argument, or whatever,
+use this three-argument form::
+
+  raise YourExceptionClass(argum, ents), None, sys.exc_info()[2]
+
+In this form, you're creating an instance of new exception, so ``raise``
+will deduce its ``type`` and ``value`` parameters from the first argument,
+thus the second expression passed to ``raise`` is always ``None``.
+And the third one is the traceback object obtained from the thread-safe
+``exc_info()`` function.
+
+In fact, if you hadn't already imported the whole ``sys`` module, it will
+be better to import just ``exc_info()``::
+
+  from sys import exc_info
+
+and raise like this::
+
+  raise YourExceptionClass(argum, ents), None, exc_info()[2]
+
+since this is the historically-preferred style in the OfflineIMAP code.

docs/doc-src/CodingGuidelines.rst

@@ -0,0 +1 @@
+../CodingGuidelines.rst

offlineimap/CustomConfig.py

@@ -16,6 +16,7 @@
 
 import os
 import re
+from sys import exc_info
 
 try:
     from ConfigParser import SafeConfigParser, Error
@@ -75,7 +76,7 @@ def getlist(self, section, option, separator_re):
             return re.split(separator_re, val)
         except re.error as e:
             raise Error("Bad split regexp '%s': %s" % \
-              (separator_re, e))
+              (separator_re, e)), None, exc_info()[2]
 
     def getdefaultlist(self, section, option, default, separator_re):
         """Same as getlist, but returns the value of `default`

offlineimap/accounts.py

@@ -210,7 +210,8 @@ def __lock(self):
             self._lockfd.close()
             raise OfflineImapError("Could not lock account %s. Is another "
                                    "instance using this account?" % self,
-                                   OfflineImapError.ERROR.REPO)
+                                   OfflineImapError.ERROR.REPO), \
+                  None, exc_info()[2]
 
     def _unlock(self):
         """Unlock the account, deleting the lock file"""

offlineimap/folder/Gmail.py

@@ -17,6 +17,7 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
 import re
+from sys import exc_info
 
 from offlineimap import imaputil, OfflineImapError
 from offlineimap import imaplibutil
@@ -144,7 +145,8 @@ def cachemessagelist(self):
                 raise OfflineImapError("FETCHING UIDs in folder [%s]%s failed. " % \
                   (self.getrepository(), self) + \
                   "Server responded '[%s] %s'" % \
-                  (res_type, response), OfflineImapError.ERROR.FOLDER)
+                  (res_type, response), OfflineImapError.ERROR.FOLDER), \
+                  None, exc_info()[2]
         finally:
             self.imapserver.releaseconnection(imapobj)
 

offlineimap/folder/GmailMaildir.py

@@ -17,6 +17,7 @@
 
 
 import os
+from sys import exc_info
 from .Maildir import MaildirFolder
 from offlineimap import OfflineImapError
 import offlineimap.accounts
@@ -170,7 +171,8 @@ def savemessagelabels(self, uid, labels, ignorelabels=set()):
             os.rename(tmppath, filepath)
         except OSError as e:
             raise OfflineImapError("Can't rename file '%s' to '%s': %s" % \
-              (tmppath, filepath, e[1]), OfflineImapError.ERROR.FOLDER)
+              (tmppath, filepath, e[1]), OfflineImapError.ERROR.FOLDER), \
+              None, exc_info()[2]
 
         if rtime != None:
             os.utime(filepath, (rtime, rtime))

offlineimap/folder/IMAP.py

@@ -594,7 +594,9 @@ def savemessage(self, uid, content, flags, rtime):
                               "repository '%s' failed (abort). Server responded: %s\n"
                               "Message content was: %s"%
                               (msg_id, self, self.getrepository(), str(e), dbg_output),
-                                               OfflineImapError.ERROR.MESSAGE)
+                                               OfflineImapError.ERROR.MESSAGE), \
+                              None, exc_info()[2]
+                    # XXX: is this still needed?
                     self.ui.error(e, exc_info()[2])
                 except imapobj.error as e: # APPEND failed
                     # If the server responds with 'BAD', append()
@@ -604,8 +606,8 @@ def savemessage(self, uid, content, flags, rtime):
                     imapobj = None
                     raise OfflineImapError("Saving msg (%s) folder '%s', repo '%s'"
                         "failed (error). Server responded: %s\nMessage content was: "
-                        "%s"% (msg_id, self, self.getrepository(), str(e), dbg_output),
-                            OfflineImapError.ERROR.MESSAGE)
+                        "%s" % (msg_id, self, self.getrepository(), str(e), dbg_output),
+                            OfflineImapError.ERROR.MESSAGE), None, exc_info()[2]
             # Checkpoint. Let it write out stuff, etc. Eg searches for
             # just uploaded messages won't work if we don't do this.
             (typ,dat) = imapobj.check()
@@ -676,6 +678,7 @@ def _fetch_from_imap(self, imapobj, uids, retry_num=1):
                 imapobj = self.imapserver.acquireconnection()
                 self.ui.error(e, exc_info()[2])
                 fails_left -= 1
+                # self.ui.error() will show the original traceback
                 if not fails_left:
                     raise e
         if data == [None] or res_type != 'OK':

offlineimap/folder/LocalStatus.py

@@ -15,6 +15,7 @@
 #    along with this program; if not, write to the Free Software
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 
+from sys import exc_info
 import os
 import threading
 
@@ -79,7 +80,7 @@ def readstatus_v1(self, fp):
                 errstr = "Corrupt line '%s' in cache file '%s'" % \
                     (line, self.filename)
                 self.ui.warn(errstr)
-                raise ValueError(errstr)
+                raise ValueError(errstr), None, exc_info()[2]
             self.messagelist[uid] = self.msglist_item_initializer(uid)
             self.messagelist[uid]['flags'] = flags
 
@@ -103,7 +104,7 @@ def readstatus(self, fp):
                 errstr = "Corrupt line '%s' in cache file '%s'" % \
                     (line, self.filename)
                 self.ui.warn(errstr)
-                raise ValueError(errstr)
+                raise ValueError(errstr), None, exc_info()[2]
             self.messagelist[uid] = self.msglist_item_initializer(uid)
             self.messagelist[uid]['flags'] = flags
             self.messagelist[uid]['mtime'] = mtime

offlineimap/folder/LocalStatusSQLite.py

@@ -16,6 +16,7 @@
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
 import os
 import re
+from sys import exc_info
 from threading import Lock
 from .Base import BaseFolder
 try:
@@ -66,7 +67,7 @@ def __init__(self, name, repository):
         except NameError:
             # sqlite import had failed
             raise UserWarning('SQLite backend chosen, but no sqlite python '
-                              'bindings available. Please install.')
+                              'bindings available. Please install.'), None, exc_info()[2]
 
         #Make sure sqlite is in multithreading SERIALIZE mode
         assert sqlite.threadsafety == 1, 'Your sqlite is not multithreading safe.'

offlineimap/folder/Maildir.py

@@ -19,6 +19,7 @@
 import time
 import re
 import os
+from sys import exc_info
 from .Base import BaseFolder
 from threading import Lock
 try:
@@ -282,7 +283,7 @@ def save_to_tmp_file(self, filename, content):
                         continue
                     severity = OfflineImapError.ERROR.MESSAGE
                     raise OfflineImapError("Unique filename %s already exists." % \
-                      filename, severity)
+                      filename, severity), None, exc_info()[2]
                 else:
                     raise
 
@@ -372,7 +373,8 @@ def savemessageflags(self, uid, flags):
             except OSError as e:
                 raise OfflineImapError("Can't rename file '%s' to '%s': %s" % (
                                        oldfilename, newfilename, e[1]),
-                                       OfflineImapError.ERROR.FOLDER)
+                                       OfflineImapError.ERROR.FOLDER), \
+                      None, exc_info()[2]
 
             self.messagelist[uid]['flags'] = flags
             self.messagelist[uid]['filename'] = newfilename

offlineimap/folder/UIDMaps.py

@@ -14,6 +14,8 @@
 #    You should have received a copy of the GNU General Public License
 #    along with this program; if not, write to the Free Software
 #    Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
+
+from sys import exc_info
 from threading import Lock
 from offlineimap import OfflineImapError
 from .IMAP import IMAPFolder
@@ -60,7 +62,7 @@ def _loadmaps(self):
                     line = line.strip()
                 except ValueError:
                     raise Exception("Corrupt line '%s' in UID mapping file '%s'"%
-                        (line, mapfilename))
+                        (line, mapfilename)), None, exc_info()[2]
                 (str1, str2) = line.split(':')
                 loc = long(str1)
                 rem = long(str2)
@@ -89,7 +91,7 @@ def _uidlist(self, mapping, items):
             raise OfflineImapError("Could not find UID for msg '{0}' (f:'{1}'."
                 " This is usually a bad thing and should be reported on the ma"
                 "iling list.".format(e.args[0], self),
-                    OfflineImapError.ERROR.MESSAGE)
+                    OfflineImapError.ERROR.MESSAGE), None, exc_info()[2]
 
     # Interface from BaseFolder
     def cachemessagelist(self):

offlineimap/imaplibutil.py

@@ -18,6 +18,7 @@
 import fcntl
 import time
 import subprocess
+from sys import exc_info
 import threading
 from hashlib import sha1
 
@@ -54,7 +55,7 @@ def select(self, mailbox='INBOX', readonly=False, force=False):
             errstr = "Server '%s' closed connection, error on SELECT '%s'. Ser"\
                 "ver said: %s" % (self.host, mailbox, e.args[0])
             severity = OfflineImapError.ERROR.FOLDER_RETRY
-            raise OfflineImapError(errstr, severity)
+            raise OfflineImapError(errstr, severity), None, exc_info()[2]
         if result[0] != 'OK':
             #in case of error, bail out with OfflineImapError
             errstr = "Error SELECTing mailbox '%s', server reply:\n%s" %\

offlineimap/imapserver.py

@@ -206,8 +206,8 @@ def __start_tls(self, imapobj):
                 imapobj.starttls()
             except imapobj.error as e:
                 raise OfflineImapError("Failed to start "
-                  "TLS connection: %s"% str(e),
-                  OfflineImapError.ERROR.REPO)
+                  "TLS connection: %s" % str(e),
+                  OfflineImapError.ERROR.REPO, None, exc_info()[2])
 
 
     ## All __authn_* procedures are helpers that do authentication.
@@ -466,7 +466,7 @@ def acquireconnection(self):
                          "'%s'. Make sure you have configured the ser"\
                          "ver name correctly and that you are online."%\
                          (self.hostname, self.repos)
-                raise OfflineImapError(reason, severity)
+                raise OfflineImapError(reason, severity), None, exc_info()[2]
 
             elif isinstance(e, SSLError) and e.errno == 1:
                 # SSL unknown protocol error
@@ -479,23 +479,23 @@ def acquireconnection(self):
                     reason = "Unknown SSL protocol connecting to host '%s' for "\
                          "repository '%s'. OpenSSL responded:\n%s"\
                          % (self.hostname, self.repos, e)
-                raise OfflineImapError(reason, severity)
+                raise OfflineImapError(reason, severity), None, exc_info()[2]
 
             elif isinstance(e, socket.error) and e.args[0] == errno.ECONNREFUSED:
                 # "Connection refused", can be a non-existing port, or an unauthorized
                 # webproxy (open WLAN?)
                 reason = "Connection to host '%s:%d' for repository '%s' was "\
                     "refused. Make sure you have the right host and port "\
                     "configured and that you are actually able to access the "\
-                    "network."% (self.hostname, self.port, self.repos)
-                raise OfflineImapError(reason, severity)
+                    "network." % (self.hostname, self.port, self.repos)
+                raise OfflineImapError(reason, severity), None, exc_info()[2]
             # Could not acquire connection to the remote;
             # socket.error(last_error) raised
             if str(e)[:24] == "can't open socket; error":
                 raise OfflineImapError("Could not connect to remote server '%s' "\
                     "for repository '%s'. Remote does not answer."
                     % (self.hostname, self.repos),
-                    OfflineImapError.ERROR.REPO)
+                    OfflineImapError.ERROR.REPO), None, exc_info()[2]
             else:
                 # re-raise all other errors
                 raise
@@ -702,7 +702,7 @@ def callback(args):
                         self.ui.error(e, exc_info()[2])
                         self.parent.releaseconnection(imapobj, True)
                     else:
-                        raise e
+                        raise
                 else:
                     success = True
             if "IDLE" in imapobj.capabilities:

offlineimap/repository/IMAP.py

@@ -103,7 +103,8 @@ def gethost(self):
             except Exception as e:
                 raise OfflineImapError("remotehosteval option for repository "\
                                        "'%s' failed:\n%s" % (self, e),
-                                       OfflineImapError.ERROR.REPO)
+                                       OfflineImapError.ERROR.REPO), \
+                      None, exc_info()[2]
             if host:
                 self._host = host
                 return self._host