<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
  <meta content="text/html;charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Just a small reply :)<br>
<blockquote cite="mid:200906101802.12855.dgarcia@iua.upf.edu"
 type="cite">
  <pre wrap="">On Wednesday 10 June 2009 16:13:52 dirk.griffioen wrote:
  </pre>
  <blockquote type="cite">
    <pre wrap="">Hi,

David and me got into a discussion yesterday on howto deal with
errorconditions in CLAM with respect to reporting 'the right thing' to
the user. As David mentioned earlier, should we use errorcodes, typed
exceptions, diagnosis functions or ...

At the moment the codebase has different styles: (boolean) returns,
asserts (that may register their own handler), throw or even exit():

        if ( jack_port_unregister ( _jackClient, it->jackPort) )
        {
            std::cerr << "JACK ERROR: unregistering port " <<
it->PortName() << std::endl;
            exit(1);
        }


        if(infile->samplerate() != sampleRate)
        {
            std::cout << "All the input audio files have to have the
same sample rate" << std::endl;
            exit(-1);
        }


David made the difference between 'programmer exceptions' (violation of
invariants) which may throw (and possibly terminate) and 'user
exceptions' - things the user did wrong but should not terminate the
program; for instance, giving a wrong file may lead to the samplerate
exit as depicted above.

For 'programmer exceptions' I like the ENFORCE idiom over (clam)assert
or 'if condition throw error' - as it says what it does, without me
interpreting code; but opinions may vary here. Either way, 'programmer
exceptions' are, well, exceptional and should be caught by the main
catchhandler, logged and the program can terminate if it pleases so.

The big question now becomes, what to tell the user? The samplerate
message in the above example will not guide the user in choosing the
correct file.

But maybe if it were more like:

        std::cout << "All the input audio files have to have the same
sample rate (did you open audiofiles?)" << std::endl;

this would give the user a hint ...

David suggested that lower level (user) exceptions should be handled by
the higher level, possibly translating them in something more
meaningfull (David, please correct me if I quote you wrong). The problem
I see with that is probably will get a large table of 'translations' and
what to think of the other causes samplerates do not match (corrupted
file, ...)

I would be all for a simple scheme: state your error and possibly hint
at a cause, throw and handle it as high as possible.
    </pre>
  </blockquote>
  <pre wrap=""><!---->
Let's put an actual use case in clam on why context is important on reporting.

An xml parser detects an error it knows the line and position and what it 
expected. At this level: "unexpected < symbol in line 12 of file a.xml". This 
is the message in an thrown StorageErr that is catch by the (api) user.

The xml parser is used to load the path of a 3d object file whose file name is 
stored in a ProcessingConfig and the load is done in the processing's 
ConcreteConfigure. At this level "XML error while loading object's path: 
unexpected < symbol in line 12 on file a.xml". If the ConcreteConfigure do 
not catch that error is a programmer error, because the upper levels don't 
know that this processing is actually reading XML's.

A configuration error itself is not always an error the (human) user must be 
reported. In fact many processings remain silently unconfigured until we 
configure them. If you start a processing that is unconfigured it is a 
programmer error (assert failed) because the programmer should check first 
that it is configured. If processing user code wants to report the (human) 
user at this level, it should query the configuration error message.

Often that processing is in a network, maybe containing several equal 
processings that may release the same exception. The processing has no 
information of its name in the network but the network knows so when the 
network is to be started i would like to know which are the processings that 
are unconfigured and why. The network provides accessors to collect such 
information.

That network migth be generated in which case, the error is normally a 
programmer error, or might be human user provided in which case the human 
user should be notified. Depending on the interface that could be a dialog or 
a console dump, or maybe we should not report and use a fallback network.

Note that we are using different schemas on reporting, examples are assertions 
for api misusage, api to query status (this is my preferred), exception 
throwing when we want to reach a far caller, like in xml parsing.

Note also that 'api to query status' is not an ErrorCode idiom. We are not 
dealing with errors differently we are just dealing with them.


  </pre>
</blockquote>
Ok, so if I read this correctly you're saying 'handle as needed'.<br>
<br>
Then, like we said yesterday - it may be best to go over concrete
situations and decide what needs to be done ...<br>
<blockquote cite="mid:200906101802.12855.dgarcia@iua.upf.edu"
 type="cite">
  <pre wrap=""></pre>
  <blockquote type="cite">
    <pre wrap="">Secondly, I have a practical question: what kind of exception should I
throw? Do I need many types or could I do with 2, one for the progammer
kind and one for the user kind (please note, I am thinking aloud here).

          class ClamUserException : Err {}
        class JackException : ClamUserException {}

        if(infile->samplerate() != sampleRate)
            throw JackException("All the input audio files have to have
the same sample rate");

where the userexception may be shown to the user at some point ...
    </pre>
  </blockquote>
  <pre wrap=""><!---->
This is not for sure as in the previous example. Indeed Assertions migth be 
shown to the user so if the application wants to get reports.

  </pre>
  <blockquote type="cite">
    <pre wrap="">But I also understood this has been discussed out already - so I'd like
to know what the findings where in order to implement them to get rid of
these exit()'s ...
    </pre>
  </blockquote>
  <pre wrap=""><!---->
Again, yes, but sure we have needs that are not covered such as in-process and 
backend errors. So, let's concentrate to solve the new problems. If they can 
be solved with the old idioms, ok, if not, let's introduce a new one.

  </pre>
</blockquote>
If I may, I would like to advocate a more clear position, a bit after
the 'guru of the week' article I mentioned earlier.<br>
- exit: may not be used<br>
- asserts: used locally (when the caller is in control of parameters,
which should be correct to the callee)<br>
- exceptions: to pass errors upwards <br>
- return codes: for instance on api level (say after a catch internally)<br>
<blockquote cite="mid:200906101802.12855.dgarcia@iua.upf.edu"
 type="cite">
  <pre wrap="">
  </pre>
  <blockquote type="cite">
    <pre wrap="">ENFORCE(infile->samplerate() == sampleRate) << JackException << "All the
input audio files have to have the same sample rate";
    </pre>
  </blockquote>
  <pre wrap=""><!---->
If we can just use c++ without adding more macros:

if (infile->samplerate() == sampleRate)  
        throw JackException() 
                << "All the input audio files have to have the same sample rate "
                << "Given: " << infile->samplerate() << " and " samplerate;

Throws, like returns are markers of an (early) exit point of the method.
Macros hide that but are usefull when we want to encode the source file and 
line number.



  </pre>
</blockquote>
Ok - that's a valid remark; but the reason I would choose the macro
over the lines that clearly states the invariant is violated, enforce
enforces invariants, if checks conditions. These are different things.<br>
<br>
But I guess we will not see easily agree on this one :)<br>
<br>
Dirk
</body>
</html>