Best Practice way to indicate that a server request has failed?

Question

I am writing an API that connects to a service which either returns a simple "Success" message or one of over 100 different flavors of failure.

Originally I thought to write the method that sends a request to this service such that if it succeeded the method returns nothing, but if it fails for whatever reason, it throws an exception.

I didn't mind this design very much, but on the other hand just today I was reading Joshua Bloch's "How to Design a Good API and Why it Matters", where he says "Throw Exceptions to indicate Exceptional Conditions...Don't force client to use exceptions for control flow." (and "Conversely, don't fail silently.")

On the other-other hand, I noticed that the HttpWebRequest I am using seems to throw an exception when the request fails, rather than returning a Response containing a "500 Internal Server Error" message.

What is the best pattern for reporting errors in this case? If I throw an exception on every failed request, am I in for massive pain at some point in the future?

Edit: Thank you very kindly for the responses so far. Some elaboration:

• it's a DLL that will be given to the clients to reference in their application.
• an analogous example of the usage would be ChargeCreditCard(CreditCardInfo i) - obviously when the ChargeCreditCard() method fails it's a huge deal; I'm just not 100% sure whether I should stop the presses or pass that responsibility on to the client.

Edit the Second: Basically I'm not entirely convinced which of these two methods to use:

try { 
ChargeCreditCard(cardNumber, expDate, hugeAmountOMoney);
} catch(ChargeFailException e) {
  // client handles error depending on type of failure as specified by specific type of exception
}

or

var status = TryChargeCreditCard(cardNumber, expDate, hugeAmountOMoney);

if(!status.wasSuccessful) {
    // client handles error depending on type of failure as specified in status   
}

e.g. when a user tries to charge a credit card, is the card being declined really an exceptional circumstance? Am I going down too far in the rabbit hole by asking this question in the first place?

Answer 1

Here's a short list of things to consider. While not comprehensive, I believe these things can help you write better code. Bottom line: Don't necessarily perceive exception handling as evil. Instead, when writing them, ask yourself: How well do I really understand the problem I am solving? More often than not, this will help you become a better developer.

1. Will other developers be able to read this? Can it be reasonably understood by the average developer? Example: ServiceConnectionException vs. a confusing ServiceDisconnectedConnectionStatusException
2. In the case of throwing an exception, how exceptional is the circumstance? What does the caller have to do in order to implement the method?
3. Is this exception fatal? Can anything really be done with this exception if it is caught? Threads aborting, out of memory.. you can't do anything useful. Don't catch it.
4. Is the exception confusing? Let's say you have a method called Car GetCarFromBigString(string desc) that takes a string and returns a Car object. If the majority use-case for that method is to generate a Car object from that string, don't throw an exception when a Car couldn't be determined from the string. Instead, write a method like bool TryGetCarFromBigString(string desc, out Car).
5. Can this be easily prevented? Can I check something, let's say the size of an array or a variable being null?

For code readability's sake, let's potentially take a look at your context.

bool IsServiceAlive()
{
   bool connected = false;  //bool is always initialized to false, but for readability in this context

   try
   {
      //Some check
      Service.Connect();
      connected = true;
   }
   catch (CouldNotConnectToSomeServiceException)
   {
      //Do what you need to do
   }

   return connected;
}

//or
void IsServiceAlive()
{
   try
   {
      //Some check
      Service.Connect();
   }
   catch (CouldNotConnectToSomeServiceException)
   {
      //Do what you need to do
      throw;
   }
}



static void Main(string[] args)
{
    //sample 1
    if (IsServiceAlive())
    {
       //do something
    }

    //sample 2
    try
    {
       if (IsServiceAlive())
       {
          //do something
       }
    }
    catch (CouldNotConnectToSomeServiceException)
    {
       //handle here
    }

    //sample 3
    try
    {
       IsServiceAlive();
       //work
    }
    catch (CouldNotConnectToSomeServiceException)
    {
       //handle here
    }
}

You can see above, that catching the CouldNotConnectToSomeServiceException in sample 3 doesn't necessarily yield any better readability if the context is simply a binary test. However, both work. But is it really necessary? Is your program hosed if you can't connect? How critical is it really? These are all factors you will need to take in to account. It's hard to tell since we don't have access to all of your code.

Let's take a look at some other options that most likely lead to problems.

//how will the code look when you have to do 50 string comparisons?  Not pretty or scalable.
public class ServiceConnectionStatus
{
     public string Description { get; set; }
}

and

//how will your code look after adding 50 more of these?
public enum ServiceConnectionStatus
{
   Success,
   Failure,
   LightningStormAtDataCenter,
   UniverseExploded
}

Answer 2

I think you need to consider a few things in your design:

1) How will the API be accessed? If you are exposing it over web services, then throwing exceptions are probably not a good idea. If the API is in a DLL that you are providing for people to reference in their applications, then exceptions may be ok.

2) How much additional data needs to travel with the return value in order to make the failure response useful for the API consumer? If you need to provide usable information in your failure message (i.e. user id and login) as opposed to a string with that information embedded, then you could utilize either custom exceptions or an "ErrorEncountered" class that contains the error code and other usable information. If you just need to pass a code back, then an ENum indicating either success (0) or failure (any non-zero value) may be appropriate.

3) Forgot this in the original response: exceptions are expensive in the .Net framework. If your API will be called once in awhile, this doesn't need to factor in. However, if the API is called for every web page that is served in a high-traffic site, for example, you definitely do not want to be throwing exceptions to indicate a request failure.

So the short answer, is that it really does depend on the exact circumstances.

Answer 3

I really like the "Throw Exceptions to indicate Exceptional Conditions" idea. They must have that name for a reason.

In a regular application, you would use File.Exists() prior to a File.Open() to prevent an exception from being thrown. Expected errors as exceptions are hard to handle.

In a client-server environment though, you may want to prevent having to send two requests and create a FileOpenResponse class to send both status and data (such as a file handle, in this case).