wiki:ExceptionRedesign
Last modified 11 years ago Last modified on 11/25/2008 10:37:47 PM

Exception Redesign

Discussion

  • Simpler to have exception type with getWrappedException().
    • Catching code likely needs to understand exception structure anyway.
  • Serge (in Association Pipeline) currently uses the exception DataProperty to store information about where the exception was thrown from.
    • Would rather have automatically included.
    • Requires macro to throw exceptions.
  • If data needed, create exception subclass with instance variables.
    • PropertySet for reporting purposes only, if at all.
  • Have different base classes: RuntimeError, LogicError.

Further Discussion

  • Prefer to add context information to existing exceptions thrown by value.
    • Context information can generally be string messages.
    • Very rare for exception catchers to need more information than type.
    • In such cases, can add member variables.
  • Need to ensure Python compatibility.
    • SWIG .i translates standard C++ exceptions already
  • No requirement for exception-throwers to log at same site.
    • Therefore need throw location information in exception.
    • Final exception catchers should generally log.

Conclusion

  • Have a simple exception class with location information and a message string.
  • Caught exceptions are added to, not wrapped.
  • Mandate use of a macro to create exceptions containing throwing location.
  • Rationalize current exception inheritance hierarchy.

Interface

namespace lsst {
namespace pex {
namespace exceptions {

// Internal use macro
#define LSST_EXCEPT_HERE \
    lsst::pex::exceptions::Exception::Tracepoint( \
        __FILE__, __LINE__, BOOST_CURRENT_FUNCTION)

// Use this macro to create an exception to throw.
#define LSST_EXCEPT(type, ...) type(#type, LSST_EXCEPT_HERE, __VA_ARGS__)

// Use this macro to add a message as well as traceback information
// when catching and rethrowing an exception.
#define LSST_EXCEPT_ADD(e, m) e.addMessage(LSST_EXCEPT_HERE, m)


// Use this macro to declare the arguments for the constructor of a
// subclass of Exception.  Any class-specific arguments must go after
// this macro.
#define LSST_EARGS_TYPED char const* ex_type, \
     lsst::pex::exceptions::Exception::Tracepoint const& ex_trace, \
     std::string const& ex_message

// Use this macro to initialize the Exception base class of a
// subclass of Exception.
#define LSST_EARGS_UNTYPED ex_type, ex_trace, ex_message

// Use this macro to declare a subclass of Exception
// with no additional data and hence no additional arguments.
// The first argument (`t`) is the name of the new subclass type.
// The second argument (`b`) is the name of the base class, which must
// be derived from `lsst::pex::exceptions::Exception` without adding
// any additional constructor arguments.
// The third argument (`c`) is intended to be the fully-specified
// C++ type of the exception for use by the SWIG interface.
// Note: Invocations of this macro are not followed by a semicolon.
// The semicolon was included in the macro expansion to enable a
// different expansion without semicolon in the SWIG context.
#define LSST_EXCEPTION_TYPE(t, b, c) \
    class t : public b { \
    public: \
        t(LSST_EARGS_TYPED) : b(LSST_EARGS_UNTYPED) { }; \
        virtual char const* ctype(void) throw() { return #c " *"; }; \
    };


// The exception class.
class Exception : public std::exception {
public:
    struct Tracepoint {
        Tracepoint(char const* file, int line, char const* func) :
            _file(file), _line(line), _func(func) { };
        char const* _file; // Compiled strings only; does not need deletion
        int _line;
        char const* _func; // Compiled strings only; does not need deletion
    };
    typedef std::vector<Tracepoint> Traceback;

    // Constructors
    Exception(LSST_EARGS_TYPED);
    virtual ~Exception(void) throw();

    // Modifiers
    // Do not call this member function directly.  Use one of the two
    // LSST_EXCEPT_ADD_* macros above.
    void addMessage(Tracepoint const& trace,
                    std::string const& message = std::string());

    // Accessors

    // Note that the vectors of Tracepoints and messages are in order
    // from the original throw through any higher-level catch/rethrows.
    Traceback const& getTraceback(void) const throw();
    std::vector<std::string> const& getMessages(void) const throw();

    // Formats the information in the exception for output.
    virtual std::ostream& addToStream(std::ostream& stream) const;

    // Used internally for the SWIG interface.
    virtual char const* ctype(void) const throw();
};

// Free function to add the string representation of an exception to a stream.
// Uses Exception::addToStream().
std::ostream& operator<<(std::ostream& stream, Exception const& e);

}
}
}

Python interface:

Each new exception class will be defined in Python via SWIG. It will be necessary to include the header file defining new exception classes in the package's .i file as well as importing the lsst/pex/exceptions/exceptionsLib.i file.

The new SWIGged exception class will derive from the lsst.pex.exceptions.Exception class but will not derive from the Python Exception base class. Instead, exceptions deriving from this Exception class will be caught (in p_lsstSwig.i) and transformed into lsst.pex.exceptions.LsstCppException instances. The LsstCppException class derives from LsstException (usable by Python code), which in turn derives from the Python Exception base class. The SWIG proxy for the customized exception class is accessible as the args[0] item in the LsstCppException.

Other exceptions thrown from C++ will be raised as Python RuntimeErrors. No attempt will be made to move the C++ traceback information into a Python traceback object.

Exception Hierarchy

There are two ways to use the capabilities of this Exception class. One is to have a very small number of pre-defined exceptions in pex::exceptions and rely on other packages to define their own more specialized exceptions. This could lead to duplication of exceptions and difficulties catching them when multiple packages are used, but it generally encourages the use of exceptions. The other way is to have nearly all exceptions defined in the central pex::exceptions package. As long as exceptions are only added and not removed or changed, there should not be package dependency issues. The need to update and re-release the central package will encourage consistency of exception usage but may discourage adding appropriate specialized exceptions.

GPDF has suggested that we ensure that there is a type of exception that will always cause a fatal exit. Such an exception need not have other type information, since by definition it cannot be handled other than to rethrow.

Possible hierarchy for first approach (based on C++):

LogicalError
RuntimeError
OtherError
FatalException

Another possible hierarchy (based on Python):

StandardError >
    ArithmeticError
    LookupError
    EnvironmentError
StopIteration
SystemExit

Possible initial hierarchy for second approach (based on existing hierarchy):

LogicalError >
    DomainError
    InvalidParameter
    LengthError
    OutOfRange
RuntimeError >
    RangeError >
        Overflow
        Underflow
    NotFound
    Memory
    IOError >
        FitsError
FatalException

Sample Usage

namespace pexExcept = lsst::pex::exceptions;

// Note: It only takes 4 lines in a header file and no m4
// to create a new derived exception class.
// Assumes minor waiver to the "no inlines in headers" rule.
// Could be 3 lines if we use `struct` instead of `class`/`public:`.
// Only 1 line using `LSST_EXCEPTION_TYPE` macro above.

class ChildException : public pexExcept::Exception {
public:
    ChildException(LSST_EARGS_TYPED) : Exception(LSST_EARGS_UNTYPED) { };
};

// Only slightly worse to define a derived class containing data.

class DetailedException : public pexExcept::Exception {
public:
    DetailedException(LSST_EARGS_TYPED, int count) :
        Exception(LSST_EARGS_UNTYPED), _count(count) { };
    virtual std::ostream& addToStream(std::ostream& stream) const {
        // Add information before and after base class message.
        return pexExcept::Exception::addToStream(
            stream << "Begin DetailedException(" << _count <<
            ")" << std::endl) <<
            "End DetailedException" << std::endl;
    };
    int getCount(void) const { return _count; };
private:
    int _count;
};

int main(void) {
    try {
        throw LSST_EXCEPT(pexExcept::Exception, "Testing 1");
    }
    catch (pexExcept::Exception const& e) {
        std::cout << e;
    }
    std::cout << "---" << std::endl;
    try {
        throw LSST_EXCEPT(ChildException, "Testing 2");
    }
    catch (pexExcept::Exception const& e) { // or ChildException const&
        std::cout << e;
    }
    std::cout << "---" << std::endl;
    try {
        throw LSST_EXCEPT(DetailedException,
            (boost::format("Testing %1%") % 3).str(), 3);
    }
    catch (DetailedException const& e) {
        std::cout << e << "DetailedException(" << e.getCount() << ")" << std::endl;
    }
    std::cout << "---" << std::endl;
    try {
        try {
            throw LSST_EXCEPT(ChildException, "Testing 4");
        }
        catch (ChildException& e) { // Note: no const here
            LSST_EXCEPT_ADD(e, "Caught exception");
            throw; // Note: not "throw e" in order to preserve type.
        }
    }
    catch(pexExcept::Exception const& f) {
        std::cout << f;
    }
    std::cout << "---" << std::endl;
    try {
        try {
            throw LSST_EXCEPT(ChildException, "Testing 5");
        }
        catch (ChildException& e) {
            LSST_EXCEPT_ADD(e, "Caught and rethrown");
            // or LSST_EXCEPT_ADD(e, (boost::format("Caught %1%") % 42).str());
            throw;
        }
    }
    catch(pexExcept::Exception const& f) {
        std::cout << f;
    }
    std::cout << "---" << std::endl;
    return 0;
}

Comments

Comment by smm on Thu 30 Oct 2008 03:27:20 PM CDT

It would be nice to add in the name of the function enclosing the exception via __func__ (or preferrably __PRETTY_FUNCTION__ where available). This is especially useful when things get thrown out of a template as it gives you the type signature of the concrete function the throw occurred in.

KTL - This functionality has been added using boost::current_function.

Comment by rowen on Thu 30 Oct 2008 06:41:40 PM CDT

This good to me so far (it does not yet address the exception hierarchy).

Two comments:

  • It is a pity that four different macros are required. Could we require that a descriptive string always be supplied? This would cut it down to two and the _ARGS could be removed.

KTL - There is now only one macro, plus two others to add traceback and to add a message if an exception is rethrown. The rethrow was not merged into the macro so that it is explicit and visible.

(Second comment removed as no longer relevant.)

Comment by smm on Fri 07 Nov 2008 03:33:08 PM CST

Can we have an addMessage() overload that takes a boost::format rather than a std::string? This is just for convenience - something similar was requested in #376.

KTL - It might be more compact to add this as an LSST_EXCEPT_ADD_FORMAT() macro taking the format and arguments. This can save writing boost::format explicitly. The only problem is that it is difficult to do this for LSST_EXCEPT() due to the varargs nature of that macro. But if you can write boost::format and str() for LSST_EXCEPT(), which is used the most, is it really necessary to avoid them for LSST_EXCEPT_ADD_MESSAGE()?

Comment by smm on Fri 07 Nov 2008 07:57:44 PM CST

Good point. So the only thing my suggestion would really save is the ().str() around a boost::format (and it would also require a constructor overload to work for LSST_EXCEPT). Having to write that is just a mild annoyance, so I'm fine with leaving it out.

Another question regarding the exception type name passed into the constructor: can't addToStream() just call typeid(*this).name() to produce that information? That should return a string that can be demangled to a canonical type name, whereas the macro captures whatever namespace alias the thrower has decided to use.

KTL - I wanted to do that, but I don't think there's a standard, portable way to demangle typeid names, which can be anything the compiler wants. G++ cxxabi.h abi::__cxa_demangle is not exactly something to rely on. An alternative could be to register the exception type-to-name mapping statically, but that seems like a lot of overhead to get rid of a namespace alias.

Comment by jmyers on Fri 14 Nov 2008 03:21:05 PM CST

I'm fairly happy with this design - it should be easy to separate from the rest of the LSST packaging and potentially use in MOPS code shared between LSST and PanSTARRS, or to generate a "dummy" version for PanSTARRS usage with a compatible interface but with simplified behavior.

Comment by Tim Axelrod on Mon 17 Nov 2008 12:29:41 PM CST

The example I'd like to see is where some main application routine calls a processing function which itself has some complicated call tree. Some member of this tree throws an exception e which is derived from pexExcept::Exception, and contains some data relevant to the circumstances that caused the exception to be thrown. The main application now catches the exception, and needs to deal with it depending on its type. I'm imagining a use of typeid(e), followed by some sort of case statement. Is that about right?

KTL - No, not exactly. It would look like this, depending on the exact hierarchy of exceptions that we define:

LSST_EXCEPTION_TYPE(MathError, Exception, lsst.pex.exceptions.MathError)
LSST_EXCEPTION_TYPE(DivideByZero, MathError, lsst.pex.exceptions.DivideByZero)
LSST_EXCEPTION_TYPE(MatrixSingular, MathError, lsst.pex.exceptions.MatrixSingular)

void lowLevel(void) {
    if (denominator == 0) {
        throw LSST_EXCEPT(DivideByZero, "Denominator was zero");
    }
}

void process(void) {
    lowLevel(void);
}

void top(int exposureId) {
    try {
        process();
    }
    catch (DivideByZero& e) {
        // I know exactly what to do with this low-level error.
        processAnotherWay();
    }
    catch (MathError& e) {
        // OK, something else occurred, perhaps a "MatrixSingular" exception.
        // I deal with it in a more general way here.
        processThirdWay();
    }
    catch (Exception& e) {
        // Something else happened.  Add my context information and re-throw.
        LSST_EXCEPT_ADD_MESSAGE(e,
            (boost::format("Exposure id = %1%") % exposureId).str());
        throw;
    }
}

Comment by rowen on Mon 17 Nov 2008 04:29:28 PM CST

Looks very good to me, though I would still prefer to always require a message; thus eliminate LSST_EXCEPT_ADD_MESSAGE and add the required message argument to LSST_EXCEPT_ADD. It simplifies the interface (one less macro and LSST_EXCEPT and LSST_ADD both take a message), encourages the user to provide a message, and if the user really has nothing additional to say they can specify a blank message "".

KTL - LSST_EXCEPT_ADD_HERE removed; LSST_EXCEPT_ADD_MESSAGE renamed to just LSST_EXCEPT_ADD.

Comment by rowen on Mon 17 Nov 2008 04:32:05 PM CST

One other comment: it would be very beneficial for the python exceptions to inherit from Exception (python's standard exception). This is the python standard thing to do, and it allows the standard paradigm of try/except Exception to catch all error-ish exceptions.

KTL - Unfortunately, this seems difficult to implement if we use SWIG to wrap the exception classes. I think we would instead have to have a completely separate Python exception hierarchy that is mapped (not wrapped) by SWIG from the C++ exceptions, with data copied from the C++ side to the Python side.

Comment by smm on Mon 17 Nov 2008 06:15:05 PM CST

There's no way to add a message without a tracepoint and vice-versa now, so should message become part of Tracepoint?

KTL - I would have liked to do this, but it poses difficulties for the varargs macro.

Regarding the Python interface - how about postprocessing the python code generated by swig to add in appropriate base classes? You'd only have to do this for a few (perhaps just one) class, since most of the exceptions would pick up the appropriate base indirectly.

KTL - This could work, but changing the SWIG Python code, even in this small way, introduces difficulties with build procedures and dependencies on just how SWIG generates its code. I'd rather find a cleaner way if at all possible.

Comment by RayPlante on Tue 18 Nov 2008 10:42:01 AM CST

There's no way to add a message without a tracepoint and vice-versa now

This is good. Based on KT's response, I take it that one assumes that the size of the message vector is the same size as the tracepoint vector.

KTL - Correct. With LSST_EXCEPT_ADD_HERE before, an empty message was added, but that is no longer an issue.

When there are multiple messages in the message vector, what's shown when you do the following:

   std::cout << e;

All the messages? or just the last one?

I guess my preference would be just the last message by default. The way I would see using this is that the added message added at the higher level is a replacement message that provides an interpretation of the lower level error in light of the known context. Hopefully, the messages get more and more user-oriented as the exception rises.

KTL - All the messages. The thought is that the complete list will be more useful for debugging, which is the primary purpose of the messages.

Add comment