2

I want to implement custom exception handling in my library so that I know where an exception occurred, so I am doing this:

try
{
    DoSomething();
}
catch(Exception ex)
{
    //LibraryException refers to any exception in the libraries exception hierarchy
    throw new LibraryException(ex.Message, ex);
}
  • Should this be avoided?

  • Does it have any performance implications?

  • What are the implications of catching, nesting and re-throwing exceptions?

Matthew Layton
  • 39,871
  • 52
  • 185
  • 313
  • Just wondering, how does this help you to know where the exception occurred? All exceptions have a stack trace, why can't you use that? – Hogan Sep 26 '12 at 13:53
  • This has been asked before, see: http://stackoverflow.com/questions/4761216/c-throwing-custom-exception-best-practices http://stackoverflow.com/questions/22623/net-throwing-exceptions-best-practices – George Stocker Sep 26 '12 at 14:01

5 Answers5

4

The only potential problem is that you're catching a non-specific Exception, so not conforming to the exception handling guidelines.

One of the following would be more conformant with these guidelines:

try
{
    DoSomething();
}
catch(SomeException ex)
{
    throw new LibraryException(ex.Message, ex);
}

or:

try
{
    DoSomething();
}
catch(Exception ex)
{
    if (ex is SomeException || ex is SomeOtherException)
    {
        throw new LibraryException(ex.Message, ex);
    }
    throw;
}

As for performance, once an Exception has been thrown, something exceptional has happened, and you probably don't care about the small additional overhead of wrapping it.

From comments:

in most cases when a library chooses to wrap exceptions it will wrap all exceptions that are thrown outside of the scope of the library, ...

I disagree with this, though it's admittedly subjective. One example of a library that wraps exceptions is SqlMembershipProvider, which wraps some specific exceptions in a ProviderException, e.g.:

try
{
    new Regex(this._PasswordStrengthRegularExpression);
}
catch (ArgumentException exception)
{
    throw new ProviderException(exception.Message, exception);
}

but other exceptions such as SqlException that a caller can't be expected to handle are propagated unwrapped.

Joe
  • 122,218
  • 32
  • 205
  • 338
  • 1
    In most cases when a library chooses to wrap exceptions it will wrap all exceptions that are thrown outside of the scope of the library, so the global catch would be appropriate. If you intend to only wrap certain types of exceptions then this would be appropriate. Oh, and I would use several `catch` blocks rather than checking the type of the exception with `is`, but that's more personal preference. – Servy Sep 26 '12 at 14:17
  • @Joe, I have to agree with Servy, even [Microsoft](http://msdn.microsoft.com/en-us/library/ms182137.aspx) is not keen on catching any general exceptions. – iMortalitySX Sep 26 '12 at 21:00
2

This is a good practice, if it adds abstraction and/or clarity to your architecture.

It makes the exception more explicit, and may hide lower-layer details from higher layers. For instance, in a Data Access Layer you may catch SqlExceptions and throw DataAccessExceptions instead, so that the higher layers do not "know" you are using SQL Server as your data store.

Roy Dictus
  • 32,551
  • 8
  • 60
  • 76
2

Should this be avoided?

That's a matter of preference. In some situations it can be advantageous, in some detrimental, and in most cases some of both. There are cases where you see this done by existing libraries, and there are other libraries that choose not to.

Does it have any performance implications?

Probably, yes, but I doubt it's real significant. It's not exactly cheap to begin with.

What are the implications of catching, nesting and re-throwing exceptions?

advantages:

  • Someone calling your method can easily catch exceptions thrown from your library.
  • Hides implementation details of your library from the caller.
  • Allows the caller to easily catch exceptions just from your library. Without this if they try to catch, say, an invalid argument exception it could come from either your library or their other code. They may want to only catch argument exceptions thrown from other places.

disadvantages:

  • It's harder to catch just one of the types of exceptions that may have been thrown from your library while not catching others.
  • It adds a lot of clutter to the exception text when it is displayed.
Servy
  • 202,030
  • 26
  • 332
  • 449
1

Ok, so as a real quick overview, when an exception is caught in a try catch block, it has to gather then entire stack for the trace. Throwing any type of exception is actually rather expensive, but it gets exponentially so when you nest throws inside of catch blocks. So really you should only throw an exception if you know exactly why you are throwing it and plan on handling it through a try. This is mostly the case when you are creating an API, and cannot predict how another developer may use your library.

If you are writing for your own application, you should avoid throwing an exception at all, since you are just throwing it to yourself (basically). In that case, you should be handling what happens if an exception is thrown not throw your own.

Unlike Python (which I like but different paradigm) and other "oops" languages, it is not good practice to control flow through exception handling in C#.

Under C# how much of a performance hit is a try, throw and catch block

Arguments for or against using try catch as logical operators

Exception Handling in C#

C# Exception (Check this one out)

CA1031: Do not catch general exceptions

Community
  • 1
  • 1
iMortalitySX
  • 1,478
  • 1
  • 9
  • 23
  • hmmm.... The statement "it is not a good practice to control flow through exception handling" is true. But that is not the case in this question, this question isn't about controlling normal flow it is about exceptions. The program is going to crash anyway -- won't have a big performance effect. – Hogan Sep 26 '12 at 14:11
1

In context, throwing a more specific exception is generally a good idea, provided that LibraryException comes from your own problem domain and provides more specific information to a developer such that they can say "Ah, a Library exception means that one of a certain class of errors occurred."

try-catches do impose their own performance overhead, but I see nothing that makes this block substantially worse than any other similar block.

David W
  • 10,062
  • 34
  • 60