Handling exceptions from a method that could fire multiple exceptions

Question

I have a API class using the MemoryStream and GZipStream classes to compress and decompress a string to a byte array.

Using these two classes a number of exceptions may be throws and I was wondering what is the best method of handling thrown exception for an API. In this case would it be be better to wrap each of the exception with my own Custom Exception or would it be preferable to capture each individual exception in the calling code?

I suppose this is a question that is not limited to this particular use case but more about general exception handling best practices.

/// <summary>
/// Compress the string using the SharpLibZip GZip compression routines
/// </summary>
/// <param name="s">String object to compress</param>
/// <returns>A GZip compressed byte array of the passed in string</returns>
/// <exception cref="Helper.Core.Compression.StringCompressionException">Throw when the compression memory stream fails </exception>
/// <exception cref="System.ArgumentNullException">Throw when string parameter is Null</exception>
/// <exception cref="System.ArgumentException">Throw when the string parameter is empty</exception>
public async Task<byte[]> CompressStringAsync(string s)
{
    if (s == null) throw new ArgumentNullException("s");
    if (string.IsNullOrWhiteSpace(s)) throw new ArgumentException("s");

    byte[] compressed = null;

    try
    {
        using (MemoryStream outStream = new MemoryStream())
        {
            using (GZipStream tinyStream = new GZipStream(outStream,CompressionMode.Compress))
            {
                using (MemoryStream memStream = new MemoryStream(Encoding.UTF8.GetBytes(s)))
                {
                    await memStream.CopyToAsync(tinyStream);
                }
            }
            compressed = outStream.ToArray();
        }
        return compressed;
    }
    catch (ArgumentNullException ex)
    {
        throw new StringCompressionException("Argument Was Null", ex);
    }
    catch (EncoderFallbackException ex)
    {
        throw new StringCompressionException("Stream Encoding Failure", ex);
    }
    catch (ArgumentException ex)
    {
        throw new StringCompressionException("Argument Was Not Valid", ex);
    }
    catch (ObjectDisposedException ex)
    {
        throw new StringCompressionException("A Stream Was Disposed", ex);
    }
    catch (NotSupportedException ex)
    {
        throw new StringCompressionException("Action Was Not Supported", ex);
    }
}

Here is a good post on catching base exceptions.

score 2 · Accepted Answer · answered Jul 11 '13 at 08:43

You're already taking advantage of the "inner exception" functionality of the Exception base class in the example code you show, so why not take it all the way?

You don't need to re-write the message of the inner exception, you just wrap the exception in your StringCompressionException class. If the client who catches your exception wants to know more detailed information about the failure, they can check the inner exception.

That makes your code a lot simpler:

try
{
    using (MemoryStream outStream = new MemoryStream())
    {
        using (GZipStream tinyStream = new GZipStream(outStream,CompressionMode.Compress))
        {
            using (MemoryStream memStream = new MemoryStream(Encoding.UTF8.GetBytes(s)))
            {
                await memStream.CopyToAsync(tinyStream);
            }
        }
        compressed = outStream.ToArray();
    }
    return compressed;
}
catch (Exception ex)
{
    throw new StringCompressionException("Compressing string failed; see inner exception for details.", ex);
}

Also keep in mind that exception message strings do not localize easily, so they're not something that you should be displaying to the end user anyway. They're really just a debugging aid, so something like "see inner exception for details" works just fine. Maintenance programmers know what to do with that.

Isn't it generally frowned upon to catch the base Exception class? Code Analysis tools tend to gripe about it. — Phil Murray, Jul 11 '13 at 08:46
@Phil Well yes, it normally is, because when you do so, you'll also catch unrecoverable errors like `OutOfMemoryException`. And you should only catch exceptions that you are prepared to handle. But in this case, you're just wrapping the underlying exception in one specific for your API and then rethrowing, so it would probably be OK. You don't have much other choice unless you do what the original code does: cherry-pick individual exceptions. The exception hierarchy in .NET isn't designed all that well, I don't think. — Cody Gray - on strike, Jul 11 '13 at 08:49

score 0 · Answer 2 · answered Jul 11 '13 at 09:08

0

Generally i would do it like so

try
{
    CodeThatThrowsExceptions();
}
catch(ArgumentException ae)
{
    //Recoverable, request new args
}
catch(OtherRecoverableException oe)
{
    //take appropriate action
}
catch(Exception e)
{
    //Unexpected irrecoverable error. Handle appropriately
}

This allows you to do what is relevant in each situation (i.e. request new arguments) and handle/throw as appropriate.

answered Jul 11 '13 at 09:08

David Colwell

2,450
20
31

How does one "request new arguments"? Does your code do something other than throw an exception when it gets passed invalid arguments? – Cody Gray - on strike Jul 12 '13 at 18:46

score 0 · Answer 3 · answered Jul 11 '13 at 10:27

0

It is always recommended that you handle specific exceptions and then general exception at the end. Also the decision depends on the kind of action that you plan to implement upon receiving the specific exception. From you code it looks you are raising same exception from all specific exceptions, so unless there is any specific action you want to take in that particular block, i would use on catch block with base exception or simply catch.

Also on thing to keep in mind is the throw new xxx resets the stack trace, so you might miss some important information from there the exception has been generated.

answered Jul 11 '13 at 10:27

seshuk

182
4

The new exception object he's throwing includes the original as its inner exception, so there's no risk of losing any information. – Cody Gray - on strike Jul 12 '13 at 18:45
I see that, i was just commenting about standard practice. :), thanks for point it out. – seshuk Jul 13 '13 at 17:14

Handling exceptions from a method that could fire multiple exceptions

3 Answers3