How should one log when an exception is triggered?

2020-02-14 02:25发布

问题:

In a program I recently wrote, I wanted to log when my "business logic" code triggered an exception in third-party or project APIs. ( To clarify, I want to log when use of an an API causes an exception. This can be many frames above the actual throw, and may be many frames below the actual catch ( where logging of the exception payload can occur. ) ) I did the following:

void former_function()
{
    /* some code here */
    try
    {
       /* some specific code that I know may throw, and want to log about */
    }
    catch( ... )
    {
       log( "an exception occurred when doing something with some other data" );
       throw;
    }
    /* some code here */
}

In short, if an exception occurs, create a catch-all clause, log the error, and re-throw. In my mind this is safe. I know in general catch-all is considered bad, since one doesn't have a reference to the exception at all to get any useful information. However, I'm just going to re-throw it, so nothing is lost.

Now, on its own it was fine, but some other programmers modified this program, and ended up violating the above. Specifically, they put a huge amount of code into the try-block in one case, and in another removed the 'throw' and placed a 'return'.

I see now my solution was brittle; it wasn't future-modification-proof.

I want a better solution that does not have these problems.

I have another potential solution that doesn't have the above issue, but I wonder what others think of it. It uses RAII, specifically a "Scoped Exit" object that implicitly triggers if std::uncaught_exception is not true on construction, yet is true on destruction:

#include <ciso646> // not, and
#include <exception> // uncaught_exception

class ExceptionTriggeredLog
{
private:
    std::string const m_log_message;
    bool const m_was_uncaught_exception;
public:
    ExceptionTriggeredLog( std::string const& r_log_message )
      : m_log_message( r_log_message ),
        m_was_uncaught_exception( std::uncaught_exception() )
    {
    }
    ~ExceptionTriggeredLog()
    {
        if( not m_was_uncaught_exception
            and std::uncaught_exception() )
        {
            try
            {
                log( m_log_message );
            }
            catch( ... )
            {
                // no exceptions can leave an destructor.
                // especially when std::uncaught_exception is true.
            }
        }
    }
};

void potential_function()
{
    /* some code here */
    {
       ExceptionTriggeredLog exception_triggered_log( "an exception occurred when doing something with some other data" );
       /* some specific code that I know may throw, and want to log about */
    }
    /* some code here */
}

I want to know:

  • technically, would this work robustly? Initially it seems to work, but I know there are some caveats about using std::uncaught_exception.
  • is there another way to accomplish what I want?

Note: I've updated this question. Specifically, I've:

  • added the try/catch that was initially missing, around the log function-call.
  • added tracking the std::uncaught_exception state on construction. This guards against the case where this object is created inside a 'try' block of another destructor which is triggered as part of exception stack-unwinding.
  • fixed the new 'potential_function' to create a named object, not a temporary object as before.

回答1:

I have no comment on your method, but it seems interesting! I have another way that might also work for what you want, and might be a little more general-purpose. It requires lambdas from C++11 though, which might or might not be an issue in your case.

It's a simple function template that accepts a lambda, runs it and catches, logs and rethrows all exceptions:

template <typename F>
void try_and_log (char const * log_message, F code_block)
{
    try {
        code_block ();
    } catch (...) {
        log (log_message);
        throw;
    }
}

The way you use it (in the simplest case) is like this:

try_and_log ("An exception was thrown here...", [&] {
    this_is_the_code ();
    you_want_executed ();
    and_its_exceptions_logged ();
});

As I said before, I don't know how it stacks against your own solution. Note that the lambda is capturing everything from its enclosing scope, which is quite convenient. Also note that I haven't actually tried this, so compile errors, logical problems and/or nuclear wars may result from this.

The problem I see here is that it is not easy to wrap this into a macro, and expecting your colleagues to write the [=] { and } parts correctly and all the time might be too much!

For wrapping and idiot-proofing purposes, you'll probably need two macros: a TRY_AND_LOG_BEGIN to emit the first line till the opening brace for the lambda and an TRY_AND_LOG_END to emit the closing brace and parenthesis. Like so:

#define TRY_AND_LOG_BEGIN(message)  try_and_log (message, [&] {
#define TRY_AND_LOG_END()           })

And you use them like this:

TRY_AND_LOG_BEGIN ("Exception happened!")  // No semicolons here!
    whatever_code_you_want ();
TRY_AND_LOG_END ();

Which is - depending on your perspective - is either a net gain or a net loss! (I personally prefer the straightforward function call and lambda syntax, which gives me more control and transparency.

Also, it is possible to write the log message at the end of the code block; just switch the two parameters of the try_and_log function.