Re: [MirageOS-devel] Error handling in Mirage - request for comments!

[Thomas Leonard <talex5@xxxxxxxxx>]

On 11 February 2015 at 16:41, Leo White <lpw25@xxxxxxxxx> wrote:
>>
>> I've had a go at writing this proposal up ("system 5"):
>>
>>   
>> https://github.com/talex5/mirage-www/blob/errors/tmpl/wiki/error-handling.md
>>
>> I think someone who wants to advocate this style should check it /
>> change it. It probably needs some worked examples showing it being
>> used too.
>>
>
> The interface given for "system 5" is not comparable for the one given
> for "system 4", giving a false sense of verbosity to the "system 5"
> example. The equivalent to the "system 4" example, would just be:

Good point. I've updated the system 4 example to include the exception
definitions:

https://github.com/talex5/mirage-www/commit/97b149b9e04e9127920961d25f5082e36c124bc4

>   type error
>
>   val pp_error : formatter -> error -> string
>
>   val read :
>     t -> int64 -> page_aligned_buffer list ->
>     (int, error) result
>
> Constructors of `error` should only be exposed if they are intended to
> be matched on. So if `read` has a kind of error ``Foo` which was
> intended to be matched on then the "system 5" approach would be:
>
>   type error = private [> `Foo]
>
>   val pp_error : formatter -> error -> string
>
>   val read :
>     t -> int64 -> page_aligned_buffer list ->
>     (int, error) result
>
> whilst "system 4" would probably look like:
>
>   exception Foo
>
>   val read :
>     t -> int64 -> page_aligned_buffer list -> int
>
> The splitting of `error` up into `write_error` and `read_error` is only
> needed if you have an interface with different sets of matchable
> errors. So if you had a `read` function with a matchable error ``Foo`
> and a `write` function with matchable error ``Bar`, then you would get
> the following:
>
>   type read_error = private [> `Foo]
>   type write_error = private [> `Bar]
>
>   type error
>   val pp_error : formatter -> error -> string
>
>   val write_error : write_error -> error
>   val read_error : read_error -> error
>
>   val read :
>     t -> int64 -> page_aligned_buffer list ->
>     (int, read_error) result
>
>   val write :
>     t -> int64 -> page_aligned_buffer list ->
>     (int, write_error) result
>
> whilst "system 4" would probably look like:
>
>   exception Foo
>
>   exception Bar
>
>   val read :
>     t -> int64 -> page_aligned_buffer list -> int
>
>   val write :
>     t -> int64 -> page_aligned_buffer list -> int
>
> A key benefit of "system 5" is that the signature contains all the
> interesting information about `read` and `write`. It clearly states that
> they are expected to fail sometimes, and that the only errors on which it is
> reasonable to behave specially are a `Foo` returned by `read` or a `Bar`
> returned by `write`. `System 4` does not convey any of this information.

Yes, but the question is whether this information is worth the
overhead. In particular:

- We must assume that some implementation of a Mirage module type may
produce an error (they're all about I/O, after all), so annotating
every function to say this doesn't provide any extra information.

- Knowing that e.g. `read` won't return `Is_read_only` is useful when
doing exhaustive matching, but we can't do that anyway for generic
module types.

>>> Some nice combinators should be provided for using ('a, 'b) Result.t and
>>> ('a, 'b) Result.t Lwt. and for lifting an ('a, Foo.error) Result.t into
>>> an ('a, Bar.error) Result.t.
>>>
>>> Exceptions that escape their intended scope, should always be treated as
>>> a programming error.
>>
>> Yes, by definition. But what should the "intended scope" be, and how
>> do we ensure the producer and consumers of the exception agree?
>
> Exceptions should basically never delibrately cross module
> boundaries. They should certainly not be part of the exposed API of a
> library. By "exceptions that escape their scope" I essentially meant
> exceptions used in third-party APIs (e.g. Not_found produced by
> List.find), these should always be caught as early as possible and
> handled appropriately.
>
> Occasionally it is reasonable for an exception like "Invalid_argument"
> to be part of a library API, where the exception is not intended to be
> caught but merely to indicate that there has been a programmer error.
>
> It is also reasonable to use exceptions locally for control-flow, but
> again these should not escape into the visible API.

That's one approach, but others are possible (though I think everyone
agrees that Not_found shouldn't be an exception). Personally, I'd say
anything you wouldn't feel comfortable displaying to the user
shouldn't be an exception.


-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel