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

[Leo White <lpw25@xxxxxxxxx>]

> I don't think exception vs error code is a reliable way to divide
> these cases up. Having your FS return `Block_error indicates a serious
> problem that might require terminating the unikernel, whereas getting
> a Division_by_zero exception from your HTTP handler is likely fairly
> harmless. Whether an exception/error is serious depends more on the
> importance of the thing that raised it.
>
> Consider the case of a filesystem that reads a corrupted disk and
> throws an exception (e.g. an assert fails). This is probably the most
> extreme case where you'd want to abort. Should it terminate the
> unikernel?
>
> It depends what the disk is being used for. If it's the main hard-disk
> then possibly. If it's some removable media the user has just inserted
> then certainly not.
>
> A good principle here is that a broken component should only be able
> to harm itself. If a filesystem fails to handle a corrupted disk
> correctly then it may further corrupt that disk, but it should not
> abort the unikernel (and thus possibly corrupt other disks in the
> middle of being written).
>
> In this case, we can imagine a fail-safe FS functor that wraps all the
> calls in the FS API so that if any one of them throws an unexpected
> exception then it unplugs the underlying block device. No need to kill
> everything.
>

I think Daniel summed this up quite well. You are quite right that in
different components the correct way to deal with programmer errors
differs, but the point is that you need to keep the two kinds of errors
(dynamic/expected and programmer/unexpected) separate in order to deal
with them appropriately. Unexpected errors mean that there is a bug in
the code and in some components they should be treated extremely
seriously. They should also probably be reported differently to
encourage bug reports.

>> - The problem with using Lwt.t as your error monad is that it becomes
>>   difficult to distiunguish between synchronous things that may return
>>   errors, asynchronous things that may return errors, and asynchornous
>>   things that should not return errors. It also seems tied up with the
>>   exception mechanism, which leads to the same problem as my previous
>>   point.
>>
>> Personally, I would probably suggest that all Mirage modules/module
>> types include in their signatures:
>>
>>   type error
>>
>>   val pp_error : formatter -> error -> string
>>
>> For cases where an error can reasonably be matched on and handled
>> specially, this should be exposed in the signature:
>>
>>   type error = private [> `Foo of foo | `Bar of bar]
>
> Aha! I knew you'd know a trick to make this work!
>
> However, different functions return different sets of errors. For
> example, BLOCK.read shouldn't return `Is_read_only. Can we handle
> that?
>

When defining the error types internally you can use:

  type write_error = [`Read_only | `Foo ]
  type read_error = [`Write_only | `Bar ]
  type error = [read_error | write_error ]

However, when writing the signatures, the thing that you would naturally
like to write for this:

  type write_error = [> `Read_only ]
  type read_error = [> `Write_only ]
  type error = [> read_error | write_error ]

doesn't work because OCaml's row polymorphism is annoyingly weak (one
day I shall find time to have my way with it and fix this
problem). However, all you are really gaining from:

  type error = [> read_error | write_error ]

are free (and sometimes implicit) coercions from write_error and
read_error into error. So the following signature is a reasonable
substitute:

  type write_error = [> `Read_only ]
  type read_error = [> `Write_only ]

  type error = [> `Read_only | `Write_only ]

  val error_of_write_error : write_error -> error
  val error_of_read_error : read_error -> error

you could also provide some conveniences like:

  pp_error : formatter -> error -> unit
  pp_write_error : formatter -> write_error -> unit
  pp_read_error : formatter -> read_error -> unit

> But I think we still need an additional "exn_of_error" here because
> whether something is a "dynamic" (expected) error or a bug changes as
> the error is propagated.

Sure, sometimes an "expected" error is not actually expected. It seems
to me that:

  exception Error : 'a * (formatter -> 'a -> unit) -> exn

should do the trick, allowing you to wrap up the error and its printer
as an exception. (Note using existential variables directly in exceptions
like this requires 4.02, for earlier versions you must nest a GADT within
the exception).

Regards,

Leo

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