Phil Murray
Phil Murray

Reputation: 6554

Handling exceptions from a method that could fire multiple exceptions

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.

Upvotes: 2

Views: 790

Answers (3)

seshuk
seshuk

Reputation: 182

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.

Upvotes: 0

David Colwell
David Colwell

Reputation: 2590

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.

Upvotes: 0

Cody Gray
Cody Gray

Reputation: 244712

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.

Upvotes: 2

Related Questions