Date: Sun, 29 Jan 2012 20:54:11 -0500
From: Austin Clements <amdragon@MIT.EDU>
To: Justus Winter <4winter@informatik.uni-hamburg.de>
Subject: Re: error handling and stderr
Message-ID: <20120130015411.GL17991@mit.edu>
References: <20120129180124.23430.33656@thinkbox.jade-hamburg.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20120129180124.23430.33656@thinkbox.jade-hamburg.de>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch mailing list <notmuch@notmuchmail.org>
Precedence: list

Quoth Justus Winter on Jan 29 at  7:01 pm:
> Hi notmuch developers,
> 
> currently there is no way to determine why opening a database using
> notmuch_database_open fails. I suppose that this is a problem for all
> functions returning a pointer, but it is especially problematic for
> this function since one cannot distinguish between a temporary failure
> (someone else opened the db read/write) and a configuration problem
> (wrong path).
> 
> This is a real problem for providing sensible feedback to the user.

I'm in the process of fixing the temporary failure, which I suspect is
the biggest problem with the lack of status return, but you're right
that a function as involved as notmuch_database_open really needs a
way to distinguish failure types.  Carl even left a note about this in
notmuch.h in 2009.

We could fix this without breaking the API by introducing a second
variant of notmuch_database_open.  I'm actually not a fan of this; I
think notmuch is young enough and the library has few enough consumers
that it's pointless to lug around compatibility code when the
alternative is distinctly better.  If we're really concerned about
compatibility, I think we should move to using versioned symbols.

> And writing an error message to stderr is a sensible thing to do if it
> is done in a command line utility but doing so within a library is a
> severe problem for any curses frontends and is generally considered a
> bad practice.

This is an unfortunate consequence of C's terrible error handling.  I
don't know how to do this effectively if we want error messages to
accompany errors (which account for almost, though not quite all
prints to stderr from libnotmuch).  We could maintain a "last error
message" on the database object, but then we have to be sure to update
this on any error.

We could simply provide a log hook.  It's not very satisfying, but it
may address the immediate problem of corrupting curses displays.

What I find most frustrating about the current error handling approach
is that it's always unclear whether a caller should report an error
message or if the callee has already taken care of it (and generally
with greater detail).  Log hooks don't particularly help with this.

> For the record, libabcs README[0] agrees with me on both aspects.
> 
> Since fixing this means breaking the api we need to discuss this
> thoroughly and identify all the functions involved.
> 
> Users of the python bindings won't be affected by an api change like
> this since we're converting any error code to exceptions anyway.