[PATCHv2 2/4] Re: Implement ui.error() and output them at end of offlinimap sync

[Nicolas Sebrecht]

On Wed, Jun 15, 2011 at 10:18:13AM +0200, Sebastian Spaeth wrote:
> 
> Output all raised Exceptions error strings to the error log. If we are
> in debug mode, we also output the traceback of the exception.
> 
> Save all exceptions that occur during the run time to a Queue and output
> them all again when the offlineimap sync is finished.
> 
> Signed-off-by: Sebastian Spaeth <Sebastian at SSpaeth.de>
> ---
> v2 fixes an erronous print_tb to a format_tb. Otherwise identical.
> 
>  offlineimap/ui/UIBase.py |   53 +++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/offlineimap/ui/UIBase.py b/offlineimap/ui/UIBase.py
> index 5524579..13ecb20 100644
> --- a/offlineimap/ui/UIBase.py
> +++ b/offlineimap/ui/UIBase.py

<...>

> @@ -82,6 +85,38 @@ class UIBase:
>          else:
>              s._msg("WARNING: " + msg)
>  
> +    def error(self, exc, exc_traceback = None, msg= None):

Whitespaces here sucks. It should be:

  def f(arg1=optional, arg2=optional)

> +        """Log a message at severity level ERROR
> +
> +        Log 'Exception' to error log, possibly prepended by a preceding
> +        error "msg" detailing at what point the error occurred.
> +
> +        In debug mode, we also output the full traceback that occurred
> +        if one has been passed in via sys.exc_traceback.
> +
> +        Also save the Exception to a stack that can be output at the end
> +        of the sync run when offlineiamp exits. It is recommended to
> +        always pass in exceptions if possible, so we can give the user
> +        the best debugging info.

I can't see anything about rendering tracebacks per-threads. It might be
welcome to prevent from mixing tracebacks each other.

> +        One example of such a call might be:
> +
> +           ui.error(exc, sys.exc_traceback, msg="While syncing Folder %s in "
> +                                                "repo %s")
> +        """
> +        if msg:
> +            self._msg("ERROR: %s\n  %s" % (msg, exc))
> +        else:
> +            self._msg("ERROR: %s" % exc)
> +
> +        if not self.debuglist:
> +            # only output tracebacks in debug mode
> +            exc_traceback = None
> +        # push exc on the queue for later output
> +        self.exc_queue.put((msg, exc, exc_traceback))
> +        if exc_traceback:
> +            self._msg(traceback.format_tb(exc_traceback))
> +
>      def registerthread(s, account):
>          """Provides a hint to UIs about which account this particular
>          thread is processing."""
> @@ -315,9 +350,21 @@ class UIBase:
>      def mainException(s):
>          s._msg(s.getMainExceptionString())
>  
> -    def terminate(s, exitstatus = 0, errortitle = None, errormsg = None):
> +    def terminate(self, exitstatus = 0, errortitle = None, errormsg = None):
>          """Called to terminate the application."""
> -        if errormsg <> None:
> +        #print any exceptions that have occurred over the run
> +        if not self.exc_queue.empty():
> +           self._msg("\nERROR: Exceptions occurred during the run!")
> +        while not self.exc_queue.empty():
> +            msg, exc, exc_traceback = self.exc_queue.get()
> +            if msg:
> +                self._msg("ERROR: %s\n  %s" % (msg, exc))
> +            else:
> +                self._msg("ERROR: %s" % (exc))
> +            if exc_traceback:
> +                self._msg("\nTraceback:\n%s" %"".join(
> +                        traceback.format_tb(exc_traceback)))
> +        if errormsg != None:

Why not use the pythonic:

  if errormsg:
    # statements

>              if errortitle <> None:

Hugh! Never paid attention to this "<>" before...

>                  sys.stderr.write('ERROR: %s\n\n%s\n'%(errortitle, errormsg))
>              else:

-- 
Nicolas Sebrecht