From c433284d27fa61485b4687512f1a9cf000ab3ddb Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Sat, 20 Dec 2014 15:29:04 +0100 Subject: [PATCH] Improve error reporting fixes #8136 refs #6070 --- debian/icinga2-common.dirs | 1 + debian/icinga2-common.postinst | 1 + icinga-app/icinga.cpp | 2 +- icinga2.spec | 1 + lib/base/CMakeLists.txt | 1 + lib/base/application.cpp | 111 +++++++++++++++++++++------------ lib/base/application.hpp | 7 ++- lib/base/exception.cpp | 96 +++++++++++++++++++++++++++- lib/base/exception.hpp | 68 +++++++------------- lib/base/tlsutility.hpp | 2 +- lib/config/configcompiler.cpp | 2 - 11 files changed, 199 insertions(+), 93 deletions(-) diff --git a/debian/icinga2-common.dirs b/debian/icinga2-common.dirs index 9a54fe9f4..e5107016c 100644 --- a/debian/icinga2-common.dirs +++ b/debian/icinga2-common.dirs @@ -5,6 +5,7 @@ var/lib/icinga2/api/repository var/lib/icinga2/api/zones var/log/icinga2 var/log/icinga2/compat/archives +var/log/icinga2/crash var/spool/icinga2 var/spool/icinga2/perfdata var/spool/icinga2/tmp diff --git a/debian/icinga2-common.postinst b/debian/icinga2-common.postinst index d77ff1400..8375e7dce 100644 --- a/debian/icinga2-common.postinst +++ b/debian/icinga2-common.postinst @@ -37,6 +37,7 @@ case "$1" in setperm nagios nagios 0750 /etc/icinga2 setperm nagios adm 2751 /var/log/icinga2 + setperm nagios adm 2750 /var/log/icinga2/crash setperm nagios adm 2751 /var/log/icinga2/compat setperm nagios adm 2755 /var/log/icinga2/compat/archives diff --git a/icinga-app/icinga.cpp b/icinga-app/icinga.cpp index b58a6552c..8a009ca21 100644 --- a/icinga-app/icinga.cpp +++ b/icinga-app/icinga.cpp @@ -312,7 +312,7 @@ int Main(void) if (vm.count("version")) { std::cout << std::endl; - Application::DisplayInfoMessage(true); + Application::DisplayInfoMessage(std::cout, true); return EXIT_SUCCESS; } diff --git a/icinga2.spec b/icinga2.spec index ddb325d7d..84129a628 100644 --- a/icinga2.spec +++ b/icinga2.spec @@ -462,6 +462,7 @@ exit 0 %attr(0750,%{icinga_user},%{icingacmd_group}) %{_localstatedir}/cache/%{name} %attr(0750,%{icinga_user},%{icingacmd_group}) %dir %{_localstatedir}/log/%{name} +%attr(0750,%{icinga_user},%{icinga_group}) %dir %{_localstatedir}/log/%{name}/crash %attr(0750,%{icinga_user},%{icingacmd_group}) %dir %{_localstatedir}/log/%{name}/compat %attr(0750,%{icinga_user},%{icingacmd_group}) %dir %{_localstatedir}/log/%{name}/compat/archives %attr(0750,%{icinga_user},%{icinga_group}) %{_localstatedir}/lib/%{name} diff --git a/lib/base/CMakeLists.txt b/lib/base/CMakeLists.txt index 376f8ab27..0828d93fa 100644 --- a/lib/base/CMakeLists.txt +++ b/lib/base/CMakeLists.txt @@ -71,6 +71,7 @@ set_target_properties ( ) install(CODE "file(MAKE_DIRECTORY \"\$ENV{DESTDIR}${CMAKE_INSTALL_FULL_LOCALSTATEDIR}/cache/icinga2\")") +install(CODE "file(MAKE_DIRECTORY \"\$ENV{DESTDIR}${CMAKE_INSTALL_FULL_LOCALSTATEDIR}/log/icinga2/crash\")") install( TARGETS base diff --git a/lib/base/application.cpp b/lib/base/application.cpp index b55f65d20..3bd1fac99 100644 --- a/lib/base/application.cpp +++ b/lib/base/application.cpp @@ -29,14 +29,15 @@ #include "base/convert.hpp" #include "base/scriptglobal.hpp" #include "base/process.hpp" -#include #include #include #include #include #include #include +#include #include +#include using namespace icinga; @@ -474,35 +475,35 @@ String Application::GetExePath(const String& argv0) /** * Display version and path information. */ -void Application::DisplayInfoMessage(bool skipVersion) +void Application::DisplayInfoMessage(std::ostream& os, bool skipVersion) { - std::cerr << "Application information:" << std::endl; + os << "Application information:" << "\n"; if (!skipVersion) - std::cerr << " Application version: " << GetVersion() << std::endl; + os << " Application version: " << GetVersion() << "\n"; - std::cerr << " Installation root: " << GetPrefixDir() << std::endl - << " Sysconf directory: " << GetSysconfDir() << std::endl - << " Run directory: " << GetRunDir() << std::endl - << " Local state directory: " << GetLocalStateDir() << std::endl - << " Package data directory: " << GetPkgDataDir() << std::endl - << " State path: " << GetStatePath() << std::endl - << " Objects path: " << GetObjectsPath() << std::endl - << " Vars path: " << GetVarsPath() << std::endl - << " PID path: " << GetPidPath() << std::endl - << " Application type: " << GetApplicationType() << std::endl; + os << " Installation root: " << GetPrefixDir() << "\n" + << " Sysconf directory: " << GetSysconfDir() << "\n" + << " Run directory: " << GetRunDir() << "\n" + << " Local state directory: " << GetLocalStateDir() << "\n" + << " Package data directory: " << GetPkgDataDir() << "\n" + << " State path: " << GetStatePath() << "\n" + << " Objects path: " << GetObjectsPath() << "\n" + << " Vars path: " << GetVarsPath() << "\n" + << " PID path: " << GetPidPath() << "\n" + << " Application type: " << GetApplicationType() << "\n"; } /** * Displays a message that tells users what to do when they encounter a bug. */ -void Application::DisplayBugMessage(void) +void Application::DisplayBugMessage(std::ostream& os) { - std::cerr << "***" << std::endl - << "* This would indicate a runtime problem or configuration error. If you believe this is a bug in Icinga 2" << std::endl - << "* please submit a bug report at https://dev.icinga.org/ and include this stack trace as well as any other" << std::endl - << "* information that might be useful in order to reproduce this problem." << std::endl - << "***" << std::endl; + os << "***" << "\n" + << "* This would indicate a runtime problem or configuration error. If you believe this is a bug in Icinga 2" << "\n" + << "* please submit a bug report at https://dev.icinga.org/ and include this stack trace as well as any other" << "\n" + << "* information that might be useful in order to reproduce this problem." << "\n" + << "***" << "\n"; } #ifndef _WIN32 @@ -538,6 +539,11 @@ void Application::SigUsr1Handler(int) RequestReopenLogs(); } +String Application::GetCrashReportFilename(void) +{ + return GetLocalStateDir() + "/log/icinga2/crash/report." + Convert::ToString(Utility::GetTime()); +} + /** * Signal handler for SIGABRT. Helps with debugging ASSERT()s. * @@ -556,13 +562,22 @@ void Application::SigAbrtHandler(int) << "Current time: " << Utility::FormatDateTime("%Y-%m-%d %H:%M:%S %z", Utility::GetTime()) << std::endl << std::endl; - DisplayInfoMessage(); + String fname = GetCrashReportFilename(); + std::ofstream ofs; + ofs.open(fname.CStr()); + + Log(LogCritical, "Application") + << "Icinga 2 has terminated unexpectedly. Additional information can be found in '" << fname << "'" << "\n"; + + DisplayInfoMessage(ofs); StackTrace trace; - std::cerr << "Stacktrace:" << std::endl; - trace.Print(std::cerr, 1); + ofs << "Stacktrace:" << "\n"; + trace.Print(ofs, 1); - DisplayBugMessage(); + DisplayBugMessage(ofs); + + ofs.close(); } #else /* _WIN32 */ /** @@ -601,21 +616,32 @@ void Application::ExceptionHandler(void) sigaction(SIGABRT, &sa, NULL); #endif /* _WIN32 */ - std::cerr << "Caught unhandled exception." << std::endl - << "Current time: " << Utility::FormatDateTime("%Y-%m-%d %H:%M:%S %z", Utility::GetTime()) << std::endl - << std::endl; + String fname = GetCrashReportFilename(); + std::ofstream ofs; + ofs.open(fname.CStr()); + + ofs << "Caught unhandled exception." << "\n" + << "Current time: " << Utility::FormatDateTime("%Y-%m-%d %H:%M:%S %z", Utility::GetTime()) << "\n" + << "\n"; - DisplayInfoMessage(); + DisplayInfoMessage(ofs); try { RethrowUncaughtException(); } catch (const std::exception& ex) { - std::cerr << std::endl - << DiagnosticInformation(ex) - << std::endl; + Log(LogCritical, "Application") + << DiagnosticInformation(ex, false) << "\n" + << "\n" + << "Additional information is available in '" << fname << "'" << "\n"; + + ofs << "\n" + << DiagnosticInformation(ex) + << "\n"; } - DisplayBugMessage(); + DisplayBugMessage(ofs); + + ofs.close(); abort(); } @@ -628,17 +654,24 @@ LONG CALLBACK Application::SEHUnhandledExceptionFilter(PEXCEPTION_POINTERS exi) l_InExceptionHandler = true; - DisplayInfoMessage(); + String fname = GetCrashReportFilename(); + std::ofstream ofs; + ofs.open(fname.CStr()); - std::cerr << "Caught unhandled SEH exception." << std::endl - << "Current time: " << Utility::FormatDateTime("%Y-%m-%d %H:%M:%S %z", Utility::GetTime()) << std::endl - << std::endl; + Log(LogCritical, "Application") + << "Icinga 2 has terminated unexpectedly. Additional information can be found in '" << fname << "'"; + + DisplayInfoMessage(ofs); + + ofs << "Caught unhandled SEH exception." << "\n" + << "Current time: " << Utility::FormatDateTime("%Y-%m-%d %H:%M:%S %z", Utility::GetTime()) << "\n" + << "\n"; StackTrace trace(exi); - std::cerr << "Stacktrace:" << std::endl; - trace.Print(std::cerr, 1); + ofs << "Stacktrace:" << "\n"; + trace.Print(ofs, 1); - DisplayBugMessage(); + DisplayBugMessage(ofs); return EXCEPTION_CONTINUE_SEARCH; } diff --git a/lib/base/application.hpp b/lib/base/application.hpp index 2a484c9ea..4be3d059e 100644 --- a/lib/base/application.hpp +++ b/lib/base/application.hpp @@ -25,6 +25,7 @@ #include "base/threadpool.hpp" #include "base/utility.hpp" #include "base/logger.hpp" +#include namespace icinga { @@ -133,7 +134,7 @@ public: static double GetStartTime(void); static void SetStartTime(double ts); - static void DisplayInfoMessage(bool skipVersion = false); + static void DisplayInfoMessage(std::ostream& os, bool skipVersion = false); protected: virtual void OnConfigLoaded(void); @@ -169,11 +170,13 @@ private: static LONG WINAPI SEHUnhandledExceptionFilter(PEXCEPTION_POINTERS exi); #endif /* _WIN32 */ - static void DisplayBugMessage(void); + static void DisplayBugMessage(std::ostream& os); static void SigAbrtHandler(int signum); static void SigUsr1Handler(int signum); static void ExceptionHandler(void); + + static String GetCrashReportFilename(void); }; } diff --git a/lib/base/exception.cpp b/lib/base/exception.cpp index a694b18ce..fa41bafe7 100644 --- a/lib/base/exception.cpp +++ b/lib/base/exception.cpp @@ -130,7 +130,59 @@ void icinga::SetLastExceptionContext(const ContextTrace& context) l_LastExceptionContext.reset(new ContextTrace(context)); } -String icinga::DiagnosticInformation(boost::exception_ptr eptr) +String icinga::DiagnosticInformation(const std::exception& ex, bool verbose, StackTrace *stack, ContextTrace *context) +{ + std::ostringstream result; + + const user_error *uex = dynamic_cast(&ex); + + String message = ex.what(); + + if (verbose) + result << boost::diagnostic_information(ex); + else + result << "Error: " << message; + + const ScriptError *dex = dynamic_cast(&ex); + + if (dex && !dex->GetDebugInfo().Path.IsEmpty()) { + result << "\nLocation:\n"; + ShowCodeFragment(result, dex->GetDebugInfo()); + } + + const posix_error *pex = dynamic_cast(&ex); + + if (!uex && verbose) { + const StackTrace *st = boost::get_error_info(ex); + + if (st) { + result << *st; + } else { + result << std::endl; + + if (!stack) + stack = GetLastExceptionStack(); + + if (stack) + result << *stack; + + } + + if (boost::get_error_info(ex) == NULL) { + result << std::endl; + + if (!context) + context = GetLastExceptionContext(); + + if (context) + result << *context; + } + } + + return result.str(); +} + +String icinga::DiagnosticInformation(boost::exception_ptr eptr, bool verbose) { StackTrace *pt = GetLastExceptionStack(); StackTrace stack; @@ -147,7 +199,7 @@ String icinga::DiagnosticInformation(boost::exception_ptr eptr) try { boost::rethrow_exception(eptr); } catch (const std::exception& ex) { - return DiagnosticInformation(ex, pt ? &stack : NULL, pc ? &context : NULL); + return DiagnosticInformation(ex, verbose, pt ? &stack : NULL, pc ? &context : NULL); } return boost::diagnostic_information(eptr); @@ -174,3 +226,43 @@ DebugInfo ScriptError::GetDebugInfo(void) const return m_DebugInfo; } +posix_error::posix_error(void) + : m_Message(NULL) +{ } + +posix_error::~posix_error(void) throw() +{ + free(m_Message); +} + +const char *posix_error::what(void) const throw() +{ + if (!m_Message) { + std::ostringstream msgbuf; + + const char * const *func = boost::get_error_info(*this); + + if (func) + msgbuf << "Function call '" << *func << "'"; + else + msgbuf << "Function call"; + + const std::string *fname = boost::get_error_info(*this); + + if (fname) + msgbuf << " for file '" << *fname << "'"; + + msgbuf << " failed"; + + const int *errnum = boost::get_error_info(*this); + + if (errnum) + msgbuf << " with error code " << *errnum << ", '" << strerror(*errnum) << "'"; + + String str = msgbuf.str(); + m_Message = strdup(str.CStr()); + } + + return m_Message; +} + diff --git a/lib/base/exception.hpp b/lib/base/exception.hpp index 64d662e10..52306a7b0 100644 --- a/lib/base/exception.hpp +++ b/lib/base/exception.hpp @@ -71,58 +71,34 @@ I2_BASE_API void SetLastExceptionContext(const ContextTrace& context); I2_BASE_API void RethrowUncaughtException(void); typedef boost::error_info StackTraceErrorInfo; -typedef boost::error_info ContextTraceErrorInfo; -template -String DiagnosticInformation(const T& ex, StackTrace *stack = NULL, ContextTrace *context = NULL) +inline std::string to_string(const StackTraceErrorInfo& e) { - std::ostringstream result; - - const user_error *uex = dynamic_cast(&ex); - - String message = ex.what(); - - if (!uex || message.IsEmpty()) - result << boost::diagnostic_information(ex); - else - result << "Error: " << message; - - const ScriptError *dex = dynamic_cast(&ex); - - if (dex && !dex->GetDebugInfo().Path.IsEmpty()) { - result << "\nLocation:\n"; - ShowCodeFragment(result, dex->GetDebugInfo()); - } - - if (!uex) { - if (boost::get_error_info(ex) == NULL) { - result << std::endl; - - if (!stack) - stack = GetLastExceptionStack(); - - if (stack) - result << *stack; - - } + return ""; +} - if (boost::get_error_info(ex) == NULL) { - result << std::endl; +typedef boost::error_info ContextTraceErrorInfo; - if (!context) - context = GetLastExceptionContext(); +inline std::string to_string(const ContextTraceErrorInfo& e) +{ + std::ostringstream msgbuf; + msgbuf << "[Context] = " << e.value(); + return msgbuf.str(); +} - if (context) - result << *context; - } - } +I2_BASE_API String DiagnosticInformation(const std::exception& ex, bool verbose = true, StackTrace *stack = NULL, ContextTrace *context = NULL); +I2_BASE_API String DiagnosticInformation(boost::exception_ptr eptr, bool verbose = true); - return result.str(); -} +class I2_BASE_API posix_error : virtual public std::exception, virtual public boost::exception { +public: + posix_error(void); + virtual ~posix_error(void) throw(); -I2_BASE_API String DiagnosticInformation(boost::exception_ptr eptr); + virtual const char *what(void) const throw(); -class I2_BASE_API posix_error : virtual public std::exception, virtual public boost::exception { }; +private: + mutable char *m_Message; +}; #ifdef _WIN32 class I2_BASE_API win32_error : virtual public std::exception, virtual public boost::exception { }; @@ -132,7 +108,7 @@ typedef boost::error_info errinfo_win32_error; inline std::string to_string(const errinfo_win32_error& e) { - return Utility::FormatErrorNumber(e.value()); + return "[errinfo_win32_error] = " + Utility::FormatErrorNumber(e.value()) + "\n"; } #endif /* _WIN32 */ @@ -141,7 +117,7 @@ typedef boost::error_info errinfo_getadd inline std::string to_string(const errinfo_getaddrinfo_error& e) { - return gai_strerror(e.value()); + return "[errinfo_getaddrinfo_error] = " + String(gai_strerror(e.value())) + "\n"; } struct errinfo_message_; diff --git a/lib/base/tlsutility.hpp b/lib/base/tlsutility.hpp index 5fa121e99..34b6f1f77 100644 --- a/lib/base/tlsutility.hpp +++ b/lib/base/tlsutility.hpp @@ -68,7 +68,7 @@ inline std::string to_string(const errinfo_openssl_error& e) message = "Unknown error."; tmp << code << ", \"" << message << "\""; - return tmp.str(); + return "[errinfo_openssl_error]" + tmp.str() + "\n"; } } diff --git a/lib/config/configcompiler.cpp b/lib/config/configcompiler.cpp index 60e40e42a..7ff9cab8e 100644 --- a/lib/config/configcompiler.cpp +++ b/lib/config/configcompiler.cpp @@ -26,8 +26,6 @@ #include #include -using std::ifstream; - using namespace icinga; std::vector ConfigCompiler::m_IncludeSearchDirs; -- 2.40.0