Better context information handling for exceptions.
authorTeemu Murtola <teemu.murtola@gmail.com>
Fri, 31 Aug 2012 18:39:17 +0000 (21:39 +0300)
committerTeemu Murtola <teemu.murtola@gmail.com>
Fri, 7 Sep 2012 09:34:31 +0000 (12:34 +0300)
- 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

src/gromacs/commandline/cmdlineparser.cpp
src/gromacs/options/options.cpp
src/gromacs/utility/exceptions.cpp
src/gromacs/utility/exceptions.h

index d9544799dd4c51abe3bc3a73579587dbb637a1bd..f3fe596a34229d9913f542c0ff45500255037933 100644 (file)
 
 #include <cctype>
 
+#include <string>
+#include <vector>
+
 #include "gromacs/options/optionsassigner.h"
 #include "gromacs/utility/exceptions.h"
-#include "gromacs/utility/messagestringcollector.h"
 
 namespace gmx
 {
@@ -97,7 +99,8 @@ void CommandLineParser::parse(int *argc, char *argv[])
 
 void CommandLineParser::parse(std::vector<std::string> *commandLine)
 {
-    MessageStringCollector errors;
+    ExceptionInitializer errors("Invalid command-line options");
+    std::string currentContext;
     // Start in the discard phase to skip options that can't be understood.
     bool bDiscard = true;
 
@@ -114,24 +117,26 @@ void CommandLineParser::parse(std::vector<std::string> *commandLine)
                 {
                     impl_->assigner_.finishOption();
                 }
-                catch (const UserInputError &ex)
+                catch (UserInputError &ex)
                 {
-                    errors.append(ex.what());
+                    ex.prependContext(currentContext);
+                    errors.addCurrentExceptionAsNested();
                 }
-                errors.finishContext();
+                currentContext.clear();
             }
-            errors.startContext("In command-line option " + *arg);
+            currentContext = "In command-line option " + *arg;
             bDiscard = false;
             try
             {
                 const char *name = arg->c_str() + 1;
                 impl_->assigner_.startOption(name);
             }
-            catch (const UserInputError &ex)
+            catch (UserInputError &ex)
             {
                 bDiscard = true;
-                errors.append(ex.what());
-                errors.finishContext();
+                ex.prependContext(currentContext);
+                errors.addCurrentExceptionAsNested();
+                currentContext.clear();
             }
         }
         else if (!bDiscard)
@@ -140,9 +145,10 @@ void CommandLineParser::parse(std::vector<std::string> *commandLine)
             {
                 impl_->assigner_.appendValue(*arg);
             }
-            catch (const UserInputError &ex)
+            catch (UserInputError &ex)
             {
-                errors.append(ex.what());
+                ex.prependContext(currentContext);
+                errors.addCurrentExceptionAsNested();
             }
         }
     }
@@ -152,17 +158,17 @@ void CommandLineParser::parse(std::vector<std::string> *commandLine)
         {
             impl_->assigner_.finishOption();
         }
-        catch (const UserInputError &ex)
+        catch (UserInputError &ex)
         {
-            errors.append(ex.what());
+            ex.prependContext(currentContext);
+            errors.addCurrentExceptionAsNested();
         }
-        errors.finishContext();
     }
     impl_->assigner_.finish();
-    if (!errors.isEmpty())
+    if (errors.hasNestedExceptions())
     {
         // TODO: This exception type may not always be appropriate.
-        GMX_THROW(InvalidInputError(errors.toString()));
+        GMX_THROW(InvalidInputError(errors));
     }
 }
 
index 73ff1a20357fbb5d2073198308922327f251aa38..c84664d8b7b25944f100157d688ce6509aa93197 100644 (file)
@@ -169,7 +169,8 @@ bool Options::isSet(const char *name) const
 
 void Options::finish()
 {
-    MessageStringCollector errors;
+    // TODO: Consider how to customize these error messages based on context.
+    ExceptionInitializer errors("Invalid input values");
     Impl::OptionList::const_iterator i;
     for (i = impl_->options_.begin(); i != impl_->options_.end(); ++i)
     {
@@ -178,10 +179,10 @@ void Options::finish()
         {
             option.finish();
         }
-        catch (const UserInputError &ex)
+        catch (UserInputError &ex)
         {
-            MessageStringContext context(&errors, "In option " + option.name());
-            errors.append(ex.what());
+            ex.prependContext("In option " + option.name());
+            errors.addCurrentExceptionAsNested();
         }
     }
     Impl::SubSectionList::const_iterator j;
@@ -192,15 +193,15 @@ void Options::finish()
         {
             section.finish();
         }
-        catch (const UserInputError &ex)
+        catch (const UserInputError &)
         {
-            errors.append(ex.what());
+            errors.addCurrentExceptionAsNested();
         }
     }
-    if (!errors.isEmpty())
+    if (errors.hasNestedExceptions())
     {
         // TODO: This exception type may not always be appropriate.
-        GMX_THROW(InvalidInputError(errors.toString()));
+        GMX_THROW(InvalidInputError(errors));
     }
 }
 
index 88172fcaea8a5447e019b64893876914152120a7..ddeca7ba6767413116f7a0ee1150817ef75a7c0e 100644 (file)
  * \author Teemu Murtola <teemu.murtola@cbr.su.se>
  * \ingroup module_utility
  */
-#include "gromacs/utility/exceptions.h"
+#include "exceptions.h"
 
 #include <boost/exception/get_error_info.hpp>
+#include <boost/shared_ptr.hpp>
 
 #include "gromacs/utility/errorcodes.h"
+#include "gromacs/utility/gmxassert.h"
 
 #include "errorformat.h"
 
 namespace gmx
 {
 
+namespace
+{
+
+/********************************************************************
+ * ErrorMessage
+ */
+
+class ErrorMessage
+{
+    public:
+        /*! \brief
+         * Creates an error message object with the specified text.
+         *
+         * \param[in] text  Text for the message.
+         */
+        explicit ErrorMessage(const std::string &text);
+
+        //! Whether this object is a context string.
+        bool isContext() const { return child_; }
+        //! Returns the text for this object.
+        const std::string &text() const { return text_; }
+        /*! \brief
+         * Returns the child object for a context object.
+         *
+         * Must not be called if isContext() returns false.
+         */
+        const ErrorMessage &child() const
+        {
+            GMX_ASSERT(isContext(),
+                       "Attempting to access nonexistent message object");
+            return *child_;
+        }
+
+        /*! \brief
+         * Creates a new message object with context prepended.
+         *
+         * \param[in] context  Context string to add.
+         * \returns   New error message object that has \p context as its text
+         *      and \c this as its child.
+         * \throws    std::bad_alloc if out of memory.
+         */
+        ErrorMessage prependContext(const std::string &context) const;
+
+    private:
+        std::string                     text_;
+        boost::shared_ptr<ErrorMessage> child_;
+};
+
+/*! \internal \brief
+ * Stores a reason or the top-most context string of an exception.
+ *
+ * \ingroup module_utility
+ */
+typedef boost::error_info<struct errinfo_message_, ErrorMessage>
+        errinfo_message;
+
+ErrorMessage::ErrorMessage(const std::string &text)
+    : text_(text)
+{
+    size_t length = text_.find_last_not_of(" \n");
+    if (length == std::string::npos)
+    {
+        length = text_.length() - 1;
+    }
+    text_.resize(length + 1);
+}
+
+ErrorMessage
+ErrorMessage::prependContext(const std::string &context) const
+{
+    ErrorMessage newMessage(context);
+    newMessage.child_.reset(new ErrorMessage(*this));
+    return newMessage;
+}
+
+/*! \brief
+ * Stores list of nested exceptions for Gromacs exceptions.
+ *
+ * \ingroup module_utility
+ */
+typedef boost::error_info<struct errinfo_message_, internal::NestedExceptionList>
+        errinfo_nested_exceptions;
+
+} // namespace
+
 /********************************************************************
  * GromacsException
  */
 
-GromacsException::GromacsException(const std::string &reason)
+GromacsException::GromacsException(const ExceptionInitializer &details)
 {
-    *this << errinfo_message(reason);
+    *this << errinfo_message(ErrorMessage(details.reason_));
+    if (details.hasNestedExceptions())
+    {
+        *this << errinfo_nested_exceptions(details.nested_);
+    }
 }
 
 const char *GromacsException::what() const throw()
 {
-    const std::string *msg = boost::get_error_info<errinfo_message>(*this);
-    return msg != NULL ? msg->c_str() : "No reason provided";
+    const ErrorMessage *msg = boost::get_error_info<errinfo_message>(*this);
+    while (msg != NULL && msg->isContext())
+    {
+        msg = &msg->child();
+    }
+    return msg != NULL ? msg->text().c_str() : "No reason provided";
+}
+
+void GromacsException::prependContext(const std::string &context)
+{
+    const ErrorMessage *msg = boost::get_error_info<errinfo_message>(*this);
+    GMX_RELEASE_ASSERT(msg != NULL, "Message should always be set");
+    *this << errinfo_message(msg->prependContext(context));
 }
 
 /********************************************************************
@@ -105,6 +207,65 @@ int NotImplementedError::errorCode() const
  * Global functions
  */
 
+namespace
+{
+
+void printExceptionMessage(FILE *fp, const std::exception &ex, int indent)
+{
+    const boost::exception *boostEx = dynamic_cast<const boost::exception *>(&ex);
+    if (boostEx != NULL)
+    {
+        // TODO: Remove duplicate context if present in multiple nested exceptions.
+        const ErrorMessage *msg = boost::get_error_info<errinfo_message>(*boostEx);
+        if (msg != NULL)
+        {
+            while (msg != NULL && msg->isContext())
+            {
+                internal::printFatalErrorMessageLine(fp, msg->text().c_str(), indent*2);
+                ++indent;
+                msg = &msg->child();
+            }
+            if (msg != NULL && !msg->text().empty())
+            {
+                internal::printFatalErrorMessageLine(fp, msg->text().c_str(), indent*2);
+            }
+        }
+        else
+        {
+            internal::printFatalErrorMessageLine(fp, ex.what(), 0);
+        }
+
+        // TODO: Treat also boost::nested_exception (not currently used, though)
+
+        const internal::NestedExceptionList *nested
+            = boost::get_error_info<errinfo_nested_exceptions>(*boostEx);
+        if (nested != NULL)
+        {
+            ++indent;
+            internal::NestedExceptionList::const_iterator ni;
+            for (ni = nested->begin(); ni != nested->end(); ++ni)
+            {
+                try
+                {
+                    rethrow_exception(*ni);
+                }
+                catch (const std::exception &nestedEx)
+                {
+                    printExceptionMessage(fp, nestedEx, indent);
+                }
+            }
+        }
+
+        // TODO: Treat errno information in boost exceptions
+    }
+    else
+    {
+        internal::printFatalErrorMessageLine(fp, ex.what(), 0);
+    }
+}
+
+} // namespace
+
 void printFatalErrorMessage(FILE *fp, const std::exception &ex)
 {
     const char *title = "Unknown exception";
@@ -135,8 +296,7 @@ void printFatalErrorMessage(FILE *fp, const std::exception &ex)
                                     funcPtr != NULL ? *funcPtr : NULL,
                                     filePtr != NULL ? *filePtr : NULL,
                                     linePtr != NULL ? *linePtr : 0);
-    internal::printFatalErrorMessageLine(fp, ex.what(), 0);
-    // TODO: Treat errno information in boost exceptions
+    printExceptionMessage(fp, ex, 0);
     internal::printFatalErrorFooter(fp);
 }
 
index 7f246325ec5ba247ddbdfd409561f5cac0ed231f..7448835a61531337e4855df9b96f0b89add42e92 100644 (file)
@@ -44,7 +44,9 @@
 
 #include <exception>
 #include <string>
+#include <vector>
 
+#include <boost/exception_ptr.hpp>
 #include <boost/exception/errinfo_api_function.hpp>
 #include <boost/exception/errinfo_errno.hpp>
 #include <boost/exception/exception.hpp>
 namespace gmx
 {
 
-/*! \brief
- * Stores a user-friendly explanation for the reason of an exception.
- *
- * Typically, should not be used directly, but through the GromacsException
- * class: it is initialized by the constructor, and can be accessed with
- * GromacsException::what().
- *
- * \inlibraryapi
- */
-typedef boost::error_info<struct errinfo_message_, std::string> errinfo_message;
+namespace internal
+{
+//! Internal container type for storing a list of nested exceptions.
+typedef std::vector<boost::exception_ptr> NestedExceptionList;
+} // namespace internal
 
 /*! \addtopublicapi
  * \{
  */
 
+/*! \brief
+ * Provides information for Gromacs exception constructors.
+ *
+ * This class exists to implement common functionality for initializing all
+ * Gromacs exceptions without having extra code in each exception class.
+ * In simple cases, it can be implicitly constructed by passing a simple string
+ * to an exception constructor.
+ * If more complex initialization is necessary, it is possible to explicitly
+ * construct an object of this type and then call other methods to add
+ * information before actually creating the exception object.
+ *
+ * \todo
+ * With the exception of the reason string, information added with this class
+ * is not currently accessible through any public API, except for calling
+ * printFatalErrorMessage().  This is not implemented as there is no current
+ * need for it, and it is not clear what would be the best alternative for the
+ * access.  It should be possible to refactor the internal implementation to
+ * suit the needs of such external access without requiring changes in code
+ * that throws these exceptions.
+ *
+ * \ingroup module_utility
+ */
+class ExceptionInitializer
+{
+    public:
+        /*! \brief
+         * Creates an initialized with the given string as the reason.
+         *
+         * \param[in] reason  Detailed reason for the exception.
+         * \throw     std::bad_alloc if out of memory.
+         *
+         * This constructor is not explicit to allow constructing exceptions
+         * with a plain string argument given to the constructor without adding
+         * extra code to each exception class.
+         */
+        ExceptionInitializer(const char *reason)
+            : reason_(reason)
+        {
+        }
+        //! \copydoc ExceptionInitializer(const char *)
+        ExceptionInitializer(const std::string &reason)
+            : reason_(reason)
+        {
+        }
+
+        /*! \brief
+         * Returns true if addCurrentExceptionAsNested() has been called.
+         *
+         * Provided for convenience for cases where exceptions will be added
+         * conditionally, and the caller wants to check whether any excetions
+         * were actually added.
+         */
+        bool hasNestedExceptions() const { return !nested_.empty(); }
+        /*! \brief
+         * Adds the currently caught exception as a nested exception.
+         *
+         * May be called multiple times; all provided exceptions will be added
+         * in a list of nested exceptions.
+         *
+         * Must not be called outside a catch block.
+         */
+        void addCurrentExceptionAsNested()
+        {
+            nested_.push_back(boost::current_exception());
+        }
+
+    private:
+        std::string                     reason_;
+        internal::NestedExceptionList   nested_;
+
+        friend class GromacsException;
+};
+
 /*! \brief
  * Base class for all exception objects in Gromacs.
  *
@@ -98,13 +168,32 @@ class GromacsException : public std::exception, public boost::exception
          */
         virtual int errorCode() const = 0;
 
+        /*! \brief
+         * Adds context information to this exception.
+         *
+         * \param[in] context  Context string to add.
+         * \throws    std::bad_alloc if out of memory.
+         *
+         * Typical use is to add additional information higher up in the call
+         * stack using this function in a catch block and the rethrow the
+         * exception.
+         *
+         * \todo
+         * The added information is currently not accessible through what(),
+         * nor through any other means except for calling
+         * printFatalErrorMessage(). See ExceptionInitializer for more
+         * discussion.
+         */
+        void prependContext(const std::string &context);
+
     protected:
         /*! \brief
-         * Creates an exception object with the provided detailed reason.
+         * Creates an exception object with the provided initializer/reason.
          *
-         * \param[in] reason Detailed reason for the exception.
+         * \param[in] details  Initializer for the exception.
+         * \throws    std::bad_alloc if out of memory.
          */
-        explicit GromacsException(const std::string &reason);
+        explicit GromacsException(const ExceptionInitializer &details);
 };
 
 /*! \brief
@@ -116,12 +205,17 @@ class FileIOError : public GromacsException
 {
     public:
         /*! \brief
-         * Creates an exception object with the provided detailed reason.
+         * Creates an exception object with the provided initializer/reason.
+         *
+         * \param[in] details  Initializer for the exception.
+         * \throws    std::bad_alloc if out of memory.
          *
-         * \param[in] reason Detailed reason for the exception.
+         * It is possible to call this constructor either with an explicit
+         * ExceptionInitializer object (useful for more complex cases), or
+         * a simple string if only a reason string needs to be provided.
          */
-        explicit FileIOError(const std::string &reason)
-            : GromacsException(reason) {}
+        explicit FileIOError(const ExceptionInitializer &details)
+            : GromacsException(details) {}
 
         virtual int errorCode() const;
 };
@@ -138,8 +232,8 @@ class UserInputError : public GromacsException
 {
     protected:
         //! \copydoc FileIOError::FileIOError()
-        explicit UserInputError(const std::string &reason)
-            : GromacsException(reason) {}
+        explicit UserInputError(const ExceptionInitializer &details)
+            : GromacsException(details) {}
 };
 
 /*! \brief
@@ -151,8 +245,8 @@ class InvalidInputError : public UserInputError
 {
     public:
         //! \copydoc FileIOError::FileIOError()
-        explicit InvalidInputError(const std::string &reason)
-            : UserInputError(reason) {}
+        explicit InvalidInputError(const ExceptionInitializer &details)
+            : UserInputError(details) {}
 
         virtual int errorCode() const;
 };
@@ -166,8 +260,8 @@ class InconsistentInputError : public UserInputError
 {
     public:
         //! \copydoc FileIOError::FileIOError()
-        explicit InconsistentInputError(const std::string &reason)
-            : UserInputError(reason) {}
+        explicit InconsistentInputError(const ExceptionInitializer &details)
+            : UserInputError(details) {}
 
         virtual int errorCode() const;
 };
@@ -181,8 +275,8 @@ class SimulationInstabilityError : public GromacsException
 {
     public:
         //! \copydoc FileIOError::FileIOError()
-        explicit SimulationInstabilityError(const std::string &reason)
-            : GromacsException(reason) {}
+        explicit SimulationInstabilityError(const ExceptionInitializer &details)
+            : GromacsException(details) {}
 
         virtual int errorCode() const;
 };
@@ -196,8 +290,8 @@ class InternalError : public GromacsException
 {
     public:
         //! \copydoc FileIOError::FileIOError()
-        explicit InternalError(const std::string &reason)
-            : GromacsException(reason) {}
+        explicit InternalError(const ExceptionInitializer &details)
+            : GromacsException(details) {}
 
         virtual int errorCode() const;
 };
@@ -211,8 +305,8 @@ class APIError : public GromacsException
 {
     public:
         //! \copydoc FileIOError::FileIOError()
-        explicit APIError(const std::string &reason)
-            : GromacsException(reason) {}
+        explicit APIError(const ExceptionInitializer &details)
+            : GromacsException(details) {}
 
         virtual int errorCode() const;
 };
@@ -226,8 +320,8 @@ class NotImplementedError : public APIError
 {
     public:
         //! \copydoc FileIOError::FileIOError()
-        explicit NotImplementedError(const std::string &reason)
-            : APIError(reason) {}
+        explicit NotImplementedError(const ExceptionInitializer &details)
+            : APIError(details) {}
 
         virtual int errorCode() const;
 };