5

My COM server implemented in Visual C++ uses a ton of other C++ code. That other C++ code sometimes wraps code in __try-__except and translates structured exceptions into custom C++ exceptions. This part I cannot change.

No method of my COM server should let those exceptions propagate through the COM boundary so it has to catch and translate them into HRESULTs. Those custom C++ exceptions contain the original error code which was obtained during translation - it's something like EXCEPTION_ACCESS_VIOLATION. The question is how I craft an appropriate HRESULT value so that the client has as much information as possible as to what happened (and perhaps decide to restart the server (and itself in case of inproc) after seeing an access violation).

Suppose it was EXCEPTION_ACCESS_VIOLATION which is defined in WinBase.h

#define EXCEPTION_ACCESS_VIOLATION STATUS_ACCESS_VIOLATION

and the latter is defined in WinNT.h

#define STATUS_ACCESS_VIOLATION ((DWORD)0xC0000005L)

I could use HRESULT_FROM_WIN32() to translate that code into HRESULT assuming that it was a Win32 error in the first place.

Do I use HRESULT_FROM_WIN32() here or do I use any other way to do the translation?

sharptooth
  • 167,383
  • 100
  • 513
  • 979
  • Maybe using ``IErrorInfo`` infrastructure can come in handy here. ``_com_error`` holds a pointer to it. On the callee side, I think I remember there was something like SetErrorInfo ... I used ATL back then... *cough* Was it 10 years ago? – BitTickler Apr 23 '15 at 11:31
  • @user2225104 Even with `IErrorInfo` I'm responsible for crafting an `HRESULT` value. – sharptooth Apr 23 '15 at 11:39
  • 0xC0000005 is already a valid HRESULT code that indicates failure, no reason to change it. Don't hide something this nasty. – Hans Passant Apr 23 '15 at 11:58
  • @HansPassant I checked - `HRESULT_FROM_WIN32()` converts `0xC0000005` into itself. I guess this means we can just use it everywhere and get "proper" values. – sharptooth Apr 23 '15 at 12:01
  • Wouldn't `HRESULT_FROM_NT()` be more appropriate for this value? Assuming that exception code == NT status code, anyway... – andlabs Apr 23 '15 at 12:11
  • I know it is a matter of taste, but... usually callers used the pattern ``if( FAILED(hr) ) { // error handling code }``. Whether that error handling code now is hand written with a switch - case statement, looking for specific HRESULT values or if they use ``GetErrorInfo()`` and draw conclusions from that information is not really crucial, given that 99.9% of the time there is no specific handler code written - ever. ``AtlSetErrorInfo(...,E_FAIL,...)`` allows to diagnose/log the problem, without opening a maintenance nightmare. – BitTickler Apr 23 '15 at 12:19
  • @user2225104 Okay, you see that it `FAILED`. What do you do? Perhaps you throw an exception which bubbles up to some reasonable layer and that layer has to *do something*. That layer might easily want to treat different `HRESULT` differently. – sharptooth Apr 23 '15 at 12:25
  • @sharptooth Every extra error return code you add, you have to maintain, test, document, ... it becomes part of the function contract. And as that of the interface contract. The fun thing about interfaces is that they can have different implementations. To over-specify the "error contract" will bring more harm than good. ``AtlSetErrorInfo`` allows to give next to the HRESULT: a helpId, a helpFileId, a description string, ... which if used wisely is worth more than a (potentially) custom HRESULT value which cannot even be found in "error lookup" tool. – BitTickler Apr 23 '15 at 12:30
  • 2
    @sharptooth Or another way to explain my point: If you as the implementer of that function which calls that C++ code do not know how to handle that structured exception, it is even more likely that the caller does not know either. For all that matters he does not even know/care you do use that C++ code for your implementation. Those above that direct caller will care even less. As you move up the call tree, you should move up with error abstraction, too. Then, the only info they need on that higher level is "my feature/functionality failed and the reason is that COM object xy." – BitTickler Apr 23 '15 at 12:42

1 Answers1

4

You are supposed to return HRESULT code, where you choose appropriate code to indicate status of operation. It does not have to be a failure code, but you typically want to show something that satisfies FAILED(...) macro, e.g. E_FAIL, or DISP_E_EXCEPTION or HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION).

It is highly unlikely that callers compare against specific exception-related HRESULT, so specific failure code makes sense rather for diagnostic. Also, as you complete handling the exception before exiting from COM method, there is no need to return specific HRESULT code because no additional actions are required or necessary.

To provide additional information, it is possible to use ISupportErrorInfo, IErrorInfo and friends. Callers can retrieve free text description and many popular environments to this automatically, so for example .NET caller will have this additional information on exception message rather than standard message generated from HRESULT code.

ATL offers AtlReportError to wrap SetErrorInfo API, which also suggests on generating HRESULT code:

... If hRes is zero, then the first four versions of AtlReportError return DISP_E_EXCEPTION. The last two versions return the result of the macro MAKE_HRESULT( 1, FACILITY_ITF, nID ).

Roman R.
  • 68,205
  • 6
  • 94
  • 158
  • I think the OP is unsure, whether or not the `HRESULT` passed across the interface should represent the root cause. Maybe you could stress, that there is generally no need to carry that specific information across interface boundaries. Your first two paragraphs state this already, but spelling it out explicitly may help. – IInspectable Apr 23 '15 at 11:45
  • 1
    @IInspectable The root cause may be of great interest to the caller - if there was a structured exception the server might get toast and the client better restart the server (and itself if it was an in-proc server). – sharptooth Apr 23 '15 at 11:52
  • @IInspectable: There is no uncertainty about passing exception across boundaries here. It is rather about what can/should be returned standard using COM strategies. COM, however, only obliges to handle the exceptions and return HRESULT codes, thus leaving clients/servers to define server specific code to indicate fatal error. – Roman R. Apr 23 '15 at 11:56
  • 1
    @sharptooth: Generally, the only information that is of interest to the caller is, whether an operation succeeded or failed, as indicated by the MSB of the `HRESULT`. The root cause is not interesting to the caller. It is, however, helpful to a support engineer. One way to deal with this is to write a minidump in your exception handler (for diagnostics), and pass *some* failure code back to the caller. – IInspectable Apr 23 '15 at 12:58