fix error message reporting?

8 files changed, 51 insertions, 56 deletions

diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 77d7fe1b24..6fa9e590cb 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -719,7 +719,7 @@ namespace LLError
 		commonInit(user_dir, app_dir, log_to_stderr);
 	}
 
-	void overrideCrashOnError(const FatalFunction& fatal_function)
+	void setFatalHandler(const FatalFunction& fatal_function)
 	{
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
 		s->mCrashFunction = fatal_function;
@@ -1306,12 +1306,12 @@ namespace LLError
 		return ;
 	}
 
-	bool Log::flush(std::ostringstream* out, const CallSite& site)
+	ErrCrashHandlerResult Log::flush(std::ostringstream* out, const CallSite& site)
 	{
 		LLMutexTrylock lock(&gLogMutex,5);
 		if (!lock.isLocked())
 		{
-			return false; // because this wasn't logged, it cannot be fatal
+			return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal
 		}
 
 		// If we hit a logging request very late during shutdown processing,
@@ -1319,7 +1319,7 @@ namespace LLError
 		// DO NOT resurrect them.
 		if (Settings::wasDeleted() || Globals::wasDeleted())
 		{
-            return false; // because this wasn't logged, it cannot be fatal
+            return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal
 		}
 
 		Globals* g = Globals::getInstance();
@@ -1353,7 +1353,7 @@ namespace LLError
 				} 
 				else
 				{
-					return false; // because this wasn't logged, it cannot be fatal
+					return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal
 				}
 			}
 			else 
@@ -1369,20 +1369,18 @@ namespace LLError
 
 		if (site.mLevel == LEVEL_ERROR)
 		{
-			g->mFatalMessage = message;
             if (s->mCrashFunction)
             {
-                s->mCrashFunction(message);
-                return false; // because an override is in effect
+                return s->mCrashFunction(message);
             }
             else
             {
-                return true; // calling macro should crash
+                return ERR_CRASH; // calling macro should crash
             }
 		}
         else
         {
-            return false; // not ERROR, so do not crash
+            return ERR_DO_NOT_CRASH; // not ERROR, so do not crash
         }
 	}
 }
diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h
index 07aa5c6f8b..2a73ccb36a 100644
--- a/indra/llcommon/llerror.h
+++ b/indra/llcommon/llerror.h
@@ -194,6 +194,8 @@ namespace LLError
 	
 	struct CallSite;
 	
+    enum ErrCrashHandlerResult { ERR_DO_NOT_CRASH, ERR_CRASH };
+
 	class LL_COMMON_API Log
 	{
 	public:
@@ -203,7 +205,7 @@ namespace LLError
 		static void flush(std::ostringstream* out, char* message);
 
         // returns false iff the calling macro should crash
-		static bool flush(std::ostringstream*, const CallSite&);
+		static ErrCrashHandlerResult flush(std::ostringstream*, const CallSite&);
 
 		static std::string demangle(const char* mangled);
 	};
@@ -386,7 +388,7 @@ volatile extern int* gCauseCrash;
 
 #define LL_ENDL                                          \
 			LLError::End();                              \
-            if (LLError::Log::flush(_out, _site))        \
+            if (LLError::ERR_CRASH == LLError::Log::flush(_out, _site)) \
                 LLERROR_CRASH                            \
         }                                                \
 	} while(0)
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index af46430f74..2fa220ba5a 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -93,16 +93,21 @@ namespace LLError
 		Control functions.
 	*/
 
-	typedef boost::function<void(const std::string&)> FatalFunction;
+    // A FatalFunction is called if set using setFatalHandler; its return controls
+    // whether or not the calling error logging code should crash.
+    // ERR_DO_NOT_CRASH should be used only in test code.
+	typedef boost::function<LLError::ErrCrashHandlerResult(const std::string&)> FatalFunction;
+    
 
 	/// Override the default behavior of crashing on LL_ERRS; this should NEVER be used except in test code
-	LL_COMMON_API void overrideCrashOnError(const FatalFunction&);
+	LL_COMMON_API void setFatalHandler(const FatalFunction&);
 		// The fatal function will be called when an message of LEVEL_ERROR
 		// is logged.  Note: supressing a LEVEL_ERROR message from being logged
 		// (by, for example, setting a class level to LEVEL_NONE), will keep
 		// the that message from causing the fatal funciton to be invoked.
+        // The 
 
-    /// Undo the effect of the overrideCrashOnError above
+    /// Undo the effect of the setFatalHandler above
 	LL_COMMON_API void restoreCrashOnError();
 
 	LL_COMMON_API std::string getFatalMessage();
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index 2f8923d2de..9dcb5c145f 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -70,7 +70,11 @@ namespace
 namespace
 {
 	static bool fatalWasCalled;
-	void fatalCall(const std::string&) { fatalWasCalled = true; }
+    LLError::ErrCrashHandlerResult fatalCall(const std::string&)
+    {
+        fatalWasCalled = true;
+        return LLError::ERR_DO_NOT_CRASH;
+    }
 }
 
 namespace tut
@@ -120,7 +124,7 @@ namespace tut
 
 			mPriorErrorSettings = LLError::saveAndResetSettings();
 			LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
-			LLError::overrideCrashOnError(fatalCall);
+			LLError::setFatalHandler(fatalCall);
 			LLError::addRecorder(mRecorder);
 		}
 
diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h
index fedc17dbe9..a412e12a04 100644
--- a/indra/llcommon/tests/wrapllerrs.h
+++ b/indra/llcommon/tests/wrapllerrs.h
@@ -46,7 +46,7 @@
 
 // statically reference the function in test.cpp... it's short, we could
 // replicate, but better to reuse
-extern void wouldHaveCrashed(const std::string& message);
+extern LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message);
 
 struct WrapLLErrs
 {
@@ -57,7 +57,7 @@ struct WrapLLErrs
     :mPriorErrorSettings(LLError::saveAndResetSettings())
     {
         // Make LL_ERRS call our own operator() method
-        LLError::overrideCrashOnError(boost::bind(&WrapLLErrs::operator(), this, _1));
+        LLError::setFatalHandler(boost::bind(&WrapLLErrs::operator(), this, _1));
     }
 
     ~WrapLLErrs()
@@ -71,7 +71,7 @@ struct WrapLLErrs
         FatalException(const std::string& what): LLException(what) {}
     };
 
-    void operator()(const std::string& message)
+    LLError::ErrCrashHandlerResult operator()(const std::string& message)
     {
         // Save message for later in case consumer wants to sense the result directly
         error = message;
@@ -201,7 +201,7 @@ public:
         mOldSettings(LLError::saveAndResetSettings()),
 		mRecorder(new CaptureLogRecorder())
     {
-        LLError::overrideCrashOnError(wouldHaveCrashed);
+        LLError::setFatalHandler(wouldHaveCrashed);
         LLError::setDefaultLevel(level);
         LLError::addRecorder(mRecorder);
     }
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index f9ee22d20a..8b9f9085b1 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -751,16 +751,7 @@ public:
 	}
 };
 
-namespace {
-// With Xcode 6, _exit() is too magical to use with boost::bind(), so provide
-// this little helper function.
-void fast_exit(int rc)
-{
-	_exit(rc);
-}
-
-
-}
+static S32 sQAModeTermCode = 0; // set from QAModeTermCode to specify exit code for LL_ERRS
 
 bool LLAppViewer::init()
 {
@@ -800,17 +791,8 @@ bool LLAppViewer::init()
 	initMaxHeapSize() ;
 	LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize"));
 
-	// Although initLoggingAndGetLastDuration() is the right place to mess with
-	// overrideCrashOnError(), we can't query gSavedSettings until after
-	// initConfiguration().
-	S32 rc(gSavedSettings.getS32("QAModeTermCode"));
-	if (rc >= 0)
-	{
-		// QAModeTermCode set, terminate with that rc on LL_ERRS. Use
-		// fast_exit() rather than exit() because normal cleanup depends too
-		// much on successful startup!
-		LLError::overrideCrashOnError(boost::bind(fast_exit, rc));
-	}
+    // if a return code is set for error exit, save it here for use in fatalErrorHandler
+    sQAModeTermCode = gSavedSettings.getS32("QAModeTermCode");
 
     mAlloc.setProfilingEnabled(gSavedSettings.getBOOL("MemProfiling"));
 
@@ -2152,8 +2134,7 @@ bool LLAppViewer::initThreads()
 	return true;
 }
 
-#ifndef LL_BUGSPLAT
-void errorCallback(const std::string &error_string)
+LLError::ErrCrashHandlerResult fatalErrorHandler(const std::string &error_string)
 {
 #ifndef LL_RELEASE_FOR_DOWNLOAD
 	OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK);
@@ -2167,8 +2148,15 @@ void errorCallback(const std::string &error_string)
 	// haven't actually trashed anything yet, we can afford to write the whole
 	// static info file.
 	LLAppViewer::instance()->writeDebugInfo();
+
+    if (sQAModeTermCode)
+    {
+        _exit(sQAModeTermCode);
+        return LLError::ERR_DO_NOT_CRASH; // notreached
+    }
+
+    return LLError::ERR_CRASH;
 }
-#endif // ! LL_BUGSPLAT
 
 void LLAppViewer::initLoggingAndGetLastDuration()
 {
@@ -2178,6 +2166,7 @@ void LLAppViewer::initLoggingAndGetLastDuration()
 	LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "")
                                 ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "")
                                 );
+	LLError::setFatalHandler(fatalErrorHandler);
 
 	// Remove the last ".old" log file.
 	std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS,
@@ -3728,12 +3717,8 @@ void LLAppViewer::processMarkerFiles()
 		else if (marker_is_same_version)
 		{
 			// the file existed, is ours, and matched our version, so we can report on what it says
-			LL_INFOS("MarkerFile") << "Exec marker '"<< mMarkerFileName << "' found; last exec FROZE" << LL_ENDL;
-#           if LL_BUGSPLAT            
+			LL_INFOS("MarkerFile") << "Exec marker '"<< mMarkerFileName << "' found; last exec crashed" << LL_ENDL;
 			gLastExecEvent = LAST_EXEC_OTHER_CRASH;
-#           else
-			gLastExecEvent = LAST_EXEC_FROZE;
-#           endif
 		}
 		else
 		{
diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp
index 3111540a13..9c2e6eadca 100644
--- a/indra/newview/llappviewermacosx.cpp
+++ b/indra/newview/llappviewermacosx.cpp
@@ -188,17 +188,17 @@ CrashMetadataSingleton::CrashMetadataSingleton()
     LLSD info;
     if (! static_file.is_open())
     {
-        LL_INFOS() << "Can't open '" << staticDebugPathname
+        LL_WARNS() << "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
+        LL_WARNS() << "Can't parse '" << staticDebugPathname
                    << "'; no metadata about previous run" << LL_ENDL;
     }
     else
     {
-        LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL;
+        LL_INFOS() << "Previous run metadata from '" << staticDebugPathname << "':" << LL_ENDL;
         logFilePathname      = get_metadata(info, "SLLog");
         userSettingsPathname = get_metadata(info, "SettingsFilename");
         OSInfo               = get_metadata(info, "OSInfo");
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index 5dabcbbc58..4f966aede8 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -75,9 +75,10 @@
 
 #include <fstream>
 
-void wouldHaveCrashed(const std::string& message)
+LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message)
 {
 	tut::fail("fatal error message: " + message);
+    return LLError::ERR_DO_NOT_CRASH;
 }
 
 namespace tut
@@ -149,7 +150,7 @@ public:
 		mOldSettings(LLError::saveAndResetSettings()),
 		mRecorder(new RecordToTempFile(pool))
 	{
-		LLError::overrideCrashOnError(wouldHaveCrashed);
+		LLError::setFatalHandler(wouldHaveCrashed);
 		LLError::setDefaultLevel(level);
 		LLError::addRecorder(mRecorder);
 	}
@@ -530,7 +531,7 @@ int main(int argc, char **argv)
 		LLError::initForApplication(".", ".", false /* do not log to stderr */);
 		LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
 	}	
-	LLError::overrideCrashOnError(wouldHaveCrashed);
+	LLError::setFatalHandler(wouldHaveCrashed);
 	std::string test_app_name(argv[0]);
 	std::string test_log = test_app_name + ".log";
 	LLFile::remove(test_log);