A matter of exception reporting style

[mzdude]

During a code review the other day I noticed we have some
code that could be refactored.

Err.h

struct Error {
int errCode;
std::wstring reason;

Error(int e, wchar_t const *r )
: errCode(e)
, reason(r)
{}

};

const int ErrCode1 = 1000;
const int Errcode2 = 1001;
....


File1.cpp
void SomeFunct() {
if(cond)
throw Error( ErrCode1, L"This is an error");
}


File2.cpp
void AnotherFunc() {
if(cond)
throw Error( ErrCode1, L"This is an Error.");
}

Note the slight discrepancy in the error strings.
Obviously refactoring is the key to consistency.
But how to refactor?

In Err.h

// option 1, function returns object
Error Error_Code1()
{ return Error(ErrCode1, L"This is an error"); }

// option 2, single definition of a static object
// in this case all ErrCodeX's go away
extern Error Error_Code1;

// option 3, function actually does the throw
void ThrowError_Code1()
{ throw Error(ErrCode1, L"This is an error"); }


void SomeFunct() {
if(cond)
throw Error_Code1();

if(cond)
throw Error_Code1;

if(cond)
ThrowError_Code1();

My current preference is leaning towards option 1 for
several reason. No time wasted in constructing lots
of exceptions at start up time (as compared to option 2).
Option 3 hides what is happening when tracing through
the higher level code. It is only the function name that
makes it obvious.

In the future, this code **may** get internationalized,
in which case the string would move to a resource file.

 

[Alf P. Steinbach]

* mzdude:

During a code review the other day I noticed we have some
code that could be refactored.

Err.h

struct Error {
int errCode;
std::wstring reason;

Error(int e, wchar_t const *r )
: errCode(e)
, reason(r)
{}

};

Should derive from std::runtime_error.

When you do introduce your own exception class it's also a Good Idea to add a
virtual rethrow() routine.

That way exceptions can more easily be propagated across C code, e.g. from
callbacks.

const int ErrCode1 = 1000;
const int Errcode2 = 1001;
...


File1.cpp
void SomeFunct() {
if(cond)
throw Error( ErrCode1, L"This is an error");
}


File2.cpp
void AnotherFunc() {
if(cond)
throw Error( ErrCode1, L"This is an Error.");
}

Note the slight discrepancy in the error strings.
Obviously refactoring is the key to consistency.
But how to refactor?

Introduce a mapping from error codes to strings.

Take care not to make that error code/string the primary message (as it seems to
be above), but just a convenience add-on.

For in order to avoid an administrative and maintainance nightmare you
absolutely don't want to force all exception usage to use predefined strings.


Cheers & hth.,

- Alf

 

[Pascal J. Bourguignon]

During a code review the other day I noticed we have some
code that could be refactored.

Err.h

struct Error {
int errCode;
std::wstring reason;

Error(int e, wchar_t const *r )
: errCode(e)
, reason(r)
{}

};

const int ErrCode1 = 1000;
const int Errcode2 = 1001;
...


File1.cpp
void SomeFunct() {
if(cond1)
throw Error( ErrCode1, L"This is an error");

}


File2.cpp
void AnotherFunc() {
if(cond2)
throw Error( ErrCode1, L"This is an Error.");

}

Note the slight discrepancy in the error strings.

ISTM that the problem is not the slight discrepancy in the strings,
but their close ressemblance, assuming the two conditions that raised
the error ErrCode1 are different!

Obviously refactoring is the key to consistency.
But how to refactor?

Actually, one problem is in the use of enums.

I understand that for external references it might be useful to have
error codes, and that small integers might be practical for such error
codes. But internally, you shouldn't have to deal with them.

class ErrorConditionOne: public Error { ErrorConditionOne():errCode(1001){}; };
class ErrorConditionTwo: public Error { ErrorConditionOne():errCode(1002){}; };

You might want to document here the meaning of ErrorConditionOne, etc,
so you can build the user documentation error-code annex
automatically:

class ErrorConditionOne: public Error { ErrorConditionOne():errCode(1001),errExplain("<Condition One Occured>"){}; };
class ErrorConditionTwo: public Error { ErrorConditionOne():errCode(1002),errExplain("<Condition Two Occured>"){}; };

But this text has nothing to do with the actual reason why condition one occured...

void funA()
if(conditionOne){
throw ErrorConditionOne("When doing <funA stuff> we expected not <conditionOne>.");
}
}

void funB()
if(conditionTwo){
throw ErrorConditionTwo("When doing <funB stuff> we expected not <conditionTwo>.");
}
}

In the future, this code **may** get internationalized,
in which case the string would move to a resource file.

In any case, almost all strings are to be internationalized. This is an orthogonal problem.

 

[mzdude]

Should derive from std::runtime_error.

Why?? We don't use narrow char unless forced to for historical
interface reasons.

When you do introduce your own exception class it's also a Good Idea to add a
virtual rethrow() routine.

That way exceptions can more easily be propagated across C code, e.g. from
callbacks.

Not sure what you mean by this.

Introduce a mapping from error codes to strings.

Take care not to make that error code/string the primary message (as it seems to
be above), but just a convenience add-on.

That is the case. The error code is the important. The message is for
the human (maybe).

 

[mzdude]

ISTM that the problem  is not the slight discrepancy in the strings,
but their close ressemblance, assuming the two conditions that raised
the error ErrCode1 are different!

No actually the conditions are identical.

Actually, one problem is in the use of enums. I don't see it that way.

I understand that for external references it might be useful to have
error codes, and that small integers might be practical for such error
codes.  But internally, you shouldn't have to deal with them.

This particular mechanism is for a progromatic interface. Through a
published API a calling program has violated the contract. We are
returning the results. In this case the results will be formulated
into XML (not that it's terribly relevant).
<Result Status="Fail" ErrId="100">Reason for failure goes here</
Result>

The calling program can take corrective measures based on the
error code more easily than it can for text.

 

[Alf P. Steinbach]

* mzdude:

Why??

Multiple exclamation marks, ... (I'm sure you can look up that quote)

Anyways, you want to base your exceptions on std::exception in order to support
code that deals with and expects std::exception, and then std::runtime_error is
the logical general choice of standard exception base class.

There's nothing more annoying than using libraries that throw non-standard
exceptions, necessitating exception conversion or big multiple catches in the
calling code.

We don't use narrow char unless forced to for historical
interface reasons.

For exceptions you should.

There is AFAIK no reason to support wide characters in exceptions.

The same internal failure can cause different user level failures; different
internal failures can cause the same user level failure; and the requirements on
information conveyed, internationalization etc. are vastly different.

So user interface messages should ideally not be carried by exceptions.

And so in a non-trivial app it's a good idea to separate exceptions (internal
failure reporting) from the user interface aspect (user failure reporting).

Not sure what you mean by this.

It means that before returning to C code that was called by your code, you can
dynamically copy the exception somewhere (for complete generality it helps if it
supports cloning), and on return from C code you can rethrow the exception with
original type.

For the C code in between may not support exception propagation.

That is the case. The error code is the important. The message is for
the human (maybe).

If you make the error code the important, then you're most probably heading for
the nightmare I mentioned.

At the worst you'll end up with a repository/database of error codes and
strings, a set of small utilities for handling that, a person in charge of that,
and umpteen man-months wasted searching for proper error codes or creating them.

Generally all that matters for code that handles an exception is that there was
an exception, and all that matters for someone inspecting a log is the textual
message and throw context. I use exceptions that can carry a Windows error code.
However, even when an exception is caused by an API failure there isn't
necessarily a reasonable error code available, or the error code can be so
generic as to not say anything really (e.g. E_FAIL), and it doesn't say which
API routine failed, nor which of my routines failed. So the important thing for
debugging, logging and so on is the text, supplied by the throwing code.


Cheers & hth.,

- Alf

 

[Alf P. Steinbach]

* mzdude:

No actually the conditions are identical.
This particular mechanism is for a progromatic interface. Through a
published API a calling program has violated the contract. We are
returning the results. In this case the results will be formulated
into XML (not that it's terribly relevant).
<Result Status="Fail" ErrId="100">Reason for failure goes here</
Result>

The calling program can take corrective measures based on the
error code more easily than it can for text.

It can even more easily distinguish exception types. ;-)


Cheers & hth.,

- Alf

 

[Thomas J. Gritzan]

It can even more easily distinguish exception types. ;-)

How to get the type of exception into XML form? For external programs,
C++ classes don't have any meaning.

But instead of the (magic) numeric constant, the XML could contain an
error code string like ErrId="ErrCode1".

 

[Alf P. Steinbach]

* Thomas J. Gritzan:

How to get the type of exception into XML form?

That depends on the usage context. E.g. for XML as the serialization form of
remote procedure calls one would presumably need to conform to that. But the
basics is not more difficult than the base exception class providing a virtual
routine that supplies the class identifier.

For external programs,
C++ classes don't have any meaning.

The OP's original problem description used examples with 'throw ...', and the
new details supplied above still talk about a calling program.

There was no XML conversion.

A problem about exception class design is not a problem of XML conversion.

But instead of the (magic) numeric constant, the XML could contain an
error code string like ErrId="ErrCode1".

Yeah.


Cheers,

- Alf

 

[mzdude]

* mzdude:


It can even more easily distinguish exception types. ;-)

Not when the calling program can be written in any language.
They may not even have types. :^)

The exception is caught by my interface logic. It is then
converted to the proper format and marshalled as string
data between the two processes.

 

[mzdude]

But instead of the (magic) numeric constant, the XML could contain an
error code string like ErrId="ErrCode1".

But then you're back to making the calling program work with strings.
Where they are trying to figure out why "errcode1" != "ErrCode1" !=
"Errcode1" ...
but 1000 always equals 1000 when converted to an integral value.

 

[mzdude]

Rather than have this thread degenerate into
a philosophical debate on exceptions, I'll just
go with option 1.

BTW, I'd love to participate in a general discussion
on the appropriate handling of exceptions.

// option 1, function returns object
Error Error_Code1()
{ return Error(ErrCode1, L"This is an error"); }

Unless someone has convincing arguements not to.

 

[Thomas J. Gritzan]

But then you're back to making the calling program work with strings.
Where they are trying to figure out why "errcode1" != "ErrCode1" !=
"Errcode1" ...
but 1000 always equals 1000 when converted to an integral value.

In your example:

<Result Status="Fail" ErrId="100">

Is Result always Result or can it also be RESULT? Can you write <result
status="FAIL" errid="100">? If the XML is case-insensitive, the data can
be, too.

A string like "ERROR_NO_MEMORY" or "FILE_NOT_FOUND" is more meaningful
than a number like 47 or 80.

 

[Thomas J. Gritzan]

Rather than have this thread degenerate into
a philosophical debate on exceptions, I'll just
go with option 1.

BTW, I'd love to participate in a general discussion
on the appropriate handling of exceptions.

// option 1, function returns object
Error Error_Code1()
{ return Error(ErrCode1, L"This is an error"); }

Unless someone has convincing arguements not to.

How about a ErrorCode to string map?

void throw_Error(int ErrorCode)
{
throw Error(ErrorCode, getErrorDescription(ErrorCode));
}

Start with a std::map or std::binary_search over a list of
ErrorCode<=>Description mappings.

That's easily extensible to locale dependent lookup.

 

[mzdude]

In your example:

<Result Status="Fail" ErrId="100">

Is Result always Result or can it also be RESULT? Can you write <result
status="FAIL" errid="100">? If the XML is case-insensitive, the data can
be, too.

Point taken.

A string like "ERROR_NO_MEMORY" or "FILE_NOT_FOUND" is more meaningful
than a number like 47 or 80.

Only for humans.
<Result Status="Fail" ErrId="100">File (c:\asdf\as.dat) not found</
Result>
is a way better message though.

And I don't even want to discuss the use of "_" vs " " (space).
ErrId="File Not Found"

 

[mzdude]

How about a ErrorCode to string map?

void throw_Error(int ErrorCode)
{
   throw Error(ErrorCode, getErrorDescription(ErrorCode));

}

Start with a std::map or std::binary_search over a list of
ErrorCode<=>Description mappings.

That's easily extensible to locale dependent lookup.

So you think calling the function that specifically
throws is the better option.

As to the mechanism of the map, that falls under
the auspices of constructing everything at once.
To be done ither at start up or as a singleton.

 

[James Kanze]

* mzdude:

[...]

When you do introduce your own exception class it's also a
Good Idea to add a virtual rethrow() routine.

The standard name for this function is raise, and it should be
virtual, in the base class. That way, the function that
ultimately throws the exception doesn't have to worry about
handling every different type. (It should be present and
virtual, in std::exception, but it isn't.)

[...]

Introduce a mapping from error codes to strings.

The real problem is that most strings reporting errors aren't
fixed---they include things like filenames, user names, etc.

 

[James Kanze]

* mzdude:

[...]

For exceptions you should.

There is AFAIK no reason to support wide characters in
exceptions.

It depends...

The same internal failure can cause different user level
failures; different internal failures can cause the same user
level failure; and the requirements on information conveyed,
internationalization etc. are vastly different.

So user interface messages should ideally not be carried by
exceptions.

Totally agreed, but the exception must carry all necessary
information, which might include things like the user name, a
filename, the actual data base request which failed, why the
request failed, etc. And some of these might only be available
as wide character strings. (I've not encountered the problem
myself, since I tend to use UTF-8 for all of my text. But it's
certainly a potential problem, especially under Windows, where
filenames, etc. may be wchar_t strings.)

It means that before returning to C code that was called by
your code, you can dynamically copy the exception somewhere
(for complete generality it helps if it supports cloning), and
on return from C code you can rethrow the exception with
original type.

Just a reminder as well---threading almost always involves C
interfaces at the lowest levels; you'll need to clone and
rethrow the exception if you want to propagate it over a join,
for example.

For the C code in between may not support exception
propagation.

In the case of join, it almost certainly doesn't, since
exception propagation involves stack unwinding, and you're
moving from one stack to another.

If you make the error code the important, then you're most
probably heading for the nightmare I mentioned.

At the worst you'll end up with a repository/database of error
codes and strings, a set of small utilities for handling that,
a person in charge of that, and umpteen man-months wasted
searching for proper error codes or creating them.

Yep. That's life. It is, regretfully, something you can't
really avoid. (But correctly managed, it's not very expensive.)

Generally all that matters for code that handles an exception
is that there was an exception,

Generally isn't always. If a transaction fails because of some
erroneous information the user entered, it's very useful to be
able to point out to him what that information was. When a
server returns an error, it should return a maximum of
information, so that when the GUI client receives the error, it
can e.g. highlight the erroneous fields in the input form,
perhaps even indicating precisely what was wrong with them.

 

[James Kanze]

On Aug 12, 10:01 am, (e-mail address removed) (Pascal J. Bourguignon)
wrote:

No actually the conditions are identical.

I don't see it that way.

This particular mechanism is for a progromatic interface.
Through a published API a calling program has violated the
contract. We are returning the results. In this case the
results will be formulated into XML (not that it's terribly
relevant).
<Result Status="Fail" ErrId="100">Reason for failure goes here</
Result>

The calling program can take corrective measures based on the
error code more easily than it can for text.

If the calling code is likely to want to distinguish between
types of errors, you probably need to make different types of
errors different derived types, so the distinction can be made
directly in the catch.

If the error is going to be propagated, and handled by a program
at the other end, then providing it with a numeric code is a
definite advantage. It still might need additional information,
however, and you should consider some means of providing this as
well. Transmission should generally be as text however, but in
such cases, I'd limit the text to a highly restricted set of
characters, unless the additional information required more.

For logging, it depends on the context. I don't think there are
any absolute rules. But on the server side, I'd recommend
logging before the exception is thrown (perhaps in the
constructor of the exception, but more likely by means of using
a macro to throw the exception---this is also a case where
you'll want Alf's possibility of throwing the exception from
within the exception).

For interactive users, the message should be formatted in the
user interface, using the information in the exception---the
exception itself should not be concerned with what the user
should see, or how the error will be presented to the user (but
it must carry all of the information that might be needed).

 

[James Kanze]

Pete Becker <[email protected]> wrote:

[...]

I have a more pragmatic approach to that: simply don't let
exceptions propagate accros threads. I can't say that I have
encounter a real hard case where it was needed.

Me neither, and that's surely a valid choice for a lot of
applications. But probably not all.