Throwing error is potentially buggy

Note: This thread is from comp.lang.c++.moderated, moved here
           because I failed twice to reply. Please refer the original
          thread for context.

On 6=E6=9C=888=E6=97=A5, =E4=B8=8B=E5=8D=885=E6=99=8255=E5=88=86, Goran <go=
ran.pu...@gmail.com> wrote:


"


I am just saying one thing that "throwing error is potentially buggy".
Not that it is convenient for an error string to be written to a log
file, nor that it is elegant to mean stack unwind to exit. I am not
even really talking about simplicity neither, even whether it is an
real error or not, but merely correctness of the error reporting
mechanism. Again, I am not talking about any implement of your or his
or her correct throwing strategy, but the prevailing, the frequently
taught, probably the 'standard' one.

Let's first confine errors to only errno-like classes, assume they
all inherit from std::exception.
class E_PERM;
class E_NOENT;
class E_SRCH;
class E_INTR;
class E_IO;
class E_BADF;
class E_AGAIN;
class E_NOMEM;
class E_NOSPC;
class E_INVAL;
class E_PIPE;
class E_TIMEDOUT;
class E_NAMETOOLONG;
class E_FBIG;
class E_LOOP;
.....

// Class String has all similar members as std::string
// except throwing E_... class
//
class String {
 // Take one for instance:
 // Throw: E_INVAL $index is invalid
 // E_FBIG String length too long
 // E_NOMEM Not enough memory
 //
 void insert(size_t index, const String& str);
};

Now we are writting a funcion process_msg. All called functions throw
E_... in exceptional condition.

// Throw: E_INVAL $in is invalid
// E_FBIG result string is too long
// E_NOMEM not enough memory
//
void process_msg(const String& in, String& out);
{
 MutexLock lock(mutex); // Not likely to fail. But shouldn't
                                              // a robust program
catch the possible
                                              // E_INVAL, E_PERM,
E_AGAIN from leak?
 String s1,s2;
 if(in.size()<=0) {
   throw E_INVAL;
 }
 db.read(s1,1000); // may throw E_INVAL, E_FBIG, E_NOMEM,...
 parse(s1,s2); // may throw E_INVAL, E_FBIG, E_NOMEM,...
 process(s1,s2,out); // may throw E_INVAL, E_FBIG, E_NOMEM,...
}'

Then, a tiny part of the loop in the server-like program:

try { process_msg(in,out); }
catch(E_INVAL& e) {
  send("Invalid argument",e ); // peer end could have a wrong message
  // Probably one want to log message here, but there are more story
  // about stack unwinding that can occur here. If error is
  // misinterpreted, the program goes to std::terminate for the good.
  // The error report is not just for logging purpose.
  // This is one reason a function should be somewhat responsible
  // to its declared thrown objects.
}
catch(E_FBIG& e) {
  send("Result too long",e); // peer end could have a wrong message
}

This pattern has been explained several times. It should be clear
that at least all process_msg(..) documented errors should be caught
(probably convert to another one) in the function process_msg(..).
The prevailing "catch only those you know how to handle" style and
the "rethrow error" preaches won't work well in such error handling
or message passing scenario.

So the revised version (not the popular style):

void process_msg(const String& in, String& out)
try {
 MutexLock lock(mutex); // Note: E_INVAL, E_PERM, E_AGAIN might be
                                             // thrown here, but no
way for a local
                                             // catch. So we added a
try/catch layer.
 String s1,s2; // E_NOMEM might be thrown
 if(in.size()<=0) {
   throw E_INVAL; // this is the responsible one
 }

 try { db.read(s1,1000); }
 catch(E_INVAL&) {
   // What to throw? process_msg(..) only documented 3 errors.
   // Do we need more errors to report? Yes, so add E_IO to mean
   // vague error.
   throw E_IO;
 }
 catch(E_INTR&) {
   // Let this be handled correctly
 }
 catch(E_IO&) {
   // E_IO had been decided for a vague error. So this error
   // can't be directly reported.
   throw E_IO;
 }
 catch(E_PERM&) {
   // process_msg(..) intends to hide the implement detail that it
   // reads data from a database or config file. So throw E_IO.
   throw E_IO;
 }
 catch(E_FBIG&) {
   // To be defensive. Not knowing how 'db.read(..)' will actually be
   throw E_IO;
 };
 // There are other possible thrown type by definition.

 s1+=in; // Lucky, E_FBIG is appropriate

 try { parse(s1,s2); }
 catch(E_INVAL&) {
   throw E_IO;
 }
 catch(E_NOENT&) {
   throw E_IO;
 }
 catch(E_FBIG&) {
   throw E_IO;
 };
 // There are other possible thrown type by definition.

 try { process(s1,s2,out); }
 catch(E_INVAL&) {
   throw E_IO;
 }
 catch(E_RANGE&) {
   throw E_IO;
 }
 catch(E_BADMSG&) {
   throw E_IO;
 }
 catch(E_FBIG&) {
   throw E_IO;
 };
 // There are other possible thrown type by definition.

 // Can the above annoying catch handlers be saved?
 // My answer is no.
}
catch(E_INVAL&) {
 throw; // *** BUG ***
                 //E_INVAL may be from the lock failure. There
                // are occasionally similar codes causing such
                // conflicts.
};

See, it is not easy for process_msg(..) to be merely responsible for
those documented throw types. To add to this burden, this style
require all functions of the program to follow this coding standard.

Note: Some approaches use specific name space or throw classes, but
they basically only reduce the chance of being misinterpreted, with
lots of unbalanced costs. Still, not the final solution.