Unexpected Error
How many times have you seen this error? Or "The operation could not be completed." Or "Catastrophic Error"
"Unexpected Error" is the message for E_FAIL. It comes out of code like this:
HRESULT F()
{
// oops, I messed up my resources & this string no longer exists.
if (!LoadString (IDS_Foo))
return E_FAIL;
// do real work here.
}
HRESULT G()
{
HRESULT hr;
if (FAILED (hr = F())
{
return hr;
}
}
The E_FAIL propogates all the way up the call stack, until the top-level caller decides to pop up a dumb message box.
As a developer, it's hard to debug, because you don't know who failed. As a user, it's impossible for you to know why things failed. The worst of all worlds.
We would be better off crashing. At least then a Watson report would be generated & we could fix the issue. (The Watson feature in VS7 has been very effective thsi way).
If we coded with exceptions instead of return values, then it would be easy for the debugger to break on a throw. But we don't.
A similar problem is code that returns S_FALSE, because we typically check for "FAILED (hr)" and S_FALSE won't get caught.
I decided to try to improve the situation in our code a bit. I did a massive search-and-replace from
return E_*
to
{ ASSERT (!"returning E_*"); return E_*; }
and
return hr;
to
{ ASSERT (hr == S_OK); return hr; }
The good news is that the build was successful. The next step is to run our checkin tests & see how it looks. I expect I'll need to back off on a small set of these asserts & the rest will persist. I hope this will allow us to catch & track down bugs more quickly.
This doesn't help with C++ SNAFUs like:
HRESULT F()
{
return FALSE; // not an error :-(
}
int G()
{
return S_FALSE; // not an error :-(
}
The best solution to this problem, IMO, is to move to C#. That's the long term plan.
Comments
- Anonymous
June 11, 2004
"We would be better off crashing. At least then a Watson report would be generated & we could fix the issue."
I would rather get a vague error dialog than lose my work because of a crash.