base icon indicating copy to clipboard operation
base copied to clipboard

Use standard naming convention and types for things that can fail

Open pchiusano opened this issue 2 years ago • 2 comments

We have some functions that return Either Text a, others that return Either Failure a, and others that require {Exception}. I think Either Text a is right out, just replace that with Either Failure a in all cases, since the Failure can contain a Text message as well as an error type.

Maybe the ones that return Either start with try, and the ones that throw exception don't.

Often both versions are useful.

pchiusano avatar Oct 13 '22 15:10 pchiusano

https://github.com/unisonweb/unison/issues/3336 is related (opened on unisonweb/unison as opposed to unisonweb/base).

ceedubs avatar Nov 03 '22 19:11 ceedubs

@runarorama and I discussed this. We considered the question - what should the type of Nat.fromText be? (or decodeUtf8, or Map.lookup, or zlib.decompress). Some options:

Nat.fromText : Text -> Optional Nat
Nat.fromText : Text ->{Exception} Nat
Nat.fromText : Text -> Either Failure Nat
Nat.fromText : Text ->{Abort} Nat
Nat.fromText : Text ->{Throw Failure} Nat

Do we offer all these variations? Which one gets the good name? What's the naming convention for the rest? Whatever we pick, it's valuable to be consistent. One less thing to think about.

We tentatively landed on:

Nat.fromText : Text -> Optional Nat
Nat.fromTextOrRaise : Text ->{Exception} Nat

Text.decodeUtf8 : Bytes -> Optional Text
Text.decodeUtf8OrRaise : Bytes ->{Exception} Text

Map.lookup : k -> Map k v -> Optional v
Map.lookupOrRaise k -> Map k v ->{Exception} v

Which can be implemented consistently across base. The OrRaise suffix is a bit verbose, but it's at least clear. We also considered ! (as in Map.lookup!) but worried that ! is overloaded for too many things already. We liked a suffix of ? but that's not supported syntax right now.

Rationale:

  • Optional is a good choice if you're going to recover from the error. When recovering from the error, you're discarding the error's contents, so None is all you need. Map.lookup is a good example.
  • Exception is a good choice if you're going to reraise the error. But you can always Exception.catch to get the error as a value if needed.
  • Variations like {Abort} or {Throw Failure} seem special purpose and less commonly useful.

An observation is that abilities are ergonomic for propagating errors, but as errors propagate over larger scopes, the handler of the error lacks sufficient information to do anything other than reraise it or deal with it in a generic way. (Vs, say, a Map.lookup for a particular key which is handled right at the call site)

But it's actually difficult to include enough information in failures to allow larger scope handers of those failures to make fine-grained choices. For instance, in larger error scopes, the same class of error can easily occur at multiple locations, so tagging errors with a type doesn't pinpoint which site produced the error and allow a non-generic response. Also, in larger error scopes, the information that could allow a more fine-grained handling of the error won't be of a common type. (Like there may be 3 text decoding failures and two Map.lookup misses)

I feel like there's maybe some nicer abstraction waiting to be discovered to allow the user to take multiple actions that can fail, then handle or reraise or combine these failures later, on a fine-grained level if desired. Validation is maybe a special case of this. But seems like this needs design and should be worked on in a separate library, then maybe folded into base if we come up with something good.

pchiusano avatar Jun 29 '23 02:06 pchiusano