[PATCH] Re: IMAP: improve quickchanged() performance

[Nicolas Sebrecht]

On Fri, Apr 15, 2011 at 09:18:19AM +0200, Sebastian Spaeth wrote:
> On Thu, 14 Apr 2011 19:46:58 +0200, Nicolas Sebrecht wrote:
> > On Tue, Apr 12, 2011 at 07:32:11PM +0200, Sebastian Spaeth wrote:
> > > On Tue, 12 Apr 2011 19:17:06 +0200, Nicolas Sebrecht <nicolas.s-dev at laposte.net> wrote:

> > > > Can't we try to fetch a deleted UID, then? How do we handle such
> > > > case?
> > IOW if we don't check for changes on remote while syncing, I wonder if
> > we won't do insane things like trying to fetch a mail which was just
> > deleted (and possibly crash).

<...>

> > The good question is: do we handle the case nicely?
> 
> I'd say this calls for a code audit, with this specific issue in mind.
> 
> What I think would make sense, is if we introduced an custom exception,
> e.g. an OfflineImap.Error(). This exception would have a "reason" string
> that we can log and print, it would have an "errno", so we can identify
> what went wrong, and it would have a "abort_level" (or similar), that
> would indicate at which level we should fail.
> 
> e.g. we could have:
> abort_level = MESSAGE:
>    Just abort this single message sync but continue with the rest of the
>    folder. This would e.g. be issued when a single message has been
>    found faulty when uploading (e.g. too big), or when a message that we
>    want to copy is missing. OfflineImap would not abort when it comes
>    across something like this.
> 
> abort_level = FOLDER:
>    Abort the folder sync, but continue with the rest of the folders in
>    this account. This would be raised if e.g. the folder is read-only,
>    or cannot be found on the server (e.g. as it has been deleted)
> 
> abort_level = ACCOUNT:
>    Abort the whole account syncing. E.g. when the mail server is down,
>    the password wrong, or the network has some error. If this error
>    occurs, we would still try to sync the other accounts, before exiting
>    with an error message.
> 
> Whenever such an exception would be raise, we store it in a stack, and
> on program exit, we output all exceptions that had been queued, so the
> user can see what has gone wrong during a sync and can react
> accordingly.
> 
> This is how I think we could deal sensibly with error condidtions, but
> others might have be better ideas, so I am open for feedback.

The idea looks good to me. I think this is a thing that we miss most: we
are currently BAD at handling issues nicely.

What we could be missing with the proposed approach are:

1) User configuration options.

  We would really gain to let users define their own options such as
  "abort_level mail on A", "abort_level folder on B". The "abort_level"
  name is not very sexy.

2) Strategies to apply on error.

  "abort" only looks not enough to me. We certainly would like to retry
  n times in some cases.


All that said, I guess it's better to ALWAYS abort on just upper level.
What will be done on raised errors is a decision for the catcher which
should be based on the user defined policy.

-- 
Nicolas Sebrecht