c# exception handling, practical example. How would you do it?

Question

I'm trying to get better at handling exceptions but I feel like my code gets very ugly, unreadable and cluttered when I try my best to catch them. I would love to see how other people approach this by giving a practical example and compare solutions.

My example method downloads data from an URL and tries to serialize it into a given type, then return an instance populated with the data.

First, without any exception-handling at all:

    private static T LoadAndSerialize<T>(string url)
    {            
        var uri = new Uri(url);
        var request = WebRequest.Create(uri);
        var response = request.GetResponse();
        var stream = response.GetResponseStream();

        var result = Activator.CreateInstance<T>();
        var serializer = new DataContractJsonSerializer(result.GetType());
        return (T)serializer.ReadObject(stream);            
    }

I feel like the method is fairly readable like this. I know there are a few unnecessary steps in the method (like WebRequest.Create() can take a string, and I could chain methods without giving them variables) there but I will leave it like this to better compare against the version with exception-handling.

This is an first attempt to handle everything that could go wrong:

    private static T LoadAndSerialize<T>(string url)
    {
        Uri uri;
        WebRequest request;
        WebResponse response;
        Stream stream;
        T instance;
        DataContractJsonSerializer serializer;

        try
        {
            uri = new Uri(url);
        }
        catch (Exception e)
        {
            throw new Exception("LoadAndSerialize : Parameter 'url' is malformed or missing.", e);
        }

        try
        {
            request = WebRequest.Create(uri);
        }
        catch (Exception e)
        {
            throw new Exception("LoadAndSerialize : Unable to create WebRequest.", e);
        }

        try
        {
            response = request.GetResponse();
        }
        catch (Exception e)
        {
            throw new Exception(string.Format("LoadAndSerialize : Error while getting response from host '{0}'.", uri.Host), e);
        }

        if (response == null) throw new Exception(string.Format("LoadAndSerialize : No response from host '{0}'.", uri.Host));

        try
        {
            stream = response.GetResponseStream();
        }
        catch (Exception e)
        {
            throw new Exception("LoadAndSerialize : Unable to get stream from response.", e);
        }

        if (stream == null) throw new Exception("LoadAndSerialize : Unable to get a stream from response.");

        try
        {
            instance = Activator.CreateInstance<T>();
        }
        catch (Exception e)
        {
            throw new Exception(string.Format("LoadAndSerialize : Unable to create and instance of '{0}' (no parameterless constructor?).", typeof(T).Name), e);
        }

        try
        {
            serializer = new DataContractJsonSerializer(instance.GetType());
        }
        catch (Exception e)
        {

            throw new Exception(string.Format("LoadAndSerialize : Unable to create serializer for '{0}' (databinding issues?).", typeof(T).Name), e);
        }


        try
        {
            instance = (T)serializer.ReadObject(stream);
        }
        catch (Exception e)
        {
            throw new Exception(string.Format("LoadAndSerialize : Unable to serialize stream into '{0}'.", typeof(T).Name), e);                   
        }

        return instance;
    }

The problem here is that while everything that could possibly go wrong will be caught and given a somewhat meaningful exception, it is a clutter-fest of significant proportions.

So, what if I chain the catching instead. My next attempt is this:

    private static T LoadAndSerialize<T>(string url)
    {
        try
        {
            var uri = new Uri(url);
            var request = WebRequest.Create(uri);
            var response = request.GetResponse();
            var stream = response.GetResponseStream();
            var serializer = new DataContractJsonSerializer(typeof(T));
            return (T)serializer.ReadObject(stream);
        }
        catch (ArgumentNullException e)
        {
            throw new Exception("LoadAndSerialize : Parameter 'url' cannot be null.", e);
        }             
        catch (UriFormatException e)
        {
            throw new Exception("LoadAndSerialize : Parameter 'url' is malformed.", e);
        }
        catch (NotSupportedException e)
        {
            throw new Exception("LoadAndSerialize : Unable to create WebRequest or get response stream, operation not supported.", e);
        }
        catch (System.Security.SecurityException e)
        {
            throw new Exception("LoadAndSerialize : Unable to create WebRequest, operation was prohibited.", e);
        }
        catch (NotImplementedException e)
        {
            throw new Exception("LoadAndSerialize : Unable to get response from WebRequest, method not implemented?!.", e);
        }
        catch(NullReferenceException e)
        {
            throw new Exception("LoadAndSerialize : Response or stream was empty.", e);
        }
    }

While it certainly is easier on the eyes, I am leaning heavily the intellisense here to provide all exceptions that can possibly be thrown from a method or class. I don't feel confident that this documentation is 100% accurate, and would be even more skeptical if some of the methods came from an assembly outside the .net framework. As an example the DataContractJsonSerializer show no exceptions on the intellisense. Does this mean the constructor will never fail? Can I be sure?

Other issues with this is that some of the methods throw the same exception, which makes the error harder to describe (this or this or this went wrong) and so is less useful to the user / debugger.

A third option would be to ignore all exceptions apart from the ones that would allow me to take an action like retrying the connection. If the url is null then the url is null, the only benefit from catching that is a little bit more verbose error message.

I would love to see your thoughts and/or implementations!

Answer 1

Rule one of exception handling - do not catch exceptions you don't know how to handle.

Catching exceptions just in order to provide nice error messages is questionable. The exception type and message already contain enough information for a developer - the messages you have provided do not add any value.

the DataContractJsonSerializer show no exceptions on the intellisense. Does this mean the constructor will never fail? Can I be sure?

No, you can't be sure. C# and .NET in general are not like Java where you have to declare what exceptions may be thrown.

A third option would be to ignore all exceptions apart from the ones that would allow me to take an action like retrying the connection.

That indeed is the best option.

You can also add a general exception handler at the top of the application that will capture all unhandled exceptions and log them.

Answer 2

First, read my article on exception handling:

http://ericlippert.com/2008/09/10/vexing-exceptions/

My advice is: you must handle the "vexing exceptions" and "exogenous exceptions" that can be thrown by your code. Vexing exceptions are "non exceptional" exceptions and so you have to handle them. Exogenous exceptions can happen due to considerations beyond your control, and so you have to handle them.

You must not handle the fatal and boneheaded exceptions. The boneheaded exceptions you don't need to handle because you are never going to do anything that causes them to be thrown. If they are thrown then you have a bug and the solution is fix the bug. Don't handle the exception; that's hiding the bug. And you can't meaningfully handle fatal exceptions because they're fatal. The process is about to go down. You might consider logging the fatal exception, but keep in mind that the logging subsystem might be the thing that triggered the fatal exception in the first place.

In short: handle only those exceptions that can possibly happen that you know how to handle. If you don't know how to handle it, leave it to your caller; the caller might know better than you do.

In your particular case: don't handle any exceptions in this method. Let the caller deal with the exceptions. If the caller passes you an url that cannot be resolved, crash them. If the bad url is a bug then the caller has a bug to fix and you are doing them a favour by bringing it to their attention. If the bad url is not a bug -- say, because the user's internet connection is messed up -- then the caller needs to find out why the fetch failed by interrogating the real exception. The caller might know how to recover, so help them.