Exception late review
I don't know if review manager still accepts reviews as review period is over but I'd like to share my ideas with others. I'm very interested in this library but I don't think that this version is good enough for inclusion into boost. As already pointed out by many people, library name is misleading. I'd expect from a library named Boost.exception that: - it provides functionality missing in std::exception (clone is one example), - it recommends deriving all boost exception classes from boost::exception and - wrapping standard exceptions with correspondent boost exceptions in boost::throw_exception. The library doesn't have enough features to cover problem domain or they are not well documented. Example shows how to attach additionaly information to FILE but it would be much better to demonstrate how information is added at each layer and then presented to end user. One example is RDBMS where some exception is raised at OS layer (e.g. file system full), then caught at db storage layer, rethown, then caught at higher layere up to SQL layer where it's nicely presented and send to a user. This case should cover tags associated with layers. Other important case is stacking up information as an exception is being processed. Stack trace is a good example. Also, it should be clear how library handle bad_alloc. In some situations, it's fine to rethrown bad_alloc instead of original exception but more fine tuned behavior is require in other situations. One example is stop adding info to a stack and just count all not added stack entries. Sorry for not very well detailed review and for being too late. I was hoping to find time and think thoroughly about interface but I couldn't find enough time. -- Alexander
On 10/22/07, Alexander Nasonov <alnsn@yandex.ru> wrote:
As already pointed out by many people, library name is misleading. I'd expect from a library named Boost.exception that:
- it provides functionality missing in std::exception (clone is one example),
As pointed out by me and by others, cloning is outside the scope of Boost Exception.
- it recommends deriving all boost exception classes from boost::exception and
I'm confused; this is exactly what Boost Exception recommends.
- wrapping standard exceptions with correspondent boost exceptions in boost::throw_exception.
boost::exception is designed as an extension of std::exception. You don't wrap standard exceptions with correspondent std::exception, you just derive std::exception. What do you mean by "wrapping"?
The library doesn't have enough features to cover problem domain or they are not well documented.
Example shows how to attach additionaly information to FILE but it would be much better to demonstrate how information is added at each layer and then presented to end user.
The example included in the documentation demonstrates all aspects of using boost::exception: - how to store information in an exception object at the time of the throw, - how to append information to an existing exception object at some intermediate level, - how to access the information at the point of the catch, to format a user-friendly message. This example seems complete to me, but it is hard for me to imagine how it would work on someone without much knowledge of boost::exception. I am very interested in ideas on how to make this example better.
Other important case is stacking up information as an exception is being processed. Stack trace is a good example.
Stack trace can not be implemented in a platform independent manner; we need the collaboration of the Boost community to implement it correctly on multiple platforms. It is a debugging feature that once implemented, will appear automatically in the ::what() message. Other "stackable" information can be stored in boost::exceptions by storing a container object (such as std::vector) using error_info, then accessing it by get_error_info to add more elements.
Also, it should be clear how library handle bad_alloc. In some situations, it's fine to rethrown bad_alloc instead of original exception but more fine tuned behavior is require in other situations.
If you run out of memory at just the right time when using Boost Exception, you might throw bad_alloc instead of the exception you wanted to throw. However, this is not specific to boost::exception; for example, you'll get the same behavior from any exception object that contains a std::string. In general, failure to throw an exception can not be "fine tuned". For example, with or without Boost Exception, throwing requires memory allocation, and when that allocation fails, something else (typically not very friendly) happens. Emil Dotchevski
Emil Dotchevski <emil <at> revergestudios.com> writes:
On 10/22/07, Alexander Nasonov <alnsn <at> yandex.ru> wrote:
- it recommends deriving all boost exception classes from boost::exception I'm confused; this is exactly what Boost Exception recommends.
Your exception class is not a replacement for std::exception. You recommend deriving from your class _and_ std::exception.
- wrapping standard exceptions with correspondent boost exceptions in boost::throw_exception.
boost::exception is designed as an extension of std::exception.
It has no is-a relation with std::exception. So, it's more like "addition" rather than "extension".
You don't wrap standard exceptions with correspondent std::exception, you just derive std::exception. What do you mean by "wrapping"?
That's what I mean: catch(std::bad_argument const& ex) { throw boost::bad_argument(ex); } ... and so on for every std exception.
The example included in the documentation demonstrates all aspects of using boost::exception:
- how to store information in an exception object at the time of the throw, - how to append information to an existing exception object at some intermediate level, - how to access the information at the point of the catch, to format a user-friendly message.
This example seems complete to me, but it is hard for me to imagine how it would work on someone without much knowledge of boost::exception. I am very interested in ideas on how to make this example better.
If you caught an exception at upper layer, it'd be extremely useful to iterate over all tags at lower layers without maintaining a list of tags. One strategy here is to create a hierarchy of tags with lower-level tags derived from upper-level tags and have a method that returns a container of tags: vector<ThisLayerTag*> infos = get_derived_error_info<ThisLayerTag>(ex); or something like that with nicer interface.
Other important case is stacking up information as an exception is being processed. Stack trace is a good example.
Stack trace can not be implemented in a platform independent manner; we need the collaboration of the Boost community to implement it correctly on multiple platforms. It is a debugging feature that once implemented, will appear automatically in the ::what() message.
OK, I'll call it "catch points" or "trace points" then. User push_back something in every catch clause and reshow. Unlike tags, which have unique types, this stack stores values of one type. For example, catch( my_error& e) { e.push_back(catch_point(__FILE__, __LINE__)); throw; } where catch_point is std::pair<char const*, int> or a struct.
Other "stackable" information can be stored in boost::exceptions by storing a container object (such as std::vector) using error_info, then accessing it by get_error_info to add more elements.
OK. Though, distinction between unique tags and tags associated with a container should be more clear.
Also, it should be clear how library handle bad_alloc. In some situations, it's fine to rethrown bad_alloc instead of original exception but more fine tuned behaviour is require in other situations.
If you run out of memory at just the right time when using Boost Exception, you might throw bad_alloc instead of the exception you wanted to throw. However, this is not specific to boost::exception; for example, you'll get the same behaviour from any exception object that contains a std::string.
There is a fundamental difference between throwing bad_alloc when trying to throw another exception and doing a lot of things inside a catch clause. In the latter case, it's good not to throw at all. std::exception designed with this in mind because what() returns char const* and has no-throw guarantee. Your library should follow this principle.
In general, failure to throw an exception can not be "fine tuned". For example, with or without Boost Exception, throwing requires memory allocation, and when that allocation fails, something else (typically not very friendly) happens.
I believe, it can be. For example, storage can be preallocated in my_error ctor. If push_back inside a catch clause throws, the library can set a flag. I'd like to repeat it. It's very important to avoid throwing exceptions implicitly inside a catch clause. -- Alexander
On 10/23/07, Alexander Nasonov <alnsn@yandex.ru> wrote:
Your exception class is not a replacement for std::exception. You recommend deriving from your class _and_ std::exception. <snip> It has no is-a relation with std::exception. So, it's more like "addition" rather than "extension".
In principle, there is no problem for boost::exception to derive from std::exception, or even to replace std::exception. In practice, we have legacy code that derives from std::exception already, and making boost::exception not derive from std::exception enables integration with existing 3rd party and standard code through enable_error_info. Do you see any practical problems with this design decision?
You don't wrap standard exceptions with correspondent std::exception, you just derive std::exception. What do you mean by "wrapping"?
That's what I mean:
catch(std::bad_argument const& ex) { throw boost::bad_argument(ex); } ... and so on for every std exception.
You probably don't want to replace std::bad_argument with some boost::bad_argument because you would break standard-complying code that expects to catch std::bad_argument. That said, boost::exception does provide integration with the standard exception classes. In your own code you can change a "throw std::bad_argument()" to "throw boost::enable_error_info(std::bad_argument())", which will not break any existing code that catches std::bad_argument, yet will enable newer, Boost Exception-aware code to intercept the exception and add information. If the std::bad_argument exception originated from the standard library or some 3rd-party code, then you can catch and throw, like this: catch(std::bad_argument const& ex) { throw boost::enable_error_info(ex); } More information is available here: http://www.revergestudios.com/boost-exception/boost-exception.htm#existing_h...
If you caught an exception at upper layer, it'd be extremely useful to iterate over all tags at lower layers without maintaining a list of tags. One strategy here is to create a hierarchy of tags with lower-level tags derived from upper-level tags and have a method that returns a container of tags:
vector<ThisLayerTag*> infos = get_derived_error_info<ThisLayerTag>(ex);
or something like that with nicer interface.
Do you have a less abstract, real-life use case for this? In particular, the reason Boost Exception lacks iteration over the stored tags is that after more than a year of using it in my own code base, I never needed to iterate. In general, the rationale for not supporting iteration is that iteration presumes catching boost::exception and then probing it for the stored information in an attempt to try to understand what went wrong. A better strategy is to catch different types for different failures, and then (knowing the semantics of each exception type) get the information you know is available in the exception object.
Stack trace can not be implemented in a platform independent manner; we need the collaboration of the Boost community to implement it correctly on multiple platforms. It is a debugging feature that once implemented, will appear automatically in the ::what() message.
OK, I'll call it "catch points" or "trace points" then. User push_back something in every catch clause and reshow. Unlike tags, which have unique types, this stack stores values of one type. For example,
catch( my_error& e) { e.push_back(catch_point(__FILE__, __LINE__)); throw; }
where catch_point is std::pair<char const*, int> or a struct.
This can be easily implemented on top of the proposed design: typedef std::pair<char const *,int> catch_point; typedef std::list<catch_point> catch_points; struct tag_catch_points: boost::error_info<catch_points> { }; void add_catch_point( boost::exception & e, char const * file, int line ) { if( !boost::get_error_info<tag_catch_points>(e) ) e << boost::error_info<tag_catch_points>(catch_points()); (*boost::get_error_info<tag_catch_points>(e)).push_back(catch_point(file,line)); } and then: catch( my_error& e) { add_catch_point(e,__FILE__,__LINE__); throw; }
<snip> std::exception designed with this in mind because what() returns char const* and has no-throw guarantee. Your library should follow this principle.
boost::exception::what() also returns char const * and has to-throw guarantee. It follows all standard requirements for exception classes.
I'd like to repeat it. It's very important to avoid throwing exceptions implicitly inside a catch clause.
I see no problem for the code inside a catch to throw exceptions. Emil Dotchevski
participants (2)
-
Alexander Nasonov -
Emil Dotchevski