Uniformly handling error codes in an unmanaged API

I'm writing a wrapper around a fairly large unmanaged API. Almost every imported method returns a common error code when it fails. For now, I'm doing this:

ErrorCode result = Api.Method();
if (result != ErrorCode.SUCCESS) {
    throw Helper.ErrorToException(result);
}

This works fine. The problem is, I have so many unmanaged method calls that this gets extremely frustrating and repetitive. So, I tried switching to this:

public static void ApiCall(Func<ErrorCode> apiMethod) {
    ErrorCode result = apiMethod();
    if (result != ErrorCode.SUCCESS) {
        throw Helper.ErrorToException(result);
    }
}

Which allows me to cut down all of those calls to one line:

Helper.ApiCall(() => Api.Method());

There are two immediate problems with this, however. First, if my unmanaged method makes use of out parameters, I have to initialize the local variables first because the method call is actually in a delegate. I would like to be able to simply declare a out destination without initializing it.

Second, if an exception is thrown, I really have no idea where it came from. The debugger jumps int开发者_如何学JAVAo the ApiCall method and the stack trace only shows the method that contains the call to ApiCall rather than the delegate itself. Since I could have many API calls in a single method, this makes debugging difficult.

I then thought about using PostSharp to wrap all of the unmanaged calls with the error code check, but I'm not sure how that would be done with extern methods. If it ends up simply creating a wrapper method for each of them, then I would have the same exception problem as with the ApiCall method, right? Plus, how would the debugger know how to show me the site of the thrown exception in my code if it only exists in the compiled assembly?

Next, I tried implementing a custom marshaler that would intercept the return value of the API calls and check the error code there. Unfortunately, you can't apply a custom marshaler to return values. But I think that would have been a really clean solution it if had worked.

[return:
    MarshalAs(UnmanagedType.CustomMarshaler, MarshalTypeRef=typeof(ApiMethod))]
public static extern ErrorCode Method();

Now I'm completely out of ideas. What are some other ways that I could handle this?

Follow ErrorHandler class from the Visual Studio 2010 SDK. It existed in earlier versions, but the new one has CallWithCOMConvention(Action), which may prove valuable depending on how your API interacts with other managed code.

Of the available methods, I recommend implementing the following:

• Succeeded(int)
  (Failed() is just !Succeeded(), so you can skip it)

• ThrowOnFailure(int)
  (Throws a proper exception for your return code)

• CallWith_MyErrorCode_Convention(Action) and CallWith_MyErrorCode_Convention(Func<int>)
  (like CallWithCOMConvention, but for your error codes)

• IsCriticalException(Exception)
  (used by CallWith_MyErrorCode_Convention)

What happens if you don't check ErrorCode.SUCCESS? Will your code quickly fail and throw an exception? Can you tell which unmanaged API failed if your managed code throws? If so, consider not checking for errors and just letting the runtime throw when your unmanaged API fails.

If this is not the case, I suggest biting the bullet and following your first idea. I know you called it "frustrating and repetitive", but after coming from a project with a "clever" macro solution to a similar problem, checking return values in method calls and wrapping exceptions is the doorway to insanity: exception messages and stack traces become misleading, you can't trace the code, performance suffers, your code become optimized for errors and goes off the rails upon success.

If a particular return value is an error, thow a unique exception then. If it might not be an error, let it go and throw if becomes an error. You said you wanted to reduce the check to one line?

if (Api.Method() != ErrorCode.SUCCESS) throw new MyWrapperException("Api.Method broke because ...");

Your proposal also throws the same exception if any method returns the same "common error code". This is another debugging nightmare; for APIs which return the same error codes from multiple calls, do this:

switch (int returnValue = Api.Method1())
{
    case ErrorCode.SUCCESS: break;
    case ErrorCode.TIMEOUT:  throw new MyWrapperException("Api.Method1 timed out in situation 1.");
    case ErrorCode.MOONPHASE: throw new MyWrapperException("Api.Method1 broke because of the moon's phase.");
    default:  throw new MyWrapperException(string.Format("Api.Method1 returned {0}.", returnValue));
}

switch (int returnValue = Api.Method2())
{
    case ErrorCode.SUCCESS: break;
    case ErrorCode.TIMEOUT:  throw new MyWrapperException("Api.Method2 timed out in situation 2, which is different from situation 1.");
    case ErrorCode.MONDAY: throw new MyWrapperException("Api.Method2 broke because of Mondays.");
    default:  throw new MyWrapperException(string.Format("Api.Method2 returned {0}.", returnValue));
}

Verbose? Yup. Frustrating? No, what's frustrating is trying to debug an app that throws the same exception from every line whatever the error.

I think, the easy way is to add aditional layer.

class Api
{
....
  private static ErrorCode  Method();//changing Method to private

  public static void NewMethod()//NewMetod is void, because error is converted to exceptions
  {
      ErrorCode result = Method();
      if (result != ErrorCode.SUCCESS) {
          throw Helper.ErrorToException(result);
  }

  }
....
}

Create a private property to hold the ErrorCode value, and throw the exception from the setter.

class Api
{
    private static ErrorCode _result;
    private static ErrorCode Result
    {
        get { return _result; }
        set
        {
            _result = value;
            if (_result != ErrorCode.SUCCESS)
            {
                throw Helper.ErrorToException(_result);
            }
        }
    }

    public static void NewMethod()
    {
        Result = Api.Method();
        Result = Api.Method2();
    }
}

Write a T4 template to do the generation for you.

Your existing code is actually really, really close. If you use an expression tree to hold the lambda, instead of a Func delegate, then your Helper.ApiCall can pull out the identity of the function that was called and add that to the exception it throws. For more information on expression trees and some very good examples, Google Marc Gravell.