From 448e82f39c472c82620bb52e0644e258b363d562 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 15 Jun 2018 16:48:20 -0400 Subject: SL-821: Try to add SecondLife.log file to Mac BugSplat crash report. Introduce new header file llappviewermacosx-for-objc.h to publish for llappdelegate-objc.mm and other Objective-C++ consumers the free functions in llappviewermacosx.cpp they consume. These were never before declared in any header file. Apparently, to date, we've been trusting to luck that Objective-C++ will infer the correct signature from calls -- and that the calls are correct with respect to the function definitions. :-P This gives us a place to introduce a new getLogFilePathname() function to query LLDir. (We don't simply #include "lldir.h" because of the pervasive use of BOOL in viewer headers; BOOL means something very different in Objective-C++.) --- indra/newview/llappviewermacosx.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'indra/newview/llappviewermacosx.cpp') diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index d472f8926b..cdbdb23d9a 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -36,6 +36,7 @@ #include "llappviewermacosx-objc.h" #include "llappviewermacosx.h" +#include "llappviewermacosx-for-objc.h" #include "llwindowmacosx-objc.h" #include "llcommandlineparser.h" @@ -147,6 +148,11 @@ void cleanupViewer() gViewerAppPtr = NULL; } +std::string getLogFilePathname() +{ + return gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.log"); +} + int main( int argc, char **argv ) { // Store off the command line args for use later. -- cgit v1.3 From 6e790fc27d22e80527789cf8e783be6c90a23505 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 18 Jun 2018 16:38:49 -0400 Subject: SL-821: Add logging in the macOS BugSplat attachment override. --- indra/newview/llappdelegate-objc.mm | 4 ++++ indra/newview/llappviewermacosx-for-objc.h | 1 + indra/newview/llappviewermacosx.cpp | 5 +++++ 3 files changed, 10 insertions(+) (limited to 'indra/newview/llappviewermacosx.cpp') diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 11a4b5d48e..2c2302ecfb 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -202,9 +202,12 @@ - (BugsplatAttachment *)attachmentForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { std::string logfile = getLogFilePathname(); + infos("Reached attachmentForBugsplatStartupManager with:"); + infos(logfile); NSString *ns_logfile = [NSString stringWithCString:logfile.c_str() encoding:NSUTF8StringEncoding]; NSData *data = [NSData dataWithContentsOfFile:ns_logfile]; + infos("Read logfile"); // Apologies for the hard-coded log-file basename, but I do not know the // incantation for "$(basename "$logfile")" in this language. @@ -212,6 +215,7 @@ [[BugsplatAttachment alloc] initWithFilename:@"SecondLife.log" attachmentData:data contentType:@"text/plain"]; + infos("returning attachment"); return attachment; } diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h index ef5d90bfef..e45cb85861 100644 --- a/indra/newview/llappviewermacosx-for-objc.h +++ b/indra/newview/llappviewermacosx-for-objc.h @@ -30,5 +30,6 @@ bool pumpMainLoop(); void handleQuit(); void cleanupViewer(); std::string getLogFilePathname(); +void infos(const std::string& message); #endif /* ! defined(LL_LLAPPVIEWERMACOSX_FOR_OBJC_H) */ diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index cdbdb23d9a..562e7ebfde 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -153,6 +153,11 @@ std::string getLogFilePathname() return gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.log"); } +void infos(const std::string& message) +{ + LL_INFOS() << message << LL_ENDL; +} + int main( int argc, char **argv ) { // Store off the command line args for use later. -- cgit v1.3 From d26c931ae2c5d33adc5fc20842b7be838a2822b4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 19 Jun 2018 15:08:56 -0400 Subject: SL-821: Send the SecondLife.log from the previous (crashed) run. Also clean up log messages. --- indra/newview/llappdelegate-objc.mm | 16 +++++++++++----- indra/newview/llappviewermacosx-for-objc.h | 2 +- indra/newview/llappviewermacosx.cpp | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) (limited to 'indra/newview/llappviewermacosx.cpp') diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 2ee294e1e6..4510f4070f 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -196,21 +196,27 @@ #if defined(LL_BUGSPLAT) +#if 0 +// Apparently this override method only contributes the User Description field +// of BugSplat's All Crashes table. Despite the method name, it would seem to +// be a bad place to try to stuff all of SecondLife.log. - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { // return NSStringFromSelector(_cmd); infos("Reached applicationLogForBugsplatStartupManager"); return @"[contents of SecondLife.log]"; } +#endif - (BugsplatAttachment *)attachmentForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { - std::string logfile = getLogFilePathname(); - infos("Reached attachmentForBugsplatStartupManager with:"); - infos(logfile); + // We get the *old* log file pathname (for SecondLife.old) because it's on + // the run *following* the crash that BugsplatStartupManager notices that + // the previous run crashed and calls this override. By that time, we've + // already renamed SecondLife.log to SecondLife.old. + std::string logfile = getOldLogFilePathname(); NSString *ns_logfile = [NSString stringWithCString:logfile.c_str() encoding:NSUTF8StringEncoding]; NSData *data = [NSData dataWithContentsOfFile:ns_logfile]; - infos("Read logfile"); // Apologies for the hard-coded log-file basename, but I do not know the // incantation for "$(basename "$logfile")" in this language. @@ -218,7 +224,7 @@ [[BugsplatAttachment alloc] initWithFilename:@"SecondLife.log" attachmentData:data contentType:@"text/plain"]; - infos("returning attachment"); + infos("attachmentForBugsplatStartupManager: attaching " + logfile); return attachment; } diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h index e45cb85861..c439297611 100644 --- a/indra/newview/llappviewermacosx-for-objc.h +++ b/indra/newview/llappviewermacosx-for-objc.h @@ -29,7 +29,7 @@ void handleUrl(const char* url_utf8); bool pumpMainLoop(); void handleQuit(); void cleanupViewer(); -std::string getLogFilePathname(); +std::string getOldLogFilePathname(); void infos(const std::string& message); #endif /* ! defined(LL_LLAPPVIEWERMACOSX_FOR_OBJC_H) */ diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 562e7ebfde..d014e992f9 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -148,9 +148,9 @@ void cleanupViewer() gViewerAppPtr = NULL; } -std::string getLogFilePathname() +std::string getOldLogFilePathname() { - return gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.log"); + return gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.old"); } void infos(const std::string& message) -- cgit v1.3 From cd52724ef8f8a19ebe28c73f39a582b56fb58093 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 28 Jun 2018 21:49:07 -0400 Subject: DRTVWR-447: Suppress BugSplat UI; auto-fill certain BugSplat data. Direct BugSplat to send crash reports without prompting, on both Windows and Mac. Add a mechanism by which code called after LL_ERRS() can retrieve the fatal log message string. (How did the crash logger extract that for Linden crash logging?) Add that fatal message to crash reports on Windows. But as BugsplatMac is engaged only on the run _after_ the crash, we no longer have that message in memory. Also add user name and region location to Windows crash reports. On Mac, (a) we don't have the information from the previous run and (b) BugsplatMac doesn't provide an API to attach that information to the crash report. Add Mac logging to indicate the success or failure of sending the crash report. Add Windows logging to indicate we're about to send. --- indra/llcommon/llerror.cpp | 33 ++++++++++++------- indra/llcommon/llerrorcontrol.h | 3 ++ indra/newview/llappdelegate-objc.mm | 34 +++++++++++++------ indra/newview/llappviewermacosx-for-objc.h | 2 ++ indra/newview/llappviewermacosx.cpp | 12 +++++++ indra/newview/llappviewerwin32.cpp | 53 +++++++++++++++++++++--------- 6 files changed, 101 insertions(+), 36 deletions(-) (limited to 'indra/newview/llappviewermacosx.cpp') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index f31a054139..b5e7e81f21 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -377,6 +377,7 @@ namespace public: std::ostringstream messageStream; bool messageStreamInUse; + std::string mFatalMessage; void addCallSite(LLError::CallSite&); void invalidateCallSites(); @@ -670,11 +671,16 @@ namespace LLError s->mCrashFunction = f; } - FatalFunction getFatalFunction() - { + FatalFunction getFatalFunction() + { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - return s->mCrashFunction; - } + return s->mCrashFunction; + } + + std::string getFatalMessage() + { + return Globals::getInstance()->mFatalMessage; + } void setTimeFunction(TimeFunction f) { @@ -1194,7 +1200,7 @@ namespace LLError { writeToRecorders(site, "error", true, true, true, false, false); } - + std::ostringstream message_stream; if (site.mPrintOnce) @@ -1219,14 +1225,19 @@ namespace LLError s->mUniqueLogMessages[message] = 1; } } - + message_stream << message; - - writeToRecorders(site, message_stream.str()); - - if (site.mLevel == LEVEL_ERROR && s->mCrashFunction) + std::string message_line(message_stream.str()); + + writeToRecorders(site, message_line); + + if (site.mLevel == LEVEL_ERROR) { - s->mCrashFunction(message_stream.str()); + g->mFatalMessage = message_line; + if (s->mCrashFunction) + { + s->mCrashFunction(message_line); + } } } } diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index caf2ba72c2..ddbcdc94a0 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -102,6 +102,9 @@ namespace LLError LL_COMMON_API FatalFunction getFatalFunction(); // Retrieve the previously-set FatalFunction + LL_COMMON_API std::string getFatalMessage(); + // Retrieve the message last passed to FatalFunction, if any + /// temporarily override the FatalFunction for the duration of a /// particular scope, e.g. for unit tests class LL_COMMON_API OverrideFatalFunction diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 4510f4070f..82e49540a4 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -74,9 +74,9 @@ #if defined(LL_BUGSPLAT) // https://www.bugsplat.com/docs/platforms/os-x#initialization -// [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES; -// [BugsplatStartupManager sharedManager].askUserDetails = NO; - [BugsplatStartupManager sharedManager].delegate = self; + [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES; + [BugsplatStartupManager sharedManager].askUserDetails = NO; + [BugsplatStartupManager sharedManager].delegate = self; [[BugsplatStartupManager sharedManager] start]; #endif } @@ -196,17 +196,20 @@ #if defined(LL_BUGSPLAT) -#if 0 -// Apparently this override method only contributes the User Description field -// of BugSplat's All Crashes table. Despite the method name, it would seem to -// be a bad place to try to stuff all of SecondLife.log. - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { -// return NSStringFromSelector(_cmd); infos("Reached applicationLogForBugsplatStartupManager"); - return @"[contents of SecondLife.log]"; + // Apparently this override method only contributes the User Description + // field of BugSplat's All Crashes table. Despite the method name, it + // would seem to be a bad place to try to stuff all of SecondLife.log. + return [NSString stringWithCString:getFatalMessage().c_str() + encoding:NSUTF8StringEncoding]; +} + +- (void)bugsplatStartupManagerWillSendCrashReport:(BugsplatStartupManager *)bugsplatStartupManager +{ + infos("Reached bugsplatStartupManagerWillSendCrashReport"); } -#endif - (BugsplatAttachment *)attachmentForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { // We get the *old* log file pathname (for SecondLife.old) because it's on @@ -228,6 +231,17 @@ return attachment; } +- (void)bugsplatStartupManagerDidFinishSendingCrashReport:(BugsplatStartupManager *)bugsplatStartupManager +{ + infos("Sent crash report to BugSplat"); +} + +- (void)bugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager didFailWithError:(NSError *)error +{ + // TODO: message string from NSError + infos("Could not send crash report to BugSplat"); +} + #endif // LL_BUGSPLAT @end diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h index c439297611..ac85d7e8c3 100644 --- a/indra/newview/llappviewermacosx-for-objc.h +++ b/indra/newview/llappviewermacosx-for-objc.h @@ -30,6 +30,8 @@ bool pumpMainLoop(); void handleQuit(); void cleanupViewer(); std::string getOldLogFilePathname(); +std::string getFatalMessage(); +std::string getAgentFullname(); void infos(const std::string& message); #endif /* ! defined(LL_LLAPPVIEWERMACOSX_FOR_OBJC_H) */ diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index d014e992f9..c3a3c3284a 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -45,6 +45,8 @@ #include "llmd5.h" #include "llfloaterworldmap.h" #include "llurldispatcher.h" +#include "llerrorcontrol.h" +#include "llvoavatarself.h" // for gAgentAvatarp->getFullname() #include #ifdef LL_CARBON_CRASH_HANDLER #include @@ -153,6 +155,16 @@ std::string getOldLogFilePathname() return gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.old"); } +std::string getFatalMessage() +{ + return LLError::getFatalMessage(); +} + +std::string getAgentFullname() +{ + return gAgentAvatarp? gAgentAvatarp->getFullname() : std::string(); +} + void infos(const std::string& message) { LL_INFOS() << message << LL_ENDL; diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index 247b94db3e..1e135fa229 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -67,6 +67,7 @@ #include "stringize.h" #include "lldir.h" +#include "llerrorcontrol.h" #include #include @@ -74,7 +75,10 @@ // Bugsplat (http://bugsplat.com) crash reporting tool #ifdef LL_BUGSPLAT #include "BugSplat.h" -#include "reader.h" // JsonCpp +#include "reader.h" // JsonCpp +#include "llagent.h" // for agent location +#include "llviewerregion.h" +#include "llvoavatarself.h" // for agent name namespace { @@ -85,7 +89,8 @@ namespace // std::basic_string instance will survive until the function returns. // Calling c_str() on a std::basic_string local to wunder() would be // Undefined Behavior: we'd be left with a pointer into a destroyed - // std::basic_string instance. + // std::basic_string instance. But we can do that with a macro... + #define WCSTR(string) wunder(string).c_str() // It would be nice if, when wchar_t is the same as __wchar_t, this whole // function would optimize away. However, we use it only for the arguments @@ -111,19 +116,35 @@ namespace bool bugsplatSendLog(UINT nCode, LPVOID lpVal1, LPVOID lpVal2) { - // If we haven't yet initialized LLDir, don't bother trying to - // find our log file. - // Alternatively -- if we might encounter trouble trying to query - // LLDir during crash cleanup -- consider making gDirUtilp an - // LLPounceable, and attach a callback that stores the pathname to - // the log file here. - if (nCode == MDSCB_EXCEPTIONCODE && gDirUtilp) + if (nCode == MDSCB_EXCEPTIONCODE) { // send the main viewer log file // widen to wstring, convert to __wchar_t, then pass c_str() sBugSplatSender->sendAdditionalFile( - wunder(gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.log")).c_str()); - } + WCSTR(gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.log"))); + + if (gAgentAvatarp) + { + // user name, when we have it + sBugSplatSender->setDefaultUserName(WCSTR(gAgentAvatarp->getFullname())); + } + + // LL_ERRS message, when there is one + sBugSplatSender->setDefaultUserDescription(WCSTR(LLError::getFatalMessage())); + + if (gAgent.getRegion()) + { + // region location, when we have it + LLVector3 loc = gAgent.getPositionAgent(); + sBugSplatSender->resetAppIdentifier( + WCSTR(STRINGIZE(gAgent.getRegion()->getName() + << '/' << loc.mV[0] + << '/' << loc.mV[1] + << '/' << loc.mV[2]))); + } + + LL_INFOS() << "Sending crash report to BugSplat." << LL_ENDL; + } // MDSCB_EXCEPTIONCODE return false; } @@ -603,10 +624,12 @@ bool LLAppViewerWin32::init() // have to convert normal wide strings to strings of __wchar_t sBugSplatSender = new MiniDmpSender( - wunder(BugSplat_DB.asString()).c_str(), - wunder(LL_TO_WSTRING(LL_VIEWER_CHANNEL)).c_str(), - wunder(version_string).c_str(), - nullptr); + WCSTR(BugSplat_DB.asString()), + WCSTR(LL_TO_WSTRING(LL_VIEWER_CHANNEL)), + WCSTR(version_string), + nullptr, // szAppIdentifier -- set later + MDSF_NONINTERACTIVE | // automatically submit report without prompting + MDSF_PREVENTHIJACKING); // disallow swiping Exception filter sBugSplatSender->setCallback(bugsplatSendLog); // engage stringize() overload that converts from wstring -- cgit v1.3 From 7dc014474de0c2d83a3cd314acd9dc0882622299 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 22 Aug 2018 10:48:29 -0400 Subject: DRTVWR-447: Attempt to post BugSplat metadata with Mac crash reports. Introduce CrashMetadata, an LLSingleton in llappviewermacosx.cpp, declared in llappviewermacosx-for-objc.h and accessed by the various BugsplatStartupManagerDelegate override methods. CrashMetadata is populated by reading the previous (presumably crashed) run's static_debug_info.log file. This replaces the previous getOldLogFilePathname(), getFatalMessage() and getAgentFullname() functions. To extend that suite for additional metadata, not only would we have to keep adding new free functions, but we'd have to keep rereading the static_debug_info.log file. Override the new applicationKeyForBugsplatStartupManager, defaultUserNameForBugsplatStartupManager, defaultUserEmailForBugsplatStartupManager methods to extract relevant fields from CrashMetadata. Change applicationLogForBugsplatStartupManager and attachmentForBugsplatStartupManager to do the same. Enhance llviewerregion.cpp to update the static_debug_info.log file every time we enter a new region. --- indra/newview/llappdelegate-objc.mm | 43 ++++++++++++++++++++++------ indra/newview/llappviewermacosx-for-objc.h | 21 ++++++++++++-- indra/newview/llappviewermacosx.cpp | 45 +++++++++++++++++++++++++----- indra/newview/llviewerregion.cpp | 19 +++++++++++++ 4 files changed, 109 insertions(+), 19 deletions(-) (limited to 'indra/newview/llappviewermacosx.cpp') diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 82e49540a4..ba697d0f77 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -199,10 +199,34 @@ - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { infos("Reached applicationLogForBugsplatStartupManager"); - // Apparently this override method only contributes the User Description - // field of BugSplat's All Crashes table. Despite the method name, it - // would seem to be a bad place to try to stuff all of SecondLife.log. - return [NSString stringWithCString:getFatalMessage().c_str() + // This strangely-named override method contributes the User Description + // metadata field. + return [NSString stringWithCString:CrashMetadata_instance().fatalMessage.c_str() + encoding:NSUTF8StringEncoding]; +} + +- (NSString *)applicationKeyForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager signal:(NSString *)signal exceptionName:(NSString *)exceptionName exceptionReason:(NSString *)exceptionReason { + // TODO: exceptionName, exceptionReason + + // Windows sends location within region as well, but that's because + // BugSplat for Windows intercepts crashes during the same run, and that + // information can be queried once. On the Mac, any metadata we have is + // written (and rewritten) to the static_debug_info.log file that we read + // at the start of the next viewer run. It seems ridiculously expensive to + // rewrite that file on every frame in which the avatar moves. + return [NSString stringWithCString:CrashMetadata_instance().regionName.c_str() + encoding:NSUTF8StringEncoding]; +} + +- (NSString *)defaultUserNameForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { + return [NSString stringWithCString:CrashMetadata_instance().agentFullname.c_str() + encoding:NSUTF8StringEncoding]; +} + +- (NSString *)defaultUserEmailForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { + // Use the email field for OS version, just as we do on Windows, until + // BugSplat provides more metadata fields. + return [NSString stringWithCString:CrashMetadata_instance().OSInfo.c_str() encoding:NSUTF8StringEncoding]; } @@ -212,11 +236,12 @@ } - (BugsplatAttachment *)attachmentForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { - // We get the *old* log file pathname (for SecondLife.old) because it's on - // the run *following* the crash that BugsplatStartupManager notices that - // the previous run crashed and calls this override. By that time, we've - // already renamed SecondLife.log to SecondLife.old. - std::string logfile = getOldLogFilePathname(); + std::string logfile = CrashMetadata_instance().logFilePathname; + // Still to do: + // userSettingsPathname + // staticDebugPathname + // but the BugsplatMac version 1.0.5 BugsplatStartupManagerDelegate API + // doesn't yet provide a way to attach more than one file. NSString *ns_logfile = [NSString stringWithCString:logfile.c_str() encoding:NSUTF8StringEncoding]; NSData *data = [NSData dataWithContentsOfFile:ns_logfile]; diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h index ac85d7e8c3..79da453cbe 100644 --- a/indra/newview/llappviewermacosx-for-objc.h +++ b/indra/newview/llappviewermacosx-for-objc.h @@ -29,9 +29,24 @@ void handleUrl(const char* url_utf8); bool pumpMainLoop(); void handleQuit(); void cleanupViewer(); -std::string getOldLogFilePathname(); -std::string getFatalMessage(); -std::string getAgentFullname(); void infos(const std::string& message); +// This struct is malleable; it only serves as a way to convey a number of +// fields from llappviewermacosx.cpp's CrashMetadata_instance() function to the +// consuming functions in llappdelegate-objc.mm. As long as both those sources +// are compiled with this same header, the content and order of CrashMetadata +// can change as needed. +struct CrashMetadata +{ + std::string logFilePathname; + std::string userSettingsPathname; + std::string staticDebugPathname; + std::string OSInfo; + std::string agentFullname; + std::string regionName; + std::string fatalMessage; +}; + +CrashMetadata& CrashMetadata_instance(); + #endif /* ! defined(LL_LLAPPVIEWERMACOSX_FOR_OBJC_H) */ diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index c3a3c3284a..7f7284a796 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -39,6 +39,7 @@ #include "llappviewermacosx-for-objc.h" #include "llwindowmacosx-objc.h" #include "llcommandlineparser.h" +#include "llsdserialize.h" #include "llviewernetwork.h" #include "llviewercontrol.h" @@ -53,6 +54,7 @@ #endif #include #include +#include #include "lldir.h" #include @@ -150,19 +152,48 @@ void cleanupViewer() gViewerAppPtr = NULL; } -std::string getOldLogFilePathname() +// The BugsplatMac API is structured as a number of different method +// overrides, each returning a different piece of metadata. But since we +// obtain such metadata by opening and parsing a file, it seems ridiculous to +// reopen and reparse it for every individual string desired. What we want is +// to open and parse the file once, retaining the data for subsequent +// requests. That's why this is an LLSingleton. +// Another approach would be to provide a function that simply returns +// CrashMetadata, storing the struct in LLAppDelegate, but nat doesn't know +// enough Objective-C++ to code that. We'd still have to detect which of the +// method overrides is called first so that the results are order-insensitive. +class CrashMetadataSingleton: public CrashMetadata, public LLSingleton { - return gDirUtilp->getExpandedFilename(LL_PATH_LOGS, "SecondLife.old"); -} + LLSINGLETON(CrashMetadataSingleton); +}; -std::string getFatalMessage() +// Populate the fields of our public base-class struct. +CrashMetadataSingleton::CrashMetadataSingleton() { - return LLError::getFatalMessage(); + // Note: we depend on being able to read the static_debug_info.log file + // from the *previous* run before we overwrite it with the new one for + // *this* run. LLAppViewer initialization must happen in the Right Order. + staticDebugPathname = *gViewerAppPtr->getStaticDebugFile(); + std::ifstream static_file(staticDebugPathname); + LLSD info; + if (static_file.is_open() && + LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED)) + { + logFilePathname = info["SLLog"].asString(); + userSettingsPathname = info["SettingsFilename"].asString(); + OSInfo = info["OSInfo"].asString(); + agentFullname = info["LoginName"].asString(); + // Translate underscores back to spaces + LLStringUtil::replaceChar(agentFullname, '_', ' '); + regionName = info["CurrentRegion"].asString(); + fatalMessage = info["FatalMessage"].asString(); + } } -std::string getAgentFullname() +// Avoid having to compile all of our LLSingleton machinery in Objective-C++. +CrashMetadata& CrashMetadata_instance() { - return gAgentAvatarp? gAgentAvatarp->getFullname() : std::string(); + return CrashMetadataSingleton::instance(); } void infos(const std::string& message) diff --git a/indra/newview/llviewerregion.cpp b/indra/newview/llviewerregion.cpp index b759c2a3ab..ca452fc766 100644 --- a/indra/newview/llviewerregion.cpp +++ b/indra/newview/llviewerregion.cpp @@ -44,6 +44,7 @@ #include "llagent.h" #include "llagentcamera.h" +#include "llappviewer.h" #include "llavatarrenderinfoaccountant.h" #include "llcallingcard.h" #include "llcommandhandler.h" @@ -104,6 +105,18 @@ typedef std::map CapabilityMap; static void log_capabilities(const CapabilityMap &capmap); +namespace +{ + +void newRegionEntry(LLViewerRegion& region) +{ + LL_INFOS("LLViewerRegion") << "Entering region [" << region.getName() << "]" << LL_ENDL; + gDebugInfo["CurrentRegion"] = region.getName(); + LLAppViewer::instance()->writeDebugInfo(); +} + +} // anonymous namespace + // support for secondlife:///app/region/{REGION} SLapps // N.B. this is defined to work exactly like the classic secondlife://{REGION} // However, the later syntax cannot support spaces in the region name because @@ -249,6 +262,9 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCoro(U64 regionHandle) return; // this error condition is not recoverable. } + // record that we just entered a new region + newRegionEntry(*regionp); + // After a few attempts, continue login. But keep trying to get the caps: if (mSeedCapAttempts >= mSeedCapMaxAttemptsBeforeLogin && STATE_SEED_GRANTED_WAIT == LLStartUp::getStartupState()) @@ -369,6 +385,9 @@ void LLViewerRegionImpl::requestBaseCapabilitiesCompleteCoro(U64 regionHandle) break; // this error condition is not recoverable. } + // record that we just entered a new region + newRegionEntry(*regionp); + LLSD capabilityNames = LLSD::emptyArray(); buildCapabilityNames(capabilityNames); -- cgit v1.3 From 8d4e6b6df0d21e18a24c8e1d9f6008c2092a24c5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 22 Aug 2018 16:16:26 -0400 Subject: DRTVWR-447: Additional logging getting metadata for previous run --- indra/newview/llappviewer.cpp | 7 ++----- indra/newview/llappviewermacosx.cpp | 33 +++++++++++++++++++++++++-------- 2 files changed, 27 insertions(+), 13 deletions(-) (limited to 'indra/newview/llappviewermacosx.cpp') diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 3e25b395c4..d324a82bf8 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -3051,14 +3051,11 @@ void LLAppViewer::writeDebugInfo(bool isStatic) ? getStaticDebugFile() : getDynamicDebugFile() ); - LL_INFOS() << "Opening debug file " << *debug_filename << LL_ENDL; - llofstream out_file(debug_filename->c_str()); + LL_INFOS() << "Writing debug file " << *debug_filename << LL_ENDL; + llofstream out_file(debug_filename->c_str()); isStatic ? LLSDSerialize::toPrettyXML(gDebugInfo, out_file) : LLSDSerialize::toPrettyXML(gDebugInfo["Dynamic"], out_file); - - - out_file.close(); } LLSD LLAppViewer::getViewerInfo() const diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 7f7284a796..77a16f7307 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -165,6 +165,14 @@ void cleanupViewer() class CrashMetadataSingleton: public CrashMetadata, public LLSingleton { LLSINGLETON(CrashMetadataSingleton); + + // convenience method to log each metadata field retrieved by constructor + std::string get_metadata(const LLSD& info, const LLSD::String& key) const + { + std::string data(info[key].asString()); + LL_INFOS() << " " << key << "='" << data << "'" << LL_ENDL; + return data; + } }; // Populate the fields of our public base-class struct. @@ -176,17 +184,26 @@ CrashMetadataSingleton::CrashMetadataSingleton() staticDebugPathname = *gViewerAppPtr->getStaticDebugFile(); std::ifstream static_file(staticDebugPathname); LLSD info; - if (static_file.is_open() && - LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED)) + if (! static_file.is_open()) + { + LL_INFOS() << "Can't open '" << staticDebugPathname + << "'; no metadata about previous run" << LL_ENDL; + } + else if (! LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED)) + { + LL_INFOS() << "Can't parse '" << staticDebugPathname + << "'; no metadata about previous run" << LL_ENDL; + } { - logFilePathname = info["SLLog"].asString(); - userSettingsPathname = info["SettingsFilename"].asString(); - OSInfo = info["OSInfo"].asString(); - agentFullname = info["LoginName"].asString(); + LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL; + logFilePathname = get_metadata(info, "SLLog"); + userSettingsPathname = get_metadata(info, "SettingsFilename"); + OSInfo = get_metadata(info, "OSInfo"); + agentFullname = get_metadata(info, "LoginName"); // Translate underscores back to spaces LLStringUtil::replaceChar(agentFullname, '_', ' '); - regionName = info["CurrentRegion"].asString(); - fatalMessage = info["FatalMessage"].asString(); + regionName = get_metadata(info, "CurrentRegion"); + fatalMessage = get_metadata(info, "FatalMessage"); } } -- cgit v1.3 From e674f11757ab55c5ca7aab4cb1c8e059fa98f466 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 23 Aug 2018 12:31:54 -0400 Subject: DRTVWR-447: Add (some) metadata to Mac crash reports. This required reordering certain operations during Mac viewer startup. Split llappviewermacosx.cpp's initViewer() function into constructViewer() (which instantiates LLAppViewerMacOSX) and initViewer() (which calls LLAppViewerMacOSX::init()). llappdelegate-objc.mm's applicationDidFinishLaunching override now calls [BugsplatStartupManager start] between constructViewer() and initViewer(): we want constructViewer() to have set up the logging subsystem so we can log the actions of BugsplatStartupManagerDelegate override methods, but otherwise we want BugsplatStartupManager in place as early as possible to catch any early crashes. Besides, initViewer() ends up overwriting the static_debug_info.log on which we depend for the *previous* run's crash metadata. Move the code that initializes the pathname of the static_debug_info.log file from LLAppViewerMacOSX::init() to the LLAppViewerMacOSX() constructor, since BugsplatStartupManagerDelegate override methods need to read (the previous run's) file. Add code to applicationLogForBugsplatStartupManager override to set new BugsplatMac 1.0.6 properties userName and userEmail. Don't log empty fields from static_debug_info.log if we couldn't read it. --- indra/newview/llappdelegate-objc.mm | 55 +++++++++++++++++++++++------- indra/newview/llappviewer.cpp | 29 +++++++++------- indra/newview/llappviewermacosx-for-objc.h | 1 + indra/newview/llappviewermacosx.cpp | 17 +++++---- 4 files changed, 69 insertions(+), 33 deletions(-) (limited to 'indra/newview/llappviewermacosx.cpp') diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 66bcf58961..f55304f30b 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -54,6 +54,25 @@ - (void) applicationDidFinishLaunching:(NSNotification *)notification { + // Call constructViewer() first so our logging subsystem is in place. This + // risks missing crashes in the LLAppViewerMacOSX constructor, but for + // present purposes it's more important to get the startup sequence + // properly logged. + // Someday I would like to modify the logging system so that calls before + // it's initialized are cached in a std::ostringstream and then, once it's + // initialized, "played back" into whatever handlers have been set up. + constructViewer(); + +#if defined(LL_BUGSPLAT) + // Engage BugsplatStartupManager *before* calling initViewer() to handle + // any crashes during initialization. + // https://www.bugsplat.com/docs/platforms/os-x#initialization + [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES; + [BugsplatStartupManager sharedManager].askUserDetails = NO; + [BugsplatStartupManager sharedManager].delegate = self; + [[BugsplatStartupManager sharedManager] start]; +#endif + frameTimer = nil; [self languageUpdated]; @@ -71,14 +90,6 @@ [[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(languageUpdated) name:@"NSTextInputContextKeyboardSelectionDidChangeNotification" object:nil]; // [[NSAppleEventManager sharedAppleEventManager] setEventHandler:self andSelector:@selector(handleGetURLEvent:withReplyEvent:) forEventClass:kInternetEventClass andEventID:kAEGetURL]; - -#if defined(LL_BUGSPLAT) - // https://www.bugsplat.com/docs/platforms/os-x#initialization - [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES; - [BugsplatStartupManager sharedManager].askUserDetails = NO; - [BugsplatStartupManager sharedManager].delegate = self; - [[BugsplatStartupManager sharedManager] start]; -#endif } - (void) handleGetURLEvent:(NSAppleEventDescriptor *)event withReplyEvent:(NSAppleEventDescriptor *)replyEvent { @@ -198,11 +209,29 @@ - (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager { - std::string fatalMessage(CrashMetadata_instance().fatalMessage); - infos("applicationLogForBugsplatStartupManager -> '" + fatalMessage + "'"); - // This strangely-named override method contributes the User Description - // metadata field. - return [NSString stringWithCString:fatalMessage.c_str() + CrashMetadata& meta(CrashMetadata_instance()); + // As of BugsplatMac 1.0.6, userName and userEmail properties are now + // exposed by the BugsplatStartupManager. Set them here, since the + // defaultUserNameForBugsplatStartupManager and + // defaultUserEmailForBugsplatStartupManager methods are called later, for + // the *current* run, rather than for the previous crashed run whose crash + // report we are about to send. + infos("applicationLogForBugsplatStartupManager setting userName = '" + + meta.agentFullname + '"'); + bugsplatStartupManager.userName = + [NSString stringWithCString:meta.agentFullname.c_str() + encoding:NSUTF8StringEncoding]; + // Use the email field for OS version, just as we do on Windows, until + // BugSplat provides more metadata fields. + infos("applicationLogForBugsplatStartupManager setting userEmail = '" + + meta.OSInfo + '"'); + bugsplatStartupManager.userEmail = + [NSString stringWithCString:meta.OSInfo.c_str() + encoding:NSUTF8StringEncoding]; + // This strangely-named override method's return value contributes the + // User Description metadata field. + infos("applicationLogForBugsplatStartupManager -> '" + meta.fatalMessage + "'"); + return [NSString stringWithCString:meta.fatalMessage.c_str() encoding:NSUTF8StringEncoding]; } diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index d324a82bf8..846b937a4e 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -707,6 +707,22 @@ LLAppViewer::LLAppViewer() // LLLoginInstance::instance().setPlatformInfo(gPlatform, LLOSInfo::instance().getOSVersionString(), LLOSInfo::instance().getOSStringSimple()); + + // Under some circumstances we want to read the static_debug_info.log file + // from the previous viewer run between this constructor call and the + // init() call, which will overwrite the static_debug_info.log file for + // THIS run. So setDebugFileNames() early. +#if LL_BUGSPLAT + // MAINT-8917: don't create a dump directory just for the + // static_debug_info.log file + std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, ""); +#else // ! LL_BUGSPLAT + // write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues. + std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, ""); +#endif // ! LL_BUGSPLAT + mDumpPath = logdir; + setMiniDumpDir(logdir); + setDebugFileNames(logdir); } LLAppViewer::~LLAppViewer() @@ -781,19 +797,6 @@ bool LLAppViewer::init() initMaxHeapSize() ; LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize")); -#if LL_BUGSPLAT - // MAINT-8917: don't create a dump directory just for the - // static_debug_info.log file - std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, ""); -#else // ! LL_BUGSPLAT - // write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues. - std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, ""); -#endif // ! LL_BUGSPLAT - mDumpPath = logdir; - setMiniDumpDir(logdir); - logdir += gDirUtilp->getDirDelimiter(); - setDebugFileNames(logdir); - // Although initLoggingAndGetLastDuration() is the right place to mess with // setFatalFunction(), we can't query gSavedSettings until after diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h index 79da453cbe..37e8a3917a 100644 --- a/indra/newview/llappviewermacosx-for-objc.h +++ b/indra/newview/llappviewermacosx-for-objc.h @@ -24,6 +24,7 @@ #include +void constructViewer(); bool initViewer(); void handleUrl(const char* url_utf8); bool pumpMainLoop(); diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 77a16f7307..81f04744f8 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -86,7 +86,7 @@ static void exceptionTerminateHandler() gOldTerminateHandler(); // call old terminate() handler } -bool initViewer() +void constructViewer() { // Set the working dir to /Contents/Resources if (chdir(gDirUtilp->getAppRODataDir().c_str()) == -1) @@ -102,18 +102,20 @@ bool initViewer() gOldTerminateHandler = std::set_terminate(exceptionTerminateHandler); gViewerAppPtr->setErrorHandler(LLAppViewer::handleViewerCrash); +} - +bool initViewer() +{ bool ok = gViewerAppPtr->init(); if(!ok) { LL_WARNS() << "Application init failed." << LL_ENDL; } - else if (!gHandleSLURL.empty()) - { - dispatchUrl(gHandleSLURL); - gHandleSLURL = ""; - } + else if (!gHandleSLURL.empty()) + { + dispatchUrl(gHandleSLURL); + gHandleSLURL = ""; + } return ok; } @@ -194,6 +196,7 @@ CrashMetadataSingleton::CrashMetadataSingleton() LL_INFOS() << "Can't parse '" << staticDebugPathname << "'; no metadata about previous run" << LL_ENDL; } + else { LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL; logFilePathname = get_metadata(info, "SLLog"); -- cgit v1.3