6

Suppose of having a library exposing the following piece of functionality:

public static class AwesomeHelpers 
{
  public static async Task<int> ComputeSomethingImportAsync(CalculationInputs input)
  {
    int result = 13;
    // actual implementation is not relevant for this discussion
    return result;
  }
}

Suppose also that the calculation could fail for 3 different reasons, depending on external services used by the calculation algorithm:

  • an external API returns a non success status code
  • a record of data is missing in the database
  • another external API returns a response containing invalid JSON (response content cannot be parsed as JSON)

The public API of the library either returns an integer which is the calculation result or throws a proper exception to communicate the calculation failure to the library users. The possible failures of the calculation process are only the ones listed above. Notice that there is not the possibility of a business failure of the calculation (e.g.: the calculation process cannot be completed based on a certain business rule); the only possible failures depend on errors at the level of external services, so throwing an exception to the library user seems the correct thing to do here.

Given this scenario, there are 2 main strategies I can think of:

  • throw 3 different exception types, each one corresponding to one of the possible failures (e.g.: FooApiErrorException, MissingRecordException, BarApiErrorException)
  • define one custom exception type to be used each time something goes wrong with the calculation process (e.g.: CalculationProcessException). In this case both the exception message and the InnerException property can be used to provide further details on the error.

In both cases the exceptions raised by the library need to be carefully documented (e.g.: XML documentation for Visual Studio intellisense).

Based on my experience and personal preference, the second strategy (defining one custom exception type to be used for all possible failures and specifying the error details with the message and the InnerException property) is the best solution, because it allows the library user to write simpler code.

What I mean is that this code

try 
{
  result = await AwesomeHelpers.ComputeSomethingImportAsync(input);
}
catch(CalculationProcessException exception) 
{
  // handle the error
  // the original exception is inside the InnerException property
  // the error message explains what happened 
}

is simpler than this:

try 
{
  result = await AwesomeHelpers.ComputeSomethingImportAsync(input);
}
catch(FooApiErrorException exception) 
{
  // handle Foo API error
}
catch(MissingRecordException exception) 
{
  // handle missing record error
}
catch(BarApiErrorException exception)
{
  // handle Bar API error
}

What do you think about this ? Any thoughts or observation on this ? Do you know any useful reference for this code design issue ?

EDIT 31st Agust 2021

It's interesting to notice that the Microsoft Azure SDK for Service Bus has adopted the approach of defining just one exception type (called ServiceBusException) for any error thrown by the library.

They use an enum property on the exception object (called Reason) to allow the client code to understand the root cause of the error. The details are available here

2
  • 2
    You're approaching this issue from the implementation point of view. I suggest you do the opposite. How will the clients of your API deal with these multiple issues? Will they want to handle all the different errors in the same way? Commented Aug 18, 2021 at 17:20
  • @VincentSavard probably, as suggested by nvoigt answer, these infra level failures does not need any special treatment. The simplest thing to do is carefully log them and go on with a default computation result or re throw the exception and let it crash. The only need is being able to record a clear and complete log message. So, probably, having different exception types is not needed here. Commented Aug 19, 2021 at 8:07

3 Answers 3

9

You might be overthinking this.

You only need actual exception types if you want your caller to be able to differentiate at runtime, which action to take in response to the error.

The question is, can your caller do anything about the errors? Is one of them an error where the caller program could decide to do something about it? For example, lets say the endpoint is not available, is it possible and a plausible scenario that the caller says "oh, then take this other backup endpoint, that should work, try again please".

You said you have no business constraints that would let this fail, so it's basically down to infrastructure failures. In my experience, programs do not have a list of backup databases, alternative REST endpoints or similar. If such a thing exists, it is baked into the infrastructure as load balancers, server farms or something like that, it's not programmed into every single application. The program, when infrastructure fails, cannot do anything to fix that. It can report the failure and either die or move on. But it cannot correct it.

So based on that premise, your caller being unable to fix the mistake on the fly, there is no point in having different exceptions. Your call worked, or it did not. If it did not, the exception should contain enough information for humans to fix the problem when they read the logs. Take any exception type that fits, maybe create one new one for your framework if you think it's needed. But that's it.

4
  • agree with your analysis. Failures at the infra level, like the one I have described in my question, are important to be tracked and carefully logged so that later it is possible to read the logs and investigate the root cause. This happens frequently in our applications which have a service oriented architecture (so they are quite exposed to failures at the infrastructure level) Commented Aug 19, 2021 at 8:02
  • To summarize, I think that in a case like the one I described the only possible action is logging the excetion and re throws it, or log the exception and decide to return a default result for the computation (based on the business rules). In any case it is important to be able to carefully log what happened. Based on this, there is probably no need to have different exception types. Commented Aug 19, 2021 at 8:04
  • 2
    For the sole purpose of logging the error having one type of exception is enough. Of course both the InnerException and the error message must be used in the log so that the logged information can be useful later to investigate the root cause of the error. Commented Aug 19, 2021 at 8:05
  • You are focused on logging and finding the cause of the problem. I would first look at what the user experience would be.
    – gnasher729
    Commented Sep 2, 2021 at 20:44
4
try 
{
  result = await AwesomeHelpers.ComputeSomethingImportAsync(input);
}
catch(FooApiErrorException exception) 
{
  // handle Foo API error
}
catch(MissingRecordException exception) 
{
  // handle missing record error
}
catch(BarApiErrorException exception)
{
  // handle Bar API error
}

Don't forget that catch blocks can rely on inherited types.

Even if your library has all three exceptions, if the user doesn't care about distinguishing between exceptions because they can't respond to specific problems anyway, the consumer is always able to just:

catch(Exception ex) { }

And all three of your exceptions will be caught.

As a good practice measure, you generally want to avoid blindly catching all exceptions, so it might be helpful to have your exception types derive from a custom base class, e.g. MyCustomLibraryException, so consumers can still catch all of your exception but not the others:

catch(MyCustomLibraryException ex) { }

Alternatively, you could also use composition and put your specific exception in MyCustomLibraryException.InnerException, but I prefer the inheritance approach here as it leads to somewhat cleaner catch logic.

2

I think it's better to avoid both of those approaches.

The purpose of a static type system is to prove (with some caveats) that your code is correct. From the perspective of the static type system, a given method is "correct" if it complies with the contract that is expressed by the method signature. That contract says:

"Given these preconditions (the method parameters) I will create this postcondition (the return value)."

If your method has been invoked, then the expressed preconditions have necessarily been met, and you are obligated by the contract to return a value. By throwing an exception, you are (a) violating the contract and (b) telling the static type system to ignore that fact.

Again, that's from the perspective of the static type system.

From the perspective of a software engineer, documentation is also part of the contract. If you throw a documented exception, then you aren't violating the contract; you're just implementing a contract that the static type system doesn't understand.

But why would you do that? A contract where you EITHER return a string OR throw a documented IOException is pretty much exactly the same as a contract where you return a discriminated union that contains EITHER a string OR an IOException. The only important difference is that the static type system can't help you if you use documented exceptions. So, again, why would you use documented exceptions instead of discriminated unions?

The same argument generalizes to any number of exceptions. If you have two types of IO exception, and you want to support error handling for both exception types, then you can return a DiscriminatedUnion<TValue, IOException, IOException> and document anything that isn't obvious from looking at the type. This is much easier than building an exception hierarchy, or defining an exception type that exposes a value that can be inspected by the error handler.

You could try to argue that an exception hierarchy is better for versioning, i.e. that documented exceptions allow greater flexibility for adding or removing error cases without a breaking change to the API, but I don't believe that's true. If you want to change the error handling, just add a new method with a new name and mark the old method obsolete. Iterating on exception hierarchies without a breaking change is hard, and the static type system can't help you.

For emphasis, I've never encountered a situation where documented exceptions helped me refactor something. I very often discover runtime errors that should have been caught at compile-time, and these are usually caused by documented exceptions.

None of this applies to undocumented exceptions. Those are very useful! But if you're thinking about how to implement error handling for an undocumented exception, then you're probably making a mistake.

If I had to use one of the approaches you described, then I would use the first approach, i.e. exception hierarchies. It's the more commonly used approach, and it avoids the need for nontrivial datatypes moving up the call stack using invisible flow control.

If discriminated unions are an option, then I will always use that approach instead, even if I need to implement DiscriminatedUnion (or something equivalent) myself.

1
  • 2
    I very much agree with your sentiment here, however I would add that C# does not have proper a proper Discriminated Union in the language and thus its a bit clunky. I've messed around with a sample implementation as can be seen here: softwareengineering.stackexchange.com/questions/409471/…
    – GHP
    Commented Aug 19, 2021 at 13:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.