Handling COM errors in C++

Reading some of Microsoft's documentation on building COM objects for use on their IIS server I came across the following bit of code.

STDMETHODIMP CHelloWorld::GetResponse()
{
    // Get the Object Context
    CComPtr<IObjectContext> pObjContext;
    HRESULT hr = ::GetObjectContext(&pObjContext);

    if (SUCCEEDED(hr))
    {   
        // Get the Properties interface
        CComPtr<IGetContextProperties> pProps;

        hr = pObjContext->QueryInterface(IID_IGetContextProperties, (void**) &pProps);

        if (SUCCEEDED(hr))
        {
            // Get the ASP Response object
            CComBSTR bstrResponse("Response");
            CComVariant vt;

            hr = pProps->GetProperty(bstrResponse, &vt);

            if (SUCCEEDED(hr))
            {
                // Convert the IDispatch pointer to an IResponse pointer
                if (V_VT(&vt) == VT_DISPATCH)
                {
                    CComPtr<IResponse> piResponse;
                    IDispatch *pDispatch = V_DISPATCH(&vt);

                    if (NULL != pDispatch)
                        hr = pDispatch->QueryInterface(IID_IResponse, (void**) &piResponse);

                    if (SUCCEEDED(hr))
                        piResponse->Write(CComVariant(OLESTR("Hello, World!")));
                }
            }
        }
    }

    return hr;
}

As sample code I have a few problems with this. The first is its sheer ugliness. The way that Microsoft writes C++ to look like C is barbarous! The nested if statements for each attempt to use a method on the COM interfaces is horrid and very error prone and of course has led to a fatal bug in the above code.

CComPtr<IResponse> piResponse;
IDispatch *pDispatch = V_DISPATCH(&vt);

if (NULL != pDispatch)
    hr = pDispatch->QueryInterface(IID_IResponse, (void**) &

if (SUCCEEDED(hr))
    piResponse->Write(CComVariant(OLESTR("Hello, World!")));

At the innermost if the code (above) fetches the Response object. It correctly checks to make sure that the pointer isn't NULL before trying to convert the IDispatch pointer to an IRequest pointer. After this hr is checked before using it.

The problem here is that if the IDispatch is NULL then the hr is never set by the line that fetches the IRequest pointer. The next line will always have a successful hr value because it comes from the previous success (already checked) and not from the QueryInterface call. The next line that sends Hello world to the browser will run on a NULL pointer and crash the program.

On older windows this would have caused the dreaded General Protection Fault. Running this on a current version of IIS will probably just force it to restart the worker (more because IIS is so resilient to errors these days, one supposes because of errors like this all over web code).

So long as the provided function is used exactly as described everything will be fine. However, if that is the case then nearly every if statement is superfluous. If we consider the other if statements important then we can't gloss over the failure on this one.

How often is this failure likely to occur? Hardly ever. But, if Microsoft's code is full of this sort of error then it's no wonder than software failures occur on a regular basis.

Why?

What is it about the code that caused the author to make this mistake? We can be pretty sure that he didn't do it on purpose.

The answer is clear from just looking at it. The constant requirement to indent this code for each error check is the prime cause. For this we cannot blame the programmer who wrote it as the code structure that is forced upon him is completely against nature.

Our nature is to think of things in terms of "Do this, then that, then the other". This is how we think and how we would expect to see the code laid out. An even worse programming error would generate something that looked like this:

STDMETHODIMP CHelloWorld::GetResponse()
{
    // Get the Object Context
    CComPtr<IObjectContext> pObjContext;
    ::GetObjectContext(&pObjContext);

    // Get the Properties interface
    CComPtr<IGetContextProperties> pProps;

    pObjContext->QueryInterface(IID_IGetContextProperties, (void**) &pProps);

    // Get the ASP Response object
    CComBSTR bstrResponse("Response");
    CComVariant vt;

    pProps->GetProperty(bstrResponse, &vt);

    // Convert the IDispatch pointer to an IResponse pointer

    CComPtr<IResponse> piResponse;
    IDispatch *pDispatch = V_DISPATCH(&vt);

    pDispatch->QueryInterface(IID_IResponse, (void**) &piResponse);

    piResponse->Write(CComVariant(OLESTR("Hello, World!")));

    return S_OK;
}

Now we've just thrown out each and every error check that we should have. Despite the likely hood of blowing up the programme completely it does more accurately show us the steps that we wanted to use and is much closer to what our intuition tells us the code should look like.

The reason why we can't do this is that Microsoft decided to use a C style mechanism for error handling in their COM code. We can't entirely blame them though as they wanted programmers to be able to write COM objects in C as well as in C++. This means that we cannot make direct use of C++'s very elegant exception mechanism that is designed to handle exactly these sorts of situations. COM's C lagacy stops exceptions from being used properly to handle this otherwise we could just leave the code as it is above.

But just because COM doesn't allow us to use exceptions doesn't mean that we can't use exception handling to make the first bit of code look more like our preferred solution and keep the errors checking in.

The solution is fairly straightforward. We start by putting some exception handling in the example:

STDMETHODIMP CHelloWorld::GetResponse()
{
    try {
 
       // Function code goes here

         return S_OK;
    } catch ( … ) {
        return E_FAIL;
    }
}

This doesn't help us directly as there is no way to turn on automatic exceptions for the COM calls but we can do it ourselves. Microsoft clearly already knows what to do as I found the following class in some example code for Windows Scripting Hosts.

struct HRESULT_EXCEPTION 
 {
     HRESULT_EXCEPTION(HRESULT hr)
     { if (FAILED(hr)) throw hr; }
 
     HRESULT operator = (HRESULT hr) 
     { if (FAILED(hr)) throw hr; return hr; }
 };

This class has a very simple task. When provided with a HRESULT it checks to see if it is from a failed call. If so then it throws the HRESULT as an exception. We can use this as below:

HRESULT_EXCEPTION HR( ::GetObjectContext(&pObjContext) );

Now if the call to GetObjectContext fails then we can just catch the exception and perform any error handling that may be required.

It's important to realise one other aspect of the code that makes this work for us and that is the use of the ATL smart pointers. These save us from worrying about freeing any COM interfaces that we may be holding.

We can now alter our exception handler to catch this:

STDMETHODIMP CHelloWorld::GetResponse()
{
    try {
 
       // Function code goes here

         return S_OK;
    } catch ( HRESULT error ) {
        return error;
    } catch ( … ) {
        return E_FAIL;
    }
}

Now we get some slightly nicer error handling and we still get the correct error number returned to the function user.

Some real code

Here are some code snippets from FOST.3™. These have been through the mill and work properly.

Below is a class declaration for a more useful version of the COM error checker.

extern class F3UTIL_DECLSPEC ComHR {
public:
    ComHR( void ) throw () {}
    inline ComHR( HRESULT hr, size_t line, const char *file );

    HRESULT operator =( HRESULT hr ) const { check( hr ); return hr; }
    HRESULT operator()( HRESULT hr ) const { check( hr ); return hr; }

    static wstring format( HRESULT hr );

private:
    void check( HRESULT hr ) const { if ( FAILED( hr ) ) doThrow( hr ); }
    void doThrow( HRESULT hr ) const;
} HR;

Its only job is to check HRESULT return values and throw them as exceptions. It also includes an option for formatting the error code and there is a global instance that can be used too. I have this in the FOST.3™ FSLib namespace.

The implementation is also pretty simple¹ [1The format function is not a good implementation. It's only really an illustration.].

FSLib::ComHR HR;


inline
FSLib::ComHR::ComHR( HRESULT hr, size_t line, const char *file ) {
    try {
        check( hr );
    } catch ( FSLib::Exceptions::Exception &e ) {
        e.info() << widen( file ) << L":" << toString( line ) << std::endl;
        throw;
    }
}


void FSLib::ComHR::doThrow( HRESULT hr ) const {
    throw FSLib::Exceptions::ComError( L"COM call failed - bad HRESULT (" + format( hr ) + L")" );
}


FSLib::wstring FSLib::ComHR::format( HRESULT hr ) {
    switch ( hr ) {
    case CLASS_E_NOAGGREGATION:
        return L"CLASS_E_NOAGGREGATION";
    case E_NOINTERFACE:
        return L"E_NOINTERFACE";
    case REGDB_E_CLASSNOTREG:
        return L"REGDB_E_CLASSNOTREG";
    default:
        return toString( hr );
    }
}

The constructor here takes a file and line number reference and adds them to the exception information. This makes use of some internal FOST.3™ exception classes which you should replace with your own.

The function FSLib::ComHR::format should also be extended for more of the known error numbers. I don't know the Windows APIs well enough to know if there is a function that will do this formatting for me.

In order to use exceptions in C++ and not propogate them to the COM host application we have to put exception handlers within every COM method. I use a couple of #defines for this:

#define FSL_TRY FSLib::Exceptions::StructuredHandler handler; try {

#define FSL_CATCH_ONLY } \
        catch( exception &e ) { Error( e.what() ); return E_FAIL; } \
        catch ( _com_error &c ) { Error( FSLib::Exceptions::ComError( c ).what() ); return E_FAIL; }
#define FSL_CATCH return S_OK; FSL_CATCH_ONLY

#define FSL_CATCH_ONLY_NOCOCLASS } \
        catch( exception & ) { return E_FAIL; } \
        catch ( _com_error & ) { return E_FAIL; }
#define FSL_CATCH_NOCOCLASS return S_OK; FSL_CATCH_ONLY_NOCOCLASS

• FSL_TRY is always used.
• FSL_CATCH_ONLY is used where the COM implementation already has a return statement dealing with the successs code.
• FSL_CATCH is used where getting to the end of the implementation without triggering an exception always indicates success.
• FSL_CATCH_ONLY_NOCOCLASS is used in the same way that FSL_FSL_CATCH_ONLY is used, but where the COM object has no coclass (and hence we can't register an error string).
• FSL_CATCH_NOCOCLASS is then the equivalent of FSL_CATCH where there is no coclass.

Here is a sample function that makes use of the class² [2Early versions of this article had a bug in it. The QueryInterface and GetTypeInfo didn't check that unknown and typeinfo where valid pointers. This shouldn't cause a problem as the Microsoft implementation that calls this never sends incorrect pointers with the wrong mask. Adding the check does no harm though.]:

STDMETHODIMP FSLibScriptSiteCoClass::GetItemInfo(
        LPCOLESTR name,
        DWORD mask,
        IUnknown **unknown,
        ITypeInfo **typeinfo ) {
    FSL_TRY
    if ( unknown ) *unknown = NULL;
    if ( typeinfo ) *typeinfo = NULL;
    for ( t_globals::const_iterator it( globals.begin() ); it != globals.end(); ++it ) {
        if ( name == (*it).first ) {
            if ( ( mask & SCRIPTINFO_IUNKNOWN ) && unknown )
                FSLib::ComHR( (*it).second->QueryInterface( unknown ), __LINE__, __FILE__ );
            if ( ( mask & SCRIPTINFO_ITYPEINFO ) && typeinfo )
                FSLib::ComHR( (*it).second->GetTypeInfo( 0, locale, typeinfo ), __LINE__, __FILE__ );
            return S_OK;
        }
    }
    return E_FAIL;
    FSL_CATCH_ONLY
}

The function is part of the implementation of the scripting host that can be used by FOST.3™ applications.