From 1ed76c382e8b87bff02b6d37cf8acd7f6b1f8063 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 10:45:06 -0400 Subject: MAINT-5011: Add llexception_test.cpp with tests (and conclusions). llexception_test.cpp is an unusual test source in that it need not be verified on every build, so its invocation in indra/llcommon/CMakeLists.txt is commented out with that remark. Its purpose is to help a developer decide what base class(es) to use for LLException, how to throw and how to catch. Our current conclusions are written up as comments in llexception_test.cpp. Added CRASH_ON_UNHANDLED_EXCEPTION() and LOG_UNHANDLED_EXCEPTION() macros to llexception.h -- macros to log __FILE__, __LINE__ and __PRETTY_FUNCTION__ of the catch site. These invoke functions in llexception.cpp so we don't need to #include llerror.h for every possible catch site. --- indra/llcommon/llexception.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 indra/llcommon/llexception.cpp (limited to 'indra/llcommon/llexception.cpp') diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp new file mode 100644 index 0000000000..f48509b2aa --- /dev/null +++ b/indra/llcommon/llexception.cpp @@ -0,0 +1,41 @@ +/** + * @file llexception.cpp + * @author Nat Goodspeed + * @date 2016-08-12 + * @brief Implementation for llexception. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llexception.h" +// STL headers +// std headers +#include +// external library headers +#include +// other Linden headers +#include "llerror.h" + +void crash_on_unhandled_exception_(const char* file, int line, const char* pretty_function) +{ + // LL_ERRS() terminates, but also propagates message into crash dump. + LL_ERRS() << file << "(" << line << "): Unhandled exception caught in " << pretty_function + << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; +} + +void log_unhandled_exception_(const char* file, int line, const char* pretty_function, + const LLContinueError& e) +{ + // Use LL_WARNS() because we seriously do not expect this to happen + // routinely, but we DO expect to return from this function. Deriving your + // exception from LLContinueError implies that such an exception should + // NOT be fatal to the viewer, only to its current task. + LL_WARNS() << file << "(" << line << "): Unhandled " << typeid(e).name() + << " exception caught in " << pretty_function + << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; +} -- cgit v1.2.3 From 993f54f6e91d78a9c2e1389ad878d6bd46e9be5b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 17 Aug 2016 15:40:03 -0400 Subject: MAINT-5011: Try to enrich catch (...) logging throughout viewer. Turns out we have a surprising number of catch (...) clauses in the viewer code base. If all we currently do is LL_ERRS() << "unknown exception" << LL_ENDL; then call CRASH_ON_UNHANDLED_EXCEPTION() instead. If what we do is LL_WARNS() << "unknown exception" << LL_ENDL; then call LOG_UNHANDLED_EXCEPTION() instead. Since many places need LOG_UNHANDLED_EXCEPTION() and nobody catches LLContinueError yet, eliminate LLContinueError& parameter from LOG_UNHANDLED_EXCEPTION(). This permits us to use the same log message as CRASH_ON_UNHANDLED_EXCEPTION(), just with a different severity level. Where a catch (...) clause actually provides contextual information, or makes an error string, add boost::current_exception_diagnostic_information() to try to figure out actual exception class and message. --- indra/llcommon/llexception.cpp | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) (limited to 'indra/llcommon/llexception.cpp') diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp index f48509b2aa..9a6dfee3f1 100644 --- a/indra/llcommon/llexception.cpp +++ b/indra/llcommon/llexception.cpp @@ -21,21 +21,28 @@ // other Linden headers #include "llerror.h" +namespace { +// used by crash_on_unhandled_exception_() and log_unhandled_exception_() +void log_unhandled_exception_(LLError::ELevel level, + const char* file, int line, const char* pretty_function) +{ + // log same message but allow caller-specified severity level + // lllog() is the macro underlying LL_ERRS(), LL_WARNS() et al. + lllog(level, false) << file << "(" << line << "): Unhandled exception caught in " + << pretty_function + << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; +} +} + void crash_on_unhandled_exception_(const char* file, int line, const char* pretty_function) { - // LL_ERRS() terminates, but also propagates message into crash dump. - LL_ERRS() << file << "(" << line << "): Unhandled exception caught in " << pretty_function - << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; + // LL_ERRS() terminates and propagates message into crash dump. + log_unhandled_exception_(LLError::LEVEL_ERROR, file, line, pretty_function); } -void log_unhandled_exception_(const char* file, int line, const char* pretty_function, - const LLContinueError& e) +void log_unhandled_exception_(const char* file, int line, const char* pretty_function) { // Use LL_WARNS() because we seriously do not expect this to happen - // routinely, but we DO expect to return from this function. Deriving your - // exception from LLContinueError implies that such an exception should - // NOT be fatal to the viewer, only to its current task. - LL_WARNS() << file << "(" << line << "): Unhandled " << typeid(e).name() - << " exception caught in " << pretty_function - << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; + // routinely, but we DO expect to return from this function. + log_unhandled_exception_(LLError::LEVEL_WARN, file, line, pretty_function); } -- cgit v1.2.3 From 4d10172d8b2c72fa809e322a3b4ff326b19ff340 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 18 Aug 2016 17:33:44 -0400 Subject: MAINT-5011: Catch unhandled exceptions in LLCoros coroutines. Wrap coroutine call in try/catch in top-level coroutine wrapper function LLCoros::toplevel(). Distinguish exception classes derived from LLContinueError (log and continue) from all others (crash with LL_ERRS). Enhance CRASH_ON_UNHANDLED_EXCEPTIONS() and LOG_UNHANDLED_EXCEPTIONS() macros to accept a context string to supplement the log message. This lets us replace many places that called boost::current_exception_diagnostic_information() with LOG_UNHANDLED_EXCEPTIONS() instead, since the explicit calls were mostly to log supplemental information. Provide supplemental information (coroutine name, function parameters) for some of the previous LOG_UNHANDLED_EXCEPTIONS() calls. This information duplicates LL_DEBUGS() information at the top of these functions, but in a typical log file we wouldn't see the LL_DEBUGS() message. Eliminate a few catch (std::exception e) clauses: the information we get from boost::current_exception_diagnostic_information() in a catch (...) clause makes it unnecessary to distinguish. In a few cases, add a final 'throw;' to a catch (...) clause: having logged the local context info, propagate the exception to be caught by higher-level try/catch. In a couple places, couldn't resist reconciling indentation within a particular function: tabs where the rest of the function uses tabs, spaces where the rest of the function uses spaces. In LLLogin::Impl::loginCoro(), eliminate some confusing comments about an array of rewritten URIs that date back to a long-deleted implementation. --- indra/llcommon/llexception.cpp | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'indra/llcommon/llexception.cpp') diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp index 9a6dfee3f1..791b115a8c 100644 --- a/indra/llcommon/llexception.cpp +++ b/indra/llcommon/llexception.cpp @@ -24,25 +24,32 @@ namespace { // used by crash_on_unhandled_exception_() and log_unhandled_exception_() void log_unhandled_exception_(LLError::ELevel level, - const char* file, int line, const char* pretty_function) + const char* file, int line, const char* pretty_function, + const std::string& context) { // log same message but allow caller-specified severity level // lllog() is the macro underlying LL_ERRS(), LL_WARNS() et al. lllog(level, false) << file << "(" << line << "): Unhandled exception caught in " - << pretty_function - << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; + << pretty_function; + if (! context.empty()) + { + LL_CONT << ": " << context; + } + LL_CONT << ":\n" << boost::current_exception_diagnostic_information() << LL_ENDL; } } -void crash_on_unhandled_exception_(const char* file, int line, const char* pretty_function) +void crash_on_unhandled_exception_(const char* file, int line, const char* pretty_function, + const std::string& context) { // LL_ERRS() terminates and propagates message into crash dump. - log_unhandled_exception_(LLError::LEVEL_ERROR, file, line, pretty_function); + log_unhandled_exception_(LLError::LEVEL_ERROR, file, line, pretty_function, context); } -void log_unhandled_exception_(const char* file, int line, const char* pretty_function) +void log_unhandled_exception_(const char* file, int line, const char* pretty_function, + const std::string& context) { // Use LL_WARNS() because we seriously do not expect this to happen // routinely, but we DO expect to return from this function. - log_unhandled_exception_(LLError::LEVEL_WARN, file, line, pretty_function); + log_unhandled_exception_(LLError::LEVEL_WARN, file, line, pretty_function, context); } -- cgit v1.2.3 From 8118762c124d2de8db40836a28b8bf12ecbefbf7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 31 Aug 2016 17:16:43 -0400 Subject: MAINT-5011: Add log tag to LOG_UNHANDLED_EXCEPTION() log messages. --- indra/llcommon/llexception.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llexception.cpp') diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp index 791b115a8c..1c39c0f827 100644 --- a/indra/llcommon/llexception.cpp +++ b/indra/llcommon/llexception.cpp @@ -29,8 +29,8 @@ void log_unhandled_exception_(LLError::ELevel level, { // log same message but allow caller-specified severity level // lllog() is the macro underlying LL_ERRS(), LL_WARNS() et al. - lllog(level, false) << file << "(" << line << "): Unhandled exception caught in " - << pretty_function; + lllog(level, false, "LLException") + << file << "(" << line << "): Unhandled exception caught in " << pretty_function; if (! context.empty()) { LL_CONT << ": " << context; -- cgit v1.2.3 From 1ed351e28f1134ba06fd9459927d0d146e204fbf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 1 Sep 2016 19:46:39 -0400 Subject: MAINT-5011: Use LL_VLOGS() rather than raw lllog() macro. Raw lllog() doesn't work for varying log level, which is why LL_VLOGS() exists. --- indra/llcommon/llexception.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'indra/llcommon/llexception.cpp') diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp index 1c39c0f827..25aa5847a2 100644 --- a/indra/llcommon/llexception.cpp +++ b/indra/llcommon/llexception.cpp @@ -28,8 +28,7 @@ void log_unhandled_exception_(LLError::ELevel level, const std::string& context) { // log same message but allow caller-specified severity level - // lllog() is the macro underlying LL_ERRS(), LL_WARNS() et al. - lllog(level, false, "LLException") + LL_VLOGS(level, "LLException") << file << "(" << line << "): Unhandled exception caught in " << pretty_function; if (! context.empty()) { -- cgit v1.2.3 From 1804da89eea38615a4dd9532757b7ef7c35d2be6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 2 Sep 2016 14:00:18 -0400 Subject: MAINT-5011: Abbreviate __FILE__ path in log_unhandled_exception_(). LLError::abbreviateFile() is specifically to avoid cluttering log output with the prefix of an absolute file path on the original build system, pointless for anyone trying to read the log. --- indra/llcommon/llexception.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llexception.cpp') diff --git a/indra/llcommon/llexception.cpp b/indra/llcommon/llexception.cpp index 25aa5847a2..b32ec2c9c9 100644 --- a/indra/llcommon/llexception.cpp +++ b/indra/llcommon/llexception.cpp @@ -20,6 +20,7 @@ #include // other Linden headers #include "llerror.h" +#include "llerrorcontrol.h" namespace { // used by crash_on_unhandled_exception_() and log_unhandled_exception_() @@ -28,8 +29,8 @@ void log_unhandled_exception_(LLError::ELevel level, const std::string& context) { // log same message but allow caller-specified severity level - LL_VLOGS(level, "LLException") - << file << "(" << line << "): Unhandled exception caught in " << pretty_function; + LL_VLOGS(level, "LLException") << LLError::abbreviateFile(file) + << "(" << line << "): Unhandled exception caught in " << pretty_function; if (! context.empty()) { LL_CONT << ": " << context; -- cgit v1.2.3