Message handling

[chipbarnaby]

Eliminate exceptions or make them optional. One idea is to have the return value of write_message indicate how to proceed. One response could be "Throw".

Replace error() / warning() etc. with messageType

Message format: Often useful to use form: GridAxis 'MyAxis': Linear extrapolation bogus

Messages should be returned as const char* or const string& (not string_view).

Additional thoughts

• Remove context_pointer from Courierr. Given that Courierr is intended to be an interface with the specifics to be provided in derived classes, it seems overthinking-ish to include context_pointer in the base class. Not all derived schemes will need a void*. In particular, I am exploring a template-based derived class that uses a typed pointer, so I am either going to ignore the void* or use it with awkward casting. Let the derived classes provide any needed state storage.
• Change the btwxt throws to logged Errors. Let the derived logging function decide whether to throw. For btwxt, change class BtwxtLogger to do the throw. Default behavior is thus unchanged, but there is more flexibility.
• Perhaps elaborate the intent of the Courierr message types. For example, error() could be documented as assuming the application code will never return. That could actually be enforced with modest restructuring of Courierr -- set things up so if the derived class error() returns, the application aborts.

---

• Review functions that return after logging, if return is meaningless we may want to throw
• Make courierr use const string& instead of string_view
• Document courierr better (example with write_message)
• Add name to RGI for messaging
• Cleaner message wording

[tanaya-mankad]

I'd like to lay out some of the pretty extensive discussion that led us to where we are with the logger re: bullet point 2. We actually moved away from the suggested method, which is essentially what we had before.

The main point is the simplest: loggers log. They neither detect, propagate, or handle errors. Client code that detects an error should issue the error, and begin its propagation, whether it's logged or not (C++ Coding Standards #69). After all, only the client function knows its postconditions or invariants have been violated - a logger bears no responsibility for the client's problems. The throwing constructors in btwxt are an example. And there's plenty of flexibility for btwxt's client:

try
{
    auto grid_axis = new GridAxis(std::vector<double>{(1.0)}); // Can't create a GridAxis with only 1 value: throw
}
catch (BtwxtException &e)
{
    // Do some extra work, extra reporting, whatever...
    throw (std::exception(e.what());
}

It's not different from try-catch around a std::vector resize().

Throwing is the preferred mechanism in C++ (C++ Coding Standards #72) because, among other things, it separates error handling from the control flow and program logic. If the logger were responsible for generating and propagating the exception, you're making a non-enforceable agreement with the client that just logging error() => fatal. Now any implementation of the logging class interface that doesn't throw, is going to violate the method's postconditions (e.g. creating a viable GridAxis).

Lesson: Even if you can override the error() function to throw, don't. If a user of error() needs to log and throw, it can indeed log, then throw, as two separate calls, no need for a derived exception class at all. (That was just one possible shortcut.) In fact, I'd go so far as to label the Courierr methods as nothrow.

As an aside, we made the decision to allow btwxt to throw across the API boundary, even though that's also not advised (C++ Coding Standards #62) due to there being no universal binary interface for C++ exceptions. We almost exclusively link statically, and even if not, most open source projects that link to it will be using the same build system to build client and libraries at once. So, it's likely to be 99.9% safe.

[chipbarnaby]

My response is that Courierr is not a logging class, it is an error-condition-routing class. It allows notifications of various events in a library to be conveyed (dare I say couriered) to the application using that library. Whether the application logs or whatever is none of the base class's business.

My argument for defining error() as "fatal -- no return possible" and using that instead of throw is to consolidate the handling schemes. The application may throw within its derived error(), but routing the event that way allows the application first dibs on what happens. Getting rid of throws in btwxt is not really the point, because std library functions used by btwxt may throw, so the application try/catch is probably needed anyway. (Although std library exceptions could be deemed rare enough to allow them to be caught at some outer level.)

What I want in the application side code is --

1. Send all courierr calls to a common function with an enum indicating the event type. I would prefer that courierr did that itself, but I will concede that separate functions have the modest advantage that an un-overridden pure virtual would cause a compile error and that would help coders who cannot count to 4.
2. The cse uses its normal error reporting functions to report the problem. This sends messages to various places (stdout, err file, report file ...)
3. Alternatively, a bunch of code could be semi-duplicated in the overridden error(), warning(), etc. to avoid the offensive map-to-a -single function.
4. If the event is fatal, terminate operation via cse mechanisms (that probably do not involve throw).
5. For any type of event, there is the option to throw so info gets back to the call point. However, if the fatal / not fatal distinction is sufficient, try/catch would not be needed at the call point (rare exceptions from std library code would be handled at an outer level).
6. In any case, what happens when the courierr calls return to the library (btwxt) must be defined. Maybe the coureirr functions should return something so the application could tell the library how to handle the event after the application does what it needs to. I think that should be defined somehow so different libraries institute the same policies (e.g. error() = fatal, call has failed; warning() = something is dubious, but the call result is usable; info() = all good, but here's some information; debug() = same as info, not clearly needed). Such policies should be documented at a minimum and enforced if practical. Certainly the assumption that error() never returns can be enforced by calling abort() (or throwing) if the derived error() does return.

All of the above reduces clutter on the application side. The design as it stands is workable, but I think it requires a bunch of code that if I ran the world, I would make unnecessary.

The other suggestions (name in RGI, use const string& for messages, eliminate the context pointer in the courrier base class, maybe more) are independent and I'm not hearing any controversy.