tags:

views:

325

answers:

8

Is there a best-practice or industry standard for throwing exceptions from a toolkit API?

Should the user facing methods catch and wrap up Exception in some sort of CustomException so that users only have to worry about CustomExceptions coming out of the API?

Or is the convention to just let those bubble up?

We're concerned with being able to document all possible exceptions our API methods might throw. (e.g. if our API method calls, Stream.Write() which throws 4 or 5 exceptions we'd have to document all of those plus other exceptions other called methods might throw)

We were thinking of doing something like this

public void customerFacingApiMethod(){
   try {
      //api functionality goes here
   } catch (Exception e) {
      throw new CustomException(e);
   }
}
+10  A: 

In my opinion having the API throwing only one exception type is a bad idea. One of the good things with different exceptions is that you can choose to catch different types of exceptions and handle them differently. Wrapping up exceptions into a single exception type would remove that facility.

Use the exception types provided by the framework where appropriate, and create your own custom exception types for specific situations where appropriate. And above all, make sure to document for each method which exceptions they may throw.

Fredrik Mörk
+1 for not hiding internal exception details
Oded
In principle, we agree. But what if one of the calls we are making changes an exception it throws, now our documentation is out of sync. Or what if their documentation is incorrect and there is a thrown exception which is not documented.
OTisler
what if what if what if.You can what if yourself to death, the point is you're destroying the greatest feature of exceptions, which is that they're typed.
Fred
+1  A: 

I don't like it. If you wrap all exceptions into a CustomException it will be difficult to catch specific exceptions without searching inside the object for the InnerExcpetion (possibly multiple levels deep if your API also uses other APIs that do the same thing). If there's an OutOfMemoryException, I'd like to know that without having to go searching for it.

Mark Byers
+1  A: 

Don't use the same exception for all.

You need the different exception types to be able to separate one case from another. For anything that you'd let it flow up until a generic exception handler block catches it would seem k, but you are preventing any other code recovery - extra actions that operate on specific cases.

You can wrap a set of exceptions, if those fit into a specific scenario that can be handled by the calling code. Don't forget that some framework exceptions already communicate the situation well.

eglasius
+1  A: 

What Fredrik Mork says is very true. I also like to add to that that you can easily document the exceptions that are thrown with xml comments (and GhostDoc).

So an API user can lookup in the documentation to see what exceptions can be thrown. However, there is a downside.

If you have a deep callstack inside your API and especially if the cyclomatic complexity becomes high (i.e. num of possible branches) you cannot recover all possible exceptions that can be thrown (perhaps by the runtime?)

So i would recommend to only throw if something if recovery is not possible and only throw in the upper layer of your API, i.e. no deeper than 2 deep. And catch all exceptions that are thrown deeper in your callstack and return them as InnerException or something like that.

Henri
+3  A: 

When throwing exceptions, make sure that the user-facing exceptions are all relevant to what the user actually did wrong with regards to your toolkit. That might mean catching or merging different filesystem exceptions into a single exception but you shouldn't ever just catch Exception and throw a new one - that's not actually telling the toolkit user what they did wrong.

Alex Kaminoff
+3  A: 

You should follow the same concepts as the .NET Framework. Your users already use the framework, so they know how it works and expect certain behavior.

  • Throw an ArgumentException-derived exception if an argument from the client is invalid (use ArgumentNullException, ArgumentOutOfRangeException, etc. as appropriate).
  • Throw an IOException-derived exception for IO errors.

Etc...

In your documentation, clearly state the preconditions for your public API and state the exceptions that will be thrown when they fail (like MSDN does).

280Z28
+1  A: 

There are two types of exceptions to consider (ignoring StackOverflowException, etc., which you can't do anything about anyway):

  1. Those that are caused by external issues (like file IO), which have to be handled.
  2. Those that can always be avoided with properly validated input.

It usually makes sense for the IO-type exceptions to be passed up the call stack unmodified, since there isn't anything you can really do about it anyway. Exceptions to this would be anticipated errors for which you might do retries, or asking the user what to do, if your API operates at the GUI level (which would be rather unusual).

Just be sure to document which exceptions of this type your API methods can throw.

Of the second type (exceptions that can always be prevented), these exceptions may be generated by your API on bad input, but should never be passed up the call-stack from the lower levels. Input to anything you call should be validated so that these errors do not occur, and if, for some reason, they can occur, then these exceptions should be wrapped up. Otherwise, the code that uses your API will have to deal with exceptions at the wrong level of abstraction.

And once again, be sure to document what sort of input will not generate exceptions, and what kinds of exceptions to expect if these rules are broken.

In all cases, throw exceptions that make sense to the user of your API, not the person programming the API itself.

Jeffrey L Whitledge
+1  A: 

The problem with catching System.Exception in the general case is that by doing so, you're effectively saying "I don't have any idea of why this failed, and that's okay, because I know I can continue anyway!"

That's rarely, if ever, true.

Wrapping up all exceptions in a single custom exception type means that your consumers will just catch that type - which is then the moral equivalent of catching System.Exception. While you may satisfy some policy/FXCop rule by doing that, you're really still just catching System.Exception.

For your exception policy, I'd start with asking this: "If I didn't know what this API actually talked to, what kind of exception would I expect to get back?" And remember that a method is a request to an object to do something, and an exception is the object's way of letting you know that it couldn't, and why.

kyoryu