Turning GetLastError() into an exception

I have a Visual Studio 2008 C++ project that uses a Win32Exception class in cases where there is an exceptional error. The Win32Exception class looks like this:

/// defines an exception based on Win32 error codes. The what() function will
/// return a formatted string returned from FormatMessage()
class Win32Exception : public std::runtime_error
{
public:
    Win32Exception() : std::runtime_error( ErrorMessage( &error_code_ ) )
    {
    };

    virtual ~Win32Exception() { };

    /// return the actual error code
    DWORD ErrorC开发者_StackOverflowode() const throw() { return error_code_; };

private:

    static std::string ErrorMessage( DWORD* error_code )
    {
        *error_code = ::GetLastError();

        std::string error_messageA;
        wchar_t* error_messageW = NULL;
        DWORD len = ::FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM | 
                                      FORMAT_MESSAGE_ALLOCATE_BUFFER |
                                      FORMAT_MESSAGE_IGNORE_INSERTS,
                                      NULL,
                                      *error_code,
                                      MAKELANGID( LANG_NEUTRAL, SUBLANG_DEFAULT ),
                                      reinterpret_cast< LPWSTR >( &error_messageW ),
                                      0,
                                      NULL );
        if( NULL != error_messageW )
        {
            // this may generate a C4244 warning. It is safe to ignore.
            std::copy( error_messageW, 
                       error_messageW + len, 
                       std::back_inserter( error_messageA ) );
            ::LocalFree( error_messageW );
        }
        return error_messageA;
    };

    /// error code returned by GetLastError()
    DWORD error_code_;

}; // class Win32Exception

The class works well in the situations it has been used in. What I would like to know is if there are any obvious cases where this will fail that I should be aware of. Any other gotchas, caveats, or general suggestions on improvements are welcome.

Please note that the boost library is not an option for this code.

• This has already done by several people, including yours truly
  • https://github.com/BillyONeal/Instalog/blob/master/LogCommon/Win32Exception.hpp
  • https://github.com/BillyONeal/Instalog/blob/master/LogCommon/Win32Exception.cpp

• Ironically, your code is not exception safe.

  if( NULL != error_messageW )
  {
      // this may generate a C4244 warning. It is safe to ignore.
      std::copy( error_messageW, 
                 error_messageW + len, 
                 std::back_inserter( error_messageA ) );
      ::LocalFree( error_messageW );
  }
  

Note that if the back_inserter causes std::bad_alloc to be thrown, the memory allocated inside FormatMessage is leaked.

• What a coincidence! I use a similar code in all my projects! It is actually a good idea.

• This code is problematic:

      // this may generate a C4244 warning. It is safe to ignore.
      std::copy( error_messageW, 
                 error_messageW + len, 
                 std::back_inserter( error_messageA ) );
  

  It just trancates WCHARs to chars. Your can either use FormatMessageA explicitly to get a message in the current code-page (ok, you can't as you said), or make convention that all your stings are UTF-8 encoded. I chose the later, see this why.

• Error message by itself may be not useful. Capturing the stack trace may be a good idea.

Realize this is old, but at least with VC++ 2015 you can throw a system_error that will do all this with the system_category() function:

try
{
    throw system_error(E_ACCESSDENIED, system_category(), "Failed to write file");
}
catch (exception& ex)
{
    cout << ex.what();
}

This would print: "Failed to write file: Access is denied"

• FormatMessage may itself fail. Some neutral "Unknown error with code %d" might be in order for such case.
• Some error codes are not really errors (ERROR_ALREADY_EXISTS), depending on what user is expecting.
• Some system functions return their own error codes (notable example being SHFileOperation) that you must handle separately. If you want them to be handled, that is.
• Consider having additional information inside exception: where is exception being thrown from (source file and line), what system function caused exception, what were the parameters of the function (at least the identifying ones, like file name, handle value, or some such). Stack trace is also good.

What I would like to know is if there are any obvious cases where this will fail that I should be aware of. Any other gotchas, caveats, or general suggestions on improvements are welcome.

The main I've problem I've had with such message retrieval has been ERROR_SUCCESS. It's rather perplexing when some operation fails, accompanied by error message "The operation succeeded". One wouldn't think that could happen, but it does.

I guess this is a special case of what Dialecticus noted, that "Some error codes are not really errors", but for most of those codes at least the message is generally acceptable.

The second problem is that most Windows system error message have a carriage return + linefeed at the end. It's problematic for insertion of messages into other text, and it breaks the convention for C++ exception messages. So, good idea to remove those chars.

Now, instead of repeating all that others have already noted, a few words about the design.

The ErrorMessage function would much more usable if was made public or moved out of the class, and took the error code by value, instead of taking pointer argument. This is the principle of keeping separate responsibilities separate. Promotes reuse.

The code in ErrorMessage would be more clear and safe and efficient if you used a destructor to deallocate the memory. Then you could also just construct the string directly in the return statement instead of using a copy loop with back inserter.

Cheers & hth.,

I was recently working on a very similar class and after reading this thread tried to make the copying part exception-safe. I introduced a little helper class that does nothing but hold the pointer to the string returned by ::FormatMessage and free it with ::LocalFree in its destructor. Copying, assigning and moving is not allowed, so one cannot get into trouble.

Here is what I came up with in total:

class windows_error {
public:
    windows_error(wchar_t const* what);

    // Getter functions
    unsigned long errorCode() const { return _code; }
    wchar_t const* description() const { return _what; }
    std::wstring errorMessage() const { return _sys_err_msg; }
private:
    unsigned long _code;
    wchar_t const* _what;
    std::wstring _sys_err_msg;
};

// This class outsources the problem of managing the string which
// was allocated with ::LocalAlloc by the ::FormatMessage function.
// This is necessary to make the constructor of windows_error exception-safe.
class LocalAllocHelper {
public:
    LocalAllocHelper(wchar_t* string) : _string(string) { }
    ~LocalAllocHelper() {
        ::LocalFree(_string);
    }

    LocalAllocHelper(LocalAllocHelper const& other) = delete;
    LocalAllocHelper(LocalAllocHelper && other) = delete;
    LocalAllocHelper& operator=(LocalAllocHelper const& other) = delete;
    LocalAllocHelper& operator=(LocalAllocHelper && other) = delete;

private:
    wchar_t* _string;
};

windows_error::windows_error(wchar_t const* what)
    : _code(::GetLastError()),
      _what(what) {
    // Create a temporary pointer to a wide string for the error message
    LPWSTR _temp_msg = 0;
    // Retrieve error message from error code and save the length
    // of the buffer which is being returned. This is needed to 
    // implement the copy and assignment constructor.
    DWORD _buffer_size = ::FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | 
                                          FORMAT_MESSAGE_FROM_SYSTEM |
                                          FORMAT_MESSAGE_IGNORE_INSERTS, 
                                          NULL, _code, 0, _temp_msg, 0, NULL);

    if(_buffer_size) {
        // When calling _sys_err_msg.resize an exception could be thrown therefore
        // the _temp_msg needs to be a managed resource.
        LocalAllocHelper helper(_temp_msg);
        _sys_err_msg.resize(_buffer_size + 1);
        std::copy(_temp_msg, _temp_msg + _buffer_size, _sys_err_msg.begin());
    }
    else {
        _sys_err_msg = std::wstring(L"Unknown error. (FormatMessage failed)");
    }
}

Maybe this will be useful for some of you.