improve logging on main failure (#111)

* add support for indenting callstack output
* Explicitly catch option-parse error and reduce log spam on bad parameters
* add command line to sentry error reports
* log command line at startup

9 files changed, 41 insertions, 34 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b1a8bba9..e89f1709a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,8 @@
 - Improvement: Make oplog/project removal more robust
 - Improvement: Made LSN number for project store oplog an unsigned 32 bit value at the top layer to increase range
 - Improvement: A request for an out of range project store chunk will now result in a "not found" status result
+- Improvement: Callstack logs on assert/error is now indented for better readability
+- Improvement: Full command line is now logged on startup and added as context information in Sentry reports
 
 ## 5.5.3
 - Feature: New 'workspaces' service which allows a user to share a local folder via zenserver. A workspace can have mulitple workspace shares and they provie an HTTP API that is compatible with the project oplog HTTP API. Workspaces and shares are preserved between runs. Workspaces feature is disabled by default - enable with `--workspaces-enabled` option when launching zenserver.
diff --git a/src/zencore/callstack.cpp b/src/zencore/callstack.cpp
index 905ab3d9e..d16605fb9 100644
--- a/src/zencore/callstack.cpp
+++ b/src/zencore/callstack.cpp
@@ -172,7 +172,7 @@ GetFrameSymbols(uint32_t FrameCount, void** Frames)
 }
 
 void
-FormatCallstack(const CallstackFrames* Callstack, StringBuilderBase& SB)
+FormatCallstack(const CallstackFrames* Callstack, StringBuilderBase& SB, std::string_view Prefix)
 {
 	bool First = true;
 	for (const std::string& Symbol : GetFrameSymbols(Callstack))
@@ -185,15 +185,19 @@ FormatCallstack(const CallstackFrames* Callstack, StringBuilderBase& SB)
 		{
 			First = false;
 		}
+		if (!Prefix.empty())
+		{
+			SB.Append(Prefix);
+		}
 		SB.Append(Symbol);
 	}
 }
 
 std::string
-CallstackToString(const CallstackFrames* Callstack)
+CallstackToString(const CallstackFrames* Callstack, std::string_view Prefix)
 {
 	StringBuilder<2048> SB;
-	FormatCallstack(Callstack, SB);
+	FormatCallstack(Callstack, SB, Prefix);
 	return SB.ToString();
 }
 
diff --git a/src/zencore/include/zencore/callstack.h b/src/zencore/include/zencore/callstack.h
index 02ba8b3c3..ef4ba0e91 100644
--- a/src/zencore/include/zencore/callstack.h
+++ b/src/zencore/include/zencore/callstack.h
@@ -29,8 +29,8 @@ GetFrameSymbols(const CallstackFrames* Callstack)
 	return GetFrameSymbols(Callstack ? Callstack->FrameCount : 0, Callstack ? Callstack->Frames : nullptr);
 }
 
-void		FormatCallstack(const CallstackFrames* Callstack, StringBuilderBase& SB);
-std::string CallstackToString(const CallstackFrames* Callstack);
+void		FormatCallstack(const CallstackFrames* Callstack, StringBuilderBase& SB, std::string_view Prefix);
+std::string CallstackToString(const CallstackFrames* Callstack, std::string_view Prefix = {});
 
 void callstack_forcelink();	 // internal
 
diff --git a/src/zencore/zencore.cpp b/src/zencore/zencore.cpp
index 9c48b355b..6a963110b 100644
--- a/src/zencore/zencore.cpp
+++ b/src/zencore/zencore.cpp
@@ -82,7 +82,7 @@ AssertException::FullDescription() const noexcept
 {
 	if (_Callstack)
 	{
-		return fmt::format("'{}'\n{}", what(), CallstackToString(_Callstack));
+		return fmt::format("'{}'\n{}", what(), CallstackToString(_Callstack, "  "));
 	}
 	return what();
 }
@@ -131,7 +131,7 @@ AssertImpl::OnAssert(const char* Filename, int LineNumber, const char* FunctionN
 
 	fmt::basic_memory_buffer<char, 2048> Message;
 	auto								 Appender = fmt::appender(Message);
-	fmt::format_to(Appender, "{}({}): ZEN_ASSERT({})\n{}", Filename, LineNumber, Msg, CallstackToString(Callstack));
+	fmt::format_to(Appender, "{}({}): ZEN_ASSERT({})\n{}", Filename, LineNumber, Msg, CallstackToString(Callstack, "  "));
 	Message.push_back('\0');
 
 	// We use direct ZEN_LOG here instead of ZEN_ERROR as we don't care about *this* code location in the log
diff --git a/src/zenserver/config.cpp b/src/zenserver/config.cpp
index 55c90cbe1..867089ded 100644
--- a/src/zenserver/config.cpp
+++ b/src/zenserver/config.cpp
@@ -565,6 +565,8 @@ ParseCliOptions(int argc, char* argv[], ZenServerOptions& ServerOptions)
 	}
 #endif
 
+	ServerOptions.CommandLine = fmt::format("{}", fmt::join(std::span<const char*>((const char**)argv, size_t(argc)), " "));
+
 	// Note to those adding future options; std::filesystem::path-type options
 	// must be read into a std::string first. As of cxxopts-3.0.0 it uses a >>
 	// stream operator to convert argv value into the options type. std::fs::path
diff --git a/src/zenserver/config.h b/src/zenserver/config.h
index 41e05c7ea..46fdd6fdf 100644
--- a/src/zenserver/config.h
+++ b/src/zenserver/config.h
@@ -165,6 +165,7 @@ struct ZenServerOptions
 	std::string TraceHost;	// Host name or IP address to send trace data to
 	std::string TraceFile;	// Path of a file to write a trace
 #endif
+	std::string CommandLine;
 };
 
 void ParseCliOptions(int argc, char* argv[], ZenServerOptions& ServerOptions);
diff --git a/src/zenserver/main.cpp b/src/zenserver/main.cpp
index 023abc8a3..c27d4eb04 100644
--- a/src/zenserver/main.cpp
+++ b/src/zenserver/main.cpp
@@ -5,6 +5,7 @@
 #include <zencore/compactbinarybuilder.h>
 #include <zencore/compactbinaryvalidation.h>
 #include <zencore/config.h>
+#include <zencore/except.h>
 #include <zencore/filesystem.h>
 #include <zencore/fmtutils.h>
 #include <zencore/logging.h>
@@ -92,10 +93,9 @@ ZenEntryPoint::Run()
 		std::string SentryDatabasePath	 = (m_ServerOptions.DataDir / ".sentry-native").string();
 		std::string SentryAttachmentPath = m_ServerOptions.AbsLogFile.string();
 
-		Sentry.Initialize(SentryDatabasePath, SentryAttachmentPath, m_ServerOptions.SentryAllowPII);
+		Sentry.Initialize(SentryDatabasePath, SentryAttachmentPath, m_ServerOptions.SentryAllowPII, m_ServerOptions.CommandLine);
 	}
 #endif
-
 	try
 	{
 		// Mutual exclusion and synchronization
@@ -179,6 +179,8 @@ ZenEntryPoint::Run()
 
 		InitializeServerLogging(m_ServerOptions);
 
+		ZEN_INFO("Command line: {}", m_ServerOptions.CommandLine);
+
 #if ZEN_USE_SENTRY
 		Sentry.LogStartupInformation();
 #endif
@@ -437,6 +439,12 @@ main(int argc, char* argv[])
 		return App.Run();
 #endif
 	}
+	catch (const OptionParseException& ParseEx)
+	{
+		// The parsing error already outputs all the details so no need to output the command line here
+		fprintf(stderr, "ERROR: %s\n", ParseEx.what());
+		return 1;
+	}
 	catch (const AssertException& AssertEx)
 	{
 		fprintf(stderr, "ERROR: Caught assert exception in main: '%s'", AssertEx.FullDescription().c_str());
diff --git a/src/zenserver/sentryintegration.cpp b/src/zenserver/sentryintegration.cpp
index 2f68a269b..5d2b851d1 100644
--- a/src/zenserver/sentryintegration.cpp
+++ b/src/zenserver/sentryintegration.cpp
@@ -190,7 +190,10 @@ SentryIntegration::~SentryIntegration()
 }
 
 void
-SentryIntegration::Initialize(std::string SentryDatabasePath, std::string SentryAttachmentPath, bool AllowPII)
+SentryIntegration::Initialize(std::string		 SentryDatabasePath,
+							  std::string		 SentryAttachmentsPath,
+							  bool				 AllowPII,
+							  const std::string& CommandLine)
 {
 	m_AllowPII = AllowPII;
 
@@ -202,11 +205,11 @@ SentryIntegration::Initialize(std::string SentryDatabasePath, std::string Sentry
 	sentry_options_set_dsn(SentryOptions, "https://[email protected]/5919284");
 	sentry_options_set_database_path(SentryOptions, SentryDatabasePath.c_str());
 	sentry_options_set_logger(SentryOptions, SentryLogFunction, this);
-	if (SentryAttachmentPath.starts_with("\\\\?\\"))
+	if (SentryAttachmentsPath.starts_with("\\\\?\\"))
 	{
-		SentryAttachmentPath = SentryAttachmentPath.substr(4);
+		SentryAttachmentsPath = SentryAttachmentsPath.substr(4);
 	}
-	sentry_options_add_attachment(SentryOptions, SentryAttachmentPath.c_str());
+	sentry_options_add_attachment(SentryOptions, SentryAttachmentsPath.c_str());
 	sentry_options_set_release(SentryOptions, ZEN_CFG_VERSION);
 
 	// sentry_options_set_debug(SentryOptions, 1);
@@ -215,6 +218,8 @@ SentryIntegration::Initialize(std::string SentryDatabasePath, std::string Sentry
 
 	if (m_SentryErrorCode == 0)
 	{
+		sentry_value_t SentryUserObject = sentry_value_new_object();
+
 		if (m_AllowPII)
 		{
 #	if ZEN_PLATFORM_WINDOWS
@@ -259,14 +264,15 @@ SentryIntegration::Initialize(std::string SentryDatabasePath, std::string Sentry
 				m_SentryHostName = "unknown";
 			}
 #	endif
-			m_SentryId						= fmt::format("{}@{}", m_SentryUserName, m_SentryHostName);
-			sentry_value_t SentryUserObject = sentry_value_new_object();
+			m_SentryId = fmt::format("{}@{}", m_SentryUserName, m_SentryHostName);
 			sentry_value_set_by_key(SentryUserObject, "id", sentry_value_new_string(m_SentryId.c_str()));
 			sentry_value_set_by_key(SentryUserObject, "username", sentry_value_new_string(m_SentryUserName.c_str()));
 			sentry_value_set_by_key(SentryUserObject, "ip_address", sentry_value_new_string("{{auto}}"));
-			sentry_set_user(SentryUserObject);
 		}
 
+		sentry_value_set_by_key(SentryUserObject, "cmd", sentry_value_new_string(CommandLine.c_str()));
+		sentry_set_user(SentryUserObject);
+
 		m_SentryLogger = spdlog::create<sentry::sentry_sink>("sentry");
 		logging::SetErrorLog("sentry");
 
@@ -279,22 +285,6 @@ SentryIntegration::Initialize(std::string SentryDatabasePath, std::string Sentry
 void
 SentryIntegration::LogStartupInformation()
 {
-#	if (ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC)
-	uid_t		   uid = geteuid();
-	struct passwd* pw  = getpwuid(uid);
-	if (pw)
-	{
-		m_SentryUserName = std::string(pw->pw_name);
-	}
-	ZEN_INFO("Username: '{}'", m_SentryUserName);
-
-	char HostNameBuffer[1023 + 1];
-	int	 err = gethostname(HostNameBuffer, sizeof(HostNameBuffer));
-	if (err == 0)
-	{
-		ZEN_INFO("Hostname: '{}'", HostNameBuffer);
-	}
-#	endif
 	if (m_IsInitialized)
 	{
 		if (m_SentryErrorCode == 0)
diff --git a/src/zenserver/sentryintegration.h b/src/zenserver/sentryintegration.h
index dd8b87ab7..29f526714 100644
--- a/src/zenserver/sentryintegration.h
+++ b/src/zenserver/sentryintegration.h
@@ -36,8 +36,8 @@ public:
 	SentryIntegration();
 	~SentryIntegration();
 
-	void		Initialize(std::string SentryDatabasePath, std::string SentryAttachmentsPath, bool AllowPII);
-	void		LogStartupInformation();
+	void Initialize(std::string SentryDatabasePath, std::string SentryAttachmentsPath, bool AllowPII, const std::string& CommandLine);
+	void LogStartupInformation();
 	static void ClearCaches();
 
 private: