Re: Some comments about GVFS

[Havoc Pennington <hp redhat com>]

Hi,

Carl Worth wrote:
> On Thu, 3 May 2007 11:11:39 +0200, "Benjamin Otte" wrote:
> >                    I much prefer the cairo model, where basically the
> > object keeps its own GError and has a function like cairo_status [3]
> > that gives you the last error that occured.
>
> It's worth pointing out an additional aspect of the cairo
> error-handling model: The object becomes entirely inert as soon as an
> error occurs within it. That is, all methods of the object first check
> the status and return immediately on error.

Yeah, I think this keeps the Cairo model from being "fully general" - it 
only works for some types of operations or objects. Another thing about 
the Cairo model is that it's either weird or possibly broken if an 
object is used by multiple threads, which could arise in the current 
gvfs presumably.

Another thing that's needed for gvfs for sure but not for Cairo perhaps 
is the error message. A strerror()/errno type of thing is just not 
sufficient to give good errors from a system with pluggable backends. 
Even in Cairo, functions like cairo_surface_write_to_png return pretty 
unhelpful info on failure. Failure is uncommon, and the info would 
generally only be helpful to programmers not users anyway (gdk-pixbuf 
returns stuff like "Value for PNG text chunk cannot be converted to 
8859-1"), but nonetheless in a lot of cases I think a detailed error can 
be better than a strerror-style string. Sometimes the error will be 
user-helpful, e.g. "You aren't allowed to save to directory Foo"

Returning *helpful* errors, even if only to programmers and techies, 
also frequently depends on context - meaning, the error can be more 
helpful if it's checked right away after every call. errno obviously is 
an extreme example since it's garbage if not checked immediately. But 
say you do a series of gvfs operations then get a "no permissions" 
error, if the error message says which operation lacked permissions, 
that is much more helpful. strerror(EPERM) is not as good as "You aren't 
allowed to save to directory Foo"

Finally, Cairo of course mixes programmer errors (g_return_if_fail in 
GLib-world) with should-be-reported errors (GError in GLib-world) with 
unavoidable-and-should-be-ignored errors (just get ignored in 
GLib-world). See GError docs for how these are separated in GLib:
http://developer.gnome.org/doc/API/2.2/glib/glib-Error-Reporting.html

For me when using Cairo drawing I would say should-be-reported errors 
aren't really possible. When the problem is that I passed nonsense to a 
Cairo API, I would prefer some verbose warning to the silent failure 
plus an error code I have to figure out the origin of. When the problem 
is something unavoidable (say not enough memory or whatever) then I just 
don't care, I'm not going to check the error code for this or in any way 
recover. So in neither case here do I find the error codes useful.

I guess for some surfaces there might be a recoverable error I can 
imagine caring about, but for painting widgets in X, it just hasn't 
happened. The Cairo error reporting is something I simply don't use in 
connection with any drawing code, except in temporary code to figure out 
how I misused the API, where in GTK I'd instead have seen a warning 
right away.

When the programmer _should_ check the error, e.g. 
cairo_surface_write_to_png, I think GError-style reporting is ideal, and 
the return code as write_to_png has is OK and does the job and is pretty 
much equivalent to GError except there's no memory management (plus) and 
no error message (minus).

So to sum up:
 - cairo model is most usable when it can be correct to ignore errors,
   which is particularly common for programming errors
   (GLib would use return_if_fail/assert) or unavoidable runtime errors
   that are best handled by simply ignoring them
 - cairo model should perhaps be extended to support an error message
   for more general applications (since in the drawing case the idea
   is to almost always ignore errors, there's no point, but for file
   and network ops, there may well be)
 - cairo model raises issues when the error state is shared by
   multiple threads accessing one object
 - when it's really probably wrong to not check errors (e.g. when
   saving a file), a GError-type model with a function arg you must
   consider, or a return value with warn_unused_result as Carl mentions,
   is better than the cairo_t store-error-on-object model IMO
 - I personally believe programmer errors should get the return_if_fail
   or assert type treatment and not be runtime-detectable, because
   a nice warning is more helpful to the programmer or in bug reports,
   and runtime detecting these is just wrong anyway - fix the bug, don't
   recover from the bug by adding bug-handling code

For those reasons I don't agree at all with Benjamin that GError is the 
"old" approach, I think it is still the right approach *for errors that 
should generally be handled or recovered from in some way*. For 
programming errors, or errors that effectively won't be handled or can't 
be handled, GError isn't right (but is also documented as inappropriate, 
and not used by GLib or GTK+ in those cases). For example, none of the 
gdk_draw_* API had GError *or* cairo-style error handling.

It's very very important IMO to distinguish the three types of errors 
(programming, recoverable, and unavoidable-and-ignorable) when designing 
any API in this area.

Havoc