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 thelog
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.
log
asnoexcept
orthrow()
or wrap it insidetry {
and} catch(...)
. – Pillagethrow()
/noexcept
, but in general it won't be, and needs to have thetry
/catch
. – Tinselstd::uncaught_exception()
) which is a dependency (I come from game development, so even dependency on the STL is still a dependency on foreign code!) but I'm the first one to admit that it's not a big deal. – Terrissbool
!) Which again, is absolutely no big deal in most (all?) situations. And obviously it needs more code. But I fail to see how using a lambda will "get uglier as you add more statements." It seems to me like the usage of both methods are written almost exactly the same: a block of arbitrary code, indented one tab. But the lambda approach does depend on the compiler to inline thetry_and_log
function, which again is not at an extraordinary thing to expect. – TerrissExceptionTriggeredLog
instance. I might be wrong but doesn't your object gets destroyed at the same line it was created? By the way, I think wrapping the above version in macros needs more care (because of the braces involved) and still more code for the macros. – Terrissbool
, ) and upon#include <exception>
and the symbolstd::uncaught_exception
. Given thatExceptionTriggeredLog
is intended to be used by-value in a small scope, and given that the methods would be implicitlyinline
, I assume any compiler would optimizate thebool
away. However,std::uncaught_exception
is called once for the "successful" path, and twice for the "error" path; I'm unsure if this function is inline-able, if this interacts with the ( somewhat slower ) exception-logic, etc. So, that is a good critique. – TinselExceptionTriggeredLog
, then the normal logic. The proposed 'try_and_log' solution adds the normal code inside as an argument; I find this.... a bit awkward. I'm familiar with functors, but don't use them regularly. Perhaps as lambda become more wide-spread, due to their convenience, this would not be an issue for me. – Tinsel