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

[Christophe Troestler <Christophe.Troestler@xxxxxxxxxxx>]

On Mon, 2 Feb 2015 12:30:39 +0000, Thomas Leonard wrote:
> 
> On 2 February 2015 at 00:45, Christophe Troestler
> <Christophe.Troestler@xxxxxxxxxxx> wrote:
> > On Sat, 31 Jan 2015 21:07:22 +0000, Thomas Leonard wrote:
> >>
> >> [â] But the real problem here is that by making exceptions fatal we
> >> turn every exception used in existing OCaml code into a security
> >> vulnerability. [â]
> >
> > Since OCaml supports exceptions, I think there is no doubt that we
> > must handle them effectively â not just wipe them under the carpet and
> > let them abort the program/unikernel.  Ideally Lwt should deal
> > sensibly with exceptions (and the discussion seems to imply there are
> > areas of improvement â but it is unclear to me how you can do that
> > efficiently without using a try block in all binds).
> 
> Lwt already puts a try block in all (sleeping) binds, so this is
> handled for you:
> 
> let bind t f =
>   let t = repr t in
>   match t.state with
>     | Return v ->
>         f v
>     | Fail _ as state ->
>         thread { state }
>     | Sleep sleeper ->
>         let res = temp t in
>         let data = !current_data in
>         add_immutable_waiter sleeper
>           (function
>              | Return v -> current_data := data; connect res (try f v
> with exn -> fail exn)
>              | Fail _ as state -> fast_connect res state
>              | _ -> assert false);
>         res

Thanks for checking.  At first, the difference of treatment between
the two âf vâ is a little odd.  Do you happen to know the reason for
it?

> > There are however errors that you want to mandate that they are dealt
> > with but not necessarily at the calling point.  A typical example to
> > me is âreadâ because it is often much "higher" in the program that an
> > appropriate response to a failing read can be made â this is important
> > for libraries that will post-process âreadâs and will have no idea
> > what to do if âreadâ fails.
> 
> Yes, this is a case where you might want an Or_error.t threaded
> through everything. Or, it might be easier to wrap the whole thing
> so that any error code returned by the read is thrown and then
> caught again.

The problem with Or_error.t is that you have to write tedious code to
propagate it at every bind.  If we adopt this, an bind infix operator
that propagates the error automatically is really needed (trying to
catch with today's messages, I see that >>=? is already used for that
purpose).  Also, with Or_error.t (or exceptions), there is no way to
ensure that the error is eventually acted upon so why not to simply
use âfailâ?

> The problem we have in Mirage is that our types are abstract. For
> example, if you have a functor
> 
>   module FS (B : BLOCK)  = ...
> 
> then the author of the FS functor doesn't know exactly what errors the
> concrete B might throw. For example, if B is a remote disk then it
> might throw network errors. So ensuring that callers enumerate all
> errors isn't very helpful in this case.
> 
> You can give FS an error code of [`Block_error of B.error] and just
> propagate it to the caller, which is what we do now. That seems quite
> clever, but I'm struggling to imagine a case where it's actually
> useful.

That's what I was going to suggest...  Declaring several `Block_error
if necessary allows to act on "classes" of errors â which is all one
can hope for given the abstraction.

> It's probably worth compiling a list of examples where Mirage code
> currently matches on specific returned Error codes and takes specific
> actions. My guess is there aren't enough to justify the complexity
> though.

That would be nice.  I don't have much familiarity with a lot of parts
of Mirage so I may be missing use cases.  As I understand the
discussion, Mirage code should be revisited, not only to see how
things are done now but also what the problems are (with the current
scheme) and what properties of the error handling we would like in
these cases.

> [...] in every one of these cases where we match on an `Error, we just
> propagate it (i.e. the default behaviour of an exception would have
> been correct).
> 
> Here are some examples:
> 
> | `Error (`Invalid_console x) -> fail (Failure (Printf.sprintf 
> "Invalid_console %s" x))
> | `Error (`Block_device e) -> fail (Failure (Fs.string_of_block_error e))
> | `Error (KV.Unknown_key key) -> fail (Invalid_argument key)
> | `Error (`Unknown msg) -> Lwt.fail (Failure msg)

I don't understand the point of this type of code.  Why wasn't âErrorâ
declared as an exception (possibly with an exception printer) and
âfailâ directly invoked on it?  This also highlights that people shy
away from having to handle even a simple `Error to the point that it
seems preferable to degenerate it as a string!

> The only case where we do something interesting is `Error (Unknown_key
> _), where we note that it was a read:
> 
> | `Error (TMPL.Unknown_key _) -> fail (Failure ("read " ^ name))
> 
> However, Unknown_key wouldn't be an exception under any proposed scheme anyway

In the previous discussion, I had the impression that having a scheme
to propagate errors seamlessly while forcing certain (classes of)
errors to be handled before they reach the main loop was important.
If we do not care about this, what is wrong with the âfailâ mechanism
of Lwt?

Best,
C.

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