Project

General

Profile

Task #985

Task #838: Improve generic error reporting routines

Context information for exceptions

Added by Teemu Murtola about 8 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
core library
Target version:
Difficulty:
uncategorized
Close

Description

It should be figured out what is the best/easiest way to add context information to exceptions, and that should then be implemented. The basic scenario is as follows:
  • There is a function f() that may throw an exception.
  • Another function g() calls f().
  • If f() throws, g() would like to add context information (that is not available to f(), since it may be called from separate contexts) to the exception and rethrow it.
  • There are potentially more of such functions in the call stack before the exception is finally caught.
  • Somewhere higher up, a generic handler should be able to format a sensible error messages from all the pieces of context information added.
Some alternatives, there can be others:
  1. Use the error_info mechanism provided by boost::exception. This is straightforward to use (just use the streaming operator to add another error_info object in a catch block and rethrow), but I think it isn't possible to access the error_info objects in insertion order. Also, it is not possible to add two instances of one type of error information. These limitations make it difficult to make the mechanism generic.
  2. Use the existing gmx::MessageStringCollector to compose a new error message from the one in the exception and rethrow the exception with the modified message. This is generic, but requires a bit of code at every use.
  3. Add a method to gmx::GromacsException to do the same as above, but with a single method call at the point of use.
An example of where the ordering of context information may be relevant is the following:
  • Values for a selection option can be parsed from a file, so the selection option is a higher level of context than the file.
  • Multiple selection options could be parsed from a single file (not currently implemented, but could easily happen), so the file is now higher in the context.
    A case where a file can include another file is an example where more than one instance of the same type of information could be useful.

Related issues

Blocks GROMACS - Task #656: Implement selection input from a file in the new frameworkClosed01/09/2011
Blocks GROMACS - Task #655: Improve selection error reporting / switch to exceptionsClosed01/09/2011

Associated revisions

Revision 8ae8e1a3 (diff)
Added by Teemu Murtola almost 8 years ago

Restructure fatal error formatting.

Split printFatalError() in errorformat.h to separate functions that
print the error header, (part of) the actual message, and the footer.
This makes it possible to print the message in calling code
incrementally instead of formatting it into a string buffer.
It is easier to print the message in parts this way, and there is also
less complications in handling possible std::bad_alloc errors from
manipulating a std::string.

Also added line wrapping functionality to the message writing function,
and improved the appearance of the header.

Part of #838 and #985.

Change-Id: Ief7399d8c7c2c0529e0bc8492726bff827e18c4c

Revision 9cc0d166 (diff)
Added by Teemu Murtola almost 8 years ago

Better context information handling for exceptions.

- GromacsException now has a prependContext() method that can be used in
a catch block to add additional context information to the exception
before rethrowing it.
- It is possible to nest exceptions instead of embedding all information
into the exception message string.

Information added by these new methods is currently not easily
accessible outside the exception implementation, but it is not needed
either. Such access can be implemented once it is more clear what are
the requirements.

Closes #985.

Change-Id: I2b9ed1d11ee3cb36cc7da927cf6730c6f8c353b2

History

#1 Updated by Teemu Murtola about 8 years ago

  • Priority changed from Normal to High

Marked as high priority since this functionality would be needed for several in-progress issues.

#2 Updated by Roland Schulz about 8 years ago

I would suggest to use error_info. error_info is overwritten when the same one is added again, thus in the example of multiple files only the file opened highest up in the hierarchy would be available for higher up stacks. If we want to support error_info types with several instances (e.g. files) we can make the error_info of type vector and append to it. I think the conversion into a message should best be only done at the location where the error is handled not at places where exceptions are rethrown / information is added.

If we think we might want to be able to add arbitrary additional information to an exception we could add

typedef std::vector<String> AdvancedErrorInfo;
typedef boost::error_info<struct tag_erradvinfo, AdvancedErrorInfo>        erradvinfo; 

(found this idea here: http://svn2.xp-dev.com/svn/Tactive-APO/trunk/Include/APO/Shared/Exception.hpp)

Maybe we could also create a

typedef boost::error_info<struct tag_erradvinfo, gmx::MessageStringCollector>        erradvinfo;

and use that for any additional information.
gmx::MessageStringCollector* m = boost::get_error_info<erradvinfo>(x);
if (m==null)
{
    m = new gmx::MessageStringCollector();
    e << m;
}
m->append(...);

Similar code would be required for vector<string> thus MessageStringCollector might be better. It might make sense to create a method of all but the last line of the previous code block. This is obvious similar to 2/3.

The advantages I see:
- Simple cases are simple (just use error_info - alternative 1 from task description)
- Exception information is computer readable (as long as e.g. vector of file is used and erradvinfo only in rare cases) and can be formatted different depending on requirements at location where exception is handled.
- erradvinfo allows all other cases where creating a specific error_info is overkill.

I think we shouldn't make it too complicated thus if your alternative 3 is easier I'm happy with that too. I think simple error_info (your alternative 1) should be sufficient in most cases.

#3 Updated by Teemu Murtola about 8 years ago

I agree that the error_info-based approach makes it very easy to format a technically detailed error message. But it's more difficult to format a user-friendly natural language message explaining the context where things went wrong, such that even a beginner could understand it without knowing how the code and the exceptions are structured. I think that thrower-provided message and context strings make this much easier.

The main (potential) problem I see is that for this to work with the error_info approach, each error_info object needs to have a very specific semantic meaning, and that each location where the error is printed out needs to know all of these and what they mean in the context of different exceptions. There is then a risk that the code that formats the error message actually needs to know the details of how the error_info objects are used in the specific code. Such coupling of two completely separate pieces of code breaks very easily. If there are very many different error_info objects for different pieces of context information, each specific to some subpart of the system, the error formatting code also explodes in complexity (not to mention that it isn't generic in the sense that it has code specific to those subparts). And there could be multiple places in the code where this formatting takes place.

For technical information (such as errno values and associated syscall names), the error_info is very nice, but I find it hard to apply to report context information from the places of code that currently use gmx::MessageStringCollector (or currently don't report any context information, as is for error messages for #656).

But we could perhaps get the best of both worlds by storing the error message into a (somewhat restructured) gmx::MessageStringCollector such that simple cases could store a simple string, and functions higher up the call stack could add context information (in the form of additional strings). If we then at some point identify cases where this level of control is not necessary, or can be achieved with a suitably generic error_info, those can then be converted. But often at first sight, most of these context cases are somehow unique, and it's very difficult to generate a suitable error_info abstraction based on a single case only. I think that adding error_info types without careful design can very easily lead to problems outlined above.

The MessageStringCollector is currently also used to gather messages from multiple exceptions and then report them as one (to identify more user input errors in one go). Some form of a vector<exception_ptr> error_info object might be nicer for this, but so far I haven't seen sufficient need for it to introduce the additional complexity. It would allow removing some complexity from the MessageStringCollector, though: at least for the exception case, there shouldn't be any need for it to contain more than one message (and the context strings for that message).

#4 Updated by Roland Schulz about 8 years ago

I agree with everything you write. It really depends on the depth in the call-tree where the exception is handled of whether the technical (error_info) or natural language (MessageStringCollector) approach is better. For an exception which isn't caught until the *Runner it should certainly already contain all important information in natural language because it makes it much more complicated to create the message at that point (as you describe). On the other hand at the very lowest level (e.g. basic file operations) not enough information is available to create a good natural language message. Thus it seems to me we should use error_info at the lowest level whenever extra information is useful for error handling and should create the natural language message at the level where it is easiest. This implies that errors without good natural language message should only be used if it is guaranteed that the exception is caught and a natural message is added much before it reaches the *Runner.
I would think that having a error_info<MessageStringCollector> is a bit better than putting it all into one string of the exception because it allows to format the string different at the highest level. But I'm not sure that it very important.

#5 Updated by Teemu Murtola about 8 years ago

  • Assignee set to Teemu Murtola

I'll make a draft based on the discussion here.

#6 Updated by Teemu Murtola almost 8 years ago

  • Status changed from New to In Progress

#7 Updated by Teemu Murtola almost 8 years ago

  • Status changed from In Progress to Closed

#8 Updated by Teemu Murtola almost 8 years ago

  • Priority changed from High to Normal

#9 Updated by Teemu Murtola over 6 years ago

  • Project changed from Source code reorganization to GROMACS
  • Category set to core library

Also available in: Atom PDF