Extended error codes
paurkedal opened this issue · 6 comments
I would like to add the extended error codes to the API. It seems straight forward except for how to define the error type, where I could use a second opinion. The options I consider are:
- Create a flat non-polymorphic variant, like the current error codes.
- Create a nested error type where the root type contains a constructors for error category which takes as an argument a detailed error code of a case-specific type.
- Map them to a polymorphic variant and create subtypes for each top-level category.
With option 1 we can't match whole categories directly. It will be possibly to match first on the normal error code, then on the detailed code where needed. The latter would not provide exhaustiveness checks within the category, which I think is its main limitation.
Option 2 would solve that, at the expense of having constructors with arguments (and breaking slightly with the style of the current API).
Option 3 would also solve it, at the expense of breaking with the style of the current API.
[Sorry for the initially empty issue, I hit enter by accident.]
Sorry, I must have missed this issue for a while, probably because GitHub only sent the initially empty email.
It might make sense to support easy matching on error categories. There may be users who are not well-acquainted with matching on polymorphic variant types so option 2 may be easier for them than option 3. Option 1 would seem awkward to use.
Please feel free to submit a PR with extended error codes.
Before I put too much engineering into it, how about something like:
module Erc : sig
type ok =
| OTHER__match_only_as_wildcard
| LOAD_PERMANENTLY
| SYMLINK
type error =
| OTHER__match_only_as_wildcard
| MISSING_COLLSEQ
| RETRY
| SNAPSHOT
type internal =
| OTHER__match_only_as_wildcard
type perm =
| OTHER__match_only_as_wildcard
(* ... *)
type t =
| OK of ok
| ERROR of error
| INTERNAL of internal
| PERM of perm
| (* ... *)
The rational for OTHER__match_only_as_wildcard
is:
- I presume the cases with detailed codes can also return the generic variant, adding a case for it avoids using the
option
type which would require repeating theSome
constructors when matching with a singlematch
. - Suggesting to the end user to match this constructor with a wildcard avoids breaking code by future version where more cases are added.
- Similarly some main cases don't have detailed cases, but to limit future breakage this allows use to add single-case variants without using the new empty variant extension, which we would need of using an
option
type.
We can find another name for it though (UNKNOWN__don't_match
?). We could also prefix constructors like done in the C macros or wrap them in sub-modules. The clashes are CONVPATH
, NOMEM
, RECOVERY
, ROLLBACK
, SNAPSHOT
, SYMLINK
, VTAB
(all present 2 times). Recent OCaml compilers can disambiguate, though:
utop #
type nomem =
| OTHER__match_only_as_wildcard
type ioerr =
| NOMEM
type t =
| NOMEM of nomem
| IOERR of ioerr
;;
utop # function IOERR NOMEM -> true | NOMEM _ -> false;;
- : t -> bool = <fun>
The don't-match policy means we can't expose the error code, though, as done for Rc
. It may be useful to expose the error codes also for known cases, either though a separate connection method or by adding Erc.to_int
.
An alternative design would be to use polymorphic variants, maybe private [> ...]
types.
I dropped the obscured constructor names in favour of UNREFINED
. All unknown ones ends up as common UNKNOWN of int
, though we have an opportunity to add UNKNOWN of int
to each of the error categories to offer classification even in such cases.
Thanks!