views:

789

answers:

3

I've been writing a lot of code recently that involves interop with the Win32 API and have been starting to wonder what's the best way to deal with native (unmanaged) errors that are caused by calls to Windows API functions.

Currently, the calls to native functions look something like this:

// NativeFunction returns true when successful and false when an error
// occurred. When an error occurs, the MSDN docs usually tell you that the
// error code can be discovered by calling GetLastError (as long as the
// SetLastError flag has been set in the DllImport attribute).
// Marshal.GetLastWin32Error is the equivalent managed function, it seems.
if (!WinApi.NativeFunction(param1, param2, param3))
    throw new Win32Exception();

The line that raises the exception can be equivalently rewritten as such I believe:

throw new Win32Exception(Marshal.GetLastWin32Error());

Now, this is all well in that it throws an exception appropriately containing the Win32 error code that was set as well as a (generally) human-readable description of the error as the Message property of the Exception object. However, I have been thinking that it would be advisable to modify/wrap at least some, if not all, of these exceptions so that they give a slightly more context-oriented error message, i.e. one more meaningful in whatever situation the native code is being used. I have considered several alternatives for this:

  1. Specifying a custom error message in the constructor for Win32Exception.

    throw new Win32Exception(Marshal.GetLastWin32Error(), "My custom error message.");
    
  2. Wrapping the Win32Exception in another Exception object so that both the original error code and message are retained (the Win32Exception is now the InnerException of the parent exception).

    throw new Exception("My custom error message.", Win32Exception(Marshal.GetLastWin32Error()));
    
  3. The same as 2, except using another Win32Exception as the wrapper exception.

  4. The same as 2, except using a custom class derived from Exception as the wrapper exception.

  5. The same as 2, except using a BCL (Base Class Library) exception as the parent when appropiate. Not sure whether it's even appropiate to set the InnerException to the Win32Exception in this case (perhaps for a low-level wrapper but not a higher-level/abstracted interface that doesn't make it obvious that Win32 interop is happening behind the scenes?)

Essentially what I want to know is: what is the recommended practice on dealing with Win32 errors in .NET? I see it done in open-source code in all sorts of different ways, but I was curious whether there were any design guidelines. If not, I'd be interested in your personal preferences here. (Perhaps you even use none of the above methods?)

+2  A: 

This isn't really specific to Win32 exceptions; the question is, when should two different error cases be identified by two different Exception-derived types, and when should they throw the same type with different values stored inside it?

Unfortunately this is impossible to answer without knowing in advance all the situations your code will be called in. :) This is the problem with only being able to filter exceptions by type. Broadly speaking, if you have a strong feeling that it would be useful to treat two error cases differently, throw different types.

Otherwise, it is frequently the case that the string returned by Exception.Message just needs to be logged or displayed to the user.

If there is additional information, wrap the Win32Exception with something more high-level of your own. For example, you're trying to do something to a file, and the user you're running under doesn't have permission to do it. Catch the Win32Exception, wrap it in an exception class of your own, whose message gives the filename and the operation being attempted, followed by the inner exception's message.

Daniel Earwicker
+1 for the additional information - there's little, if anything, more frustrating than getting a "File Not Found" exception with no reference to which file can't be found.
Harper Shelby
@Earwicker: You make a fair point... Also, I'm asking this question specifically in relation to Win32Exception because they often (though not always) tend to be rather vague and unhelpful, as Harper notes.
Noldorin
To clarify your last paragraph, do you mean that it is best to append the inner error message *as well* as wrap it as the inner exception?
Noldorin
@Noldorin - sadly, yes, unless you know that the caller is going to follow a pattern for building composite messages (see update above).
Daniel Earwicker
Agh, my Windows VM is dead, so no update above.
Daniel Earwicker
+1  A: 

My view has always been that the appropriate way to handle this depends on the target audience and how your class is going to be used.

If the caller of your class/method is going to realize that they are calling into Win32 in one form or another, I would use option 1) you have specified. This seems the most "clear" to me. (However, if this is the case, I'd name your class in a manner that makes it clear that the Win32 API is going to be used directly). That being said, there are exceptions in the BCL that actually subclass Win32Exception to be more clear, instead of just wrapping it. For example, SocketException derives from Win32Exception. I've never personally used that approach, but it does seem like a potentially clean way to handle this.

If the caller of your class is going to have no idea that you're calling into the Win32 API directly, I would handle the exception, and use a custom, more descriptive exception you define. For example, if I'm using your class, and there's no indication that you're using the Win32 api (since you're using it internally for some specific, non-obvious reason), I would have no reason to suspect that I may need to handle a Win32Exception. You could always document this, but it seems more reasonable to me to trap it and give an exception that would have more meaning in your specific business context. In this case, I might wrap the initial Win32Exception as an inner exception (ie: your case 4), but depending on what caused the internal exception, I might not.

Also, there are many times when a Win32Exception would be thrown from a native call, but there are other exceptions in the BCL that are more relevant. This is the case when you're calling into a native API that isn't wrapped, but there are similar functions that ARE wrapped in the BCL. In that case, I would probably trap the exception, make sure that it is what I'm expecting, but then throw the standard, BCL exception in its place. A good example of this would be to use SecurityException instead of throwing a Win32Exception.

In general, though, I would avoid option 2 and 3 you listed.

Option two throws a general Exception type - I would pretty much recommend avoiding that entirely. It seems unreasonable to wrap a specific exception into a more generalized one.

Option three seems redundant - There's really no advantage over Option 1.

Reed Copsey
Thanks for the suggestions. I'm thinking it may be best in my situation to throw the Win32Exception directly, as the class containing the code is really a wrapper around stuff that manipulates other processes/threads/shared memory.
Noldorin
(contd.) Throwing BCL exceptions (and wrapping Win32 ones?) might also be appropiate. Option 4 does indeed seem like the best when one wants to hide the fact that Win32 interop is being used (i.e. a relatively high-level interface).
Noldorin
+1  A: 

Personally I would do #2 or #4...Preferably #4. Wrap Win32Exception inside your exception which is context sensitive. Like this:

void ReadFile()
{
    if (!WinApi.NativeFunction(param1, param2, param3))
        throw MyReadFileException("Couldn't read file", new Win32Exception());
}

This way if somebody catches the exception, they will have a pretty good idea where the problem occurred. I wouldn't do #1 because it requires the catch to interpret your text error message. And #3 doesn't really give any additional information.

Keltex
Agreed. Option 4 is probably what I'll use when I want to hide Win32 exceptions - possibly using BCL exceptions rather than custom ones where appropiate.
Noldorin