fsharp / fsharp.github.io

F# Core Engineering Group
http://fsharp.github.io
45 stars 53 forks source link

Guidelines over exceptions vs success/failure results #28

Closed isaacabraham closed 5 years ago

isaacabraham commented 9 years ago

I'm referring to http://fsharp.org/specs/component-design-guidelines/#do-follow-the-net-guidelines-for-exceptions-including-for-f-to-f-libraries and ultimately to this thread on twitter https://twitter.com/isaac_abraham/status/596724853274710016.

What are peoples thoughts on handling "error cases" in terms of guidance? Some ideas that spring to mind that might get the discussion started: -

  1. Just follow standard .NET convention. Throw exceptions for exceptional cases (although what this is is open to debate!), otherwise return types. Benefits are that this fits nicely with the typical .NET guidelines (obviously) but does not take advantage of DUs and pattern matching, nor does it make code particularly easy to reason about.
  2. Return back some form of success / failure and pattern match over it. Have some form of combinator that converts an exception into a failure case, plus in conjunction with some built-in (or quickly created) active patterns you can easily match over Success or Exception (or Failure).
  3. As above but use some form of computation expression e.g. Chessie in order to avoid the nesting of match clauses that typically entails.

Just to mix things up, playing as devil's advocate here I would ask - if using something like 3, the question then perhaps becomes "what are we gaining over using exceptions if a computation expression is being used as a form of premature exit, just like an exception is"?

ReedCopsey commented 9 years ago

Note that my suggestion for using exceptions was specifically for a case where interop with C# (or any other non-F# language) is required, in order to match standard Framework Design Guidelines.

If the consumer will most likely be using F#, then I would prefer using DUs and pattern matching, in whatever form makes the most sense.

baronfel commented 9 years ago

Reed's point echoes how I use my projects at work. Most of them are (my attempt at) idiomatic F# for as far as I can, and then there's usually a .Compatibility namespace that smooths over the rough edges for constructing DU cases and mapping between the various collections.

tpetricek commented 9 years ago

I think there is a very fine line between unexpected "error cases" and expected "invalid cases".

Expected invalid cases For example, the railway oriented programming and the Chessie project are not (in my opinion) about error cases. They are about input validation - and it is not unexpected that the input will be invalid. The Chessie project makes it easy to collect a list of (expected) validation errors together with a final result. This is a case where using DUs makes a lot of sense because you have more complicated logic than with plain exceptions (accumulating validation messages) and the result is really "extended result" rather than just error.

Unexpected error cases On the other hand, if you are handling exceptional cases (that should not normally happen), then I don't see a reason for not using exceptions. That is, if the control flow is "whenever something goes wrong, jump to the first recovery point", then it makes a lot more sense to use exceptions - they have nice language support (do not require funky operators or additional boilerplate) and they have been designed for this case.

Summary So, I think that the wording in the guidelines is actually still valid - it says that you should use exceptions in unexpected exceptional cases - and I think this is a good advice. In fact, the very next sentence says you should use "None option value" for normal control flow - but this could be extended by adding "or custom discriminated union".

Bellarmine-Head commented 9 years ago

1) I'm surprised that the guidelines recommend throwing exceptions for F#-to-F# libraries. For F#-to-C#, yes. But for F#-to-F# I tend to follow Scott Wlaschin's guidance here (scroll down to "The error-code-based approach"; I like the quote from Raymond Chen).

2) On Scott's exceptions page it says that "failwith throws a generic System.Exception". And the guidelines suggest the same thing: "Do not throw System.Exception when it will escape to user code. This includes avoiding the use of failwith [...]". But in fact it is not so. I know from experience that it throws something else, and MSDN says that it's Microsoft.FSharp.Core.FailureException (a good reason not to use failwith in F#-to-C# scenario).

3) In fact, the very next sentence says you should use "None option value" for normal control flow - but this could be extended by adding "or custom discriminated union". Yes, agreed. And in addition, one of the Core.Choice types. For Railway Oriented Programming, I like to use Choice2Of2 for the error condition + associated data.

dsyme commented 9 years ago

Re "failwith" - MSDN is wrong - it throws System.Exception. That should be fixed.

Bellarmine-Head commented 9 years ago

@dsyme: You're right. Testing in VS2012 and 2013 with a console app, failwith throws System.Exception in both cases. And yet I could have sworn that I've seen it throw something else on occasion. Weird. Probably my faulty memory. Anyway, MSDN is definitely wrong.

Bellarmine-Head commented 9 years ago

Reported via submitting feedback on the MSDN page itself:-

As pointed out by Don Syme, "failwith" throws System.Exception, not Microsoft.FSharp.Core.FailureException.

References to the Core source:- https://github.com/fsharp/fsharp/blob/057dbf77df7355321c3c18137c2d552bdfa1272b/src/fsharp/FSharp.Core/prim-types.fs#L3526 https://github.com/fsharp/fsharp/blob/057dbf77df7355321c3c18137c2d552bdfa1272b/src/fsharp/FSharp.Core/prim-types.fs#L3558

dsyme commented 9 years ago

@isaacabraham It feels to me that we should add a new guideline saying "Consider reifying exceptional cases as data, using either Option<_>, Choice<_,Exception>, Choice<_,ExceptionDispatchInfo> or a new discriminated union type". It's definitely a legitimate and entirely respectable F#-to-F# programming technique.

An addition guideline saying "Consider using 'Try' as a prefix to indicate a function returning exceptional conditions as data. For example tryFind, tryPick, connection.TryConnect()"

BTW updating the guidelines with a full list of recommended collection function names from F# 4.0 would also be nice.

tpetricek commented 9 years ago

I think Choice<_, Exception> is great for locally scoped exception handling (say, within file), but I would not recommend it for public API. For example, lots of async code uses this because it needs to pass exceptions as data, but then it throws the exception within the async { .. } block. This is an important technique - but I think exposing the user to pattern matching on Choice1Of2 and Choice2Of2 would be poor practice (custom DU types are fine though, because they carry some meaning...)

kjnilsson commented 8 years ago

The problem with using custom DUs to carry exception data is that they compose poorly. At least with Choice there is readily available plumbing that can be used. I think the semantics of Choice<'a, exn> are fairly well known.