Fix error handling in ARB

[westram]

Problems: several errors are not safely propagated upwards

1. reason is GB_export_error, which is evil by design
2. reason are error-return values which are not used (dropped)

Solutions:

1. change GB_export_error interface
   • GB_export_error() shall return 'void'
   • GB_export_error() shall fail, if there's already an error exported

2. avoid dropped errors
   • C++: do not use GB_ERROR, instead use ARB_ERROR. done in:
     • SL/FASTALIGNER, EDIT4(partly) [6279]
   • C: attribute all functions returning GB_ERROR with __ATTR_USERESULT (not much C left)

3. stop using GB_export_error
   • GB_export_error is used in (and was probably "designed" for)
     • functions that either report RESULT or ERROR (i.e. return RESULT via pointer and export error if RESULT is NULL) and in
     • functions that may additionally return neighter (e.g. GB_find finding nothing)
   • alternatively such functions could return special objects containing RESULT and ERROR
     • such an object should
       • refuse to accept both (RESULT and ERROR)
       • refuse access to RESULT w/o having tested for ERROR
       • refuse to destroy when completely ignored (similar to ARB_ERROR)

[westram]

• implemented and tested smarter error type ARB_ERROR [6277]

[epruesse]

"alternatively such functions could return special objects containing RESULT and ERROR"

Well, the standard way of returning an error object in C++ is "throw object"

So, since we're moving slowly towards C++ anyway, how about approaching this with a new C++ API for the ? Just a thin wrapper layer for now, similar to GB_transaction.

Might be actionism. If you think it's an idea to follow up on, I could create a ticket so that we can start thinking about a sensible, stable "API".

[westram]

Replying to epruesse:

"alternatively such functions could return special objects containing RESULT and ERROR" Well, the standard way of returning an error object in C++ is "throw object"

Problem is to ensure that somebody catches (in all existing code using ARBDB)

So, since we're moving slowly towards C++ anyway, how about approaching this with a new C++ API for the ? Just a thin wrapper layer for now, similar to GB_transaction.

The last thin wrapper layer has been removed with [6318], since it was never used. I don't believe in introducing new layers, i believe in changing old layers into new ones step by step.

[epruesse]

Problem is to ensure that somebody catches (in all existing code using ARBDB)

This cannot be helped in any case. Fixing error handling can't be done with "regexp-refactoring". The relevant code always has to be reviewed.

The ticket says that the core issue is that the errors are not propagates upwards. Well, this is exactly what exceptions are designed for. They have their issues, but I doubt we can come up with something better. Conceptually there's a lot of thought behind exceptions, and I'd expect the gcc implementation of a language intrinsic to be far better than any library code we could hack up…

The last thin wrapper layer has been removed with [6318], since it was never used.

It was removed because it was broken, AFAIK. We have unit tests now, so we can have code in development that stays "healthy" even if there are only few "".

I don't believe in introducing new layers, i believe in changing old layers into new ones step by step.

Then consider the "step" in this case being the development of a new OO-style API for the .

Many other projects have followed the "C++ wrapper layer" concept. It's basically just a "binding layer", just like the Perl-layer we already have. Separating the OO stuff from the C-style ARBDB code has the benefit of keeping the original API intact for all the code that needs it and cannot be rewriten easily/quickly. Moving from pure imperative to OO also isn't something to be done incrementally, it requires rethinking the entire structure and would amount to rewriting most of ARBDB. Not something we have the resources for currently.

[westram]

Replying to epruesse:

Problem is to ensure that somebody catches (in all existing code using ARBDB)

The relevant code always has to be reviewed.

Here relevant means any code using functions from ARBDB. Far too much to be done by "reviewing" code. Instead the API has to enforce contracts. And the contract this ticket is trying to establish is errors may not pass unchecked.

The ticket says that the core issue is that the errors are not propagates upwards. Well, this is exactly what exceptions are designed for.

True - and for sure that is where ARBDB should go (at some point in the future). But introducing exceptions into code (that does not yet use any) requires to make the code exception safe. Amongst many other things that requires at least a complete rewrite of resource acquisition between any potential throw-catch-combination. That is a big task of its own and needs to be performed before even thinking about exceptions.

The last thin wrapper layer has been removed with [6318], since it was never used.

It was removed because it was broken, AFAIK.

The other way round: it rotted, because it was rarely used. Originally it was introduced with similar arguments as yours, but it never reproduced the full functionality of the C-API, so nobody used it. And because nobody used it, it was never improved. And so on…

I don't believe in introducing new layers, i believe in changing old layers into new ones step by step.

Then consider the "step" in this case being the development of a new OO-style API for the .

I don't see the point. How does some new-API improve error handling of the underlying old-API and those clients still using old-API? w/o exceptions you won't gain anything..

Moving from pure imperative to OO also isn't something to be done incrementally,

There is a load of books describing how to do that, arguing why it is far more promising to do it incrementally, esp. in huge-sized legacy software.

I know you tend towards BFUD and so did I, but ARB has reinstructed me