refactor zen command return value handling (#487)

- Improvement: Use consistent language for command line argument parsing errors
- Improvement: Changed zen command parsing errors to output help first and error last to make it easier to spot the error
- Improvement: Refactor zen command return codes to conform to valid Linux range (0-255)

        kSuccess     = 0,
        kOtherError  = 1,
        kBadInput    = 2,

        kOutOfMemory = 16,
        kOutOfDisk   = 17,

        kAssertError = 70,
        
        kHttpOtherClientError = 80,
        kHttpCantConnectError = 81,
        kHttpNotFound         = 66,  // NotFound(404)
        kHttpUnauthorized     = 77,  // Unauthorized(401),
        kHttpSLLError         = 82,
        kHttpForbidden        = 83,  // Forbidden(403)
        kHttpTimeout          = 84,  // RequestTimeout(408)
        kHttpConflict         = 85,  // Conflict(409)
        kHttpNoHost           = 86,
        
        kHttpOtherServerError	   = 90,
        kHttpInternalServerError = 91,  // InternalServerError(500)
        kHttpServiceUnavailable  = 69,  // ServiceUnavailable(503)
        kHttpBadGateway          = 92,  // BadGateway(502)
        kHttpGatewayTimeout      = 93,  // GatewayTimeout(504)

1 files changed, 101 insertions, 159 deletions

diff --git a/src/zen/cmds/admin_cmd.cpp b/src/zen/cmds/admin_cmd.cpp
index dca6da6c4..f0a14691b 100644
--- a/src/zen/cmds/admin_cmd.cpp
+++ b/src/zen/cmds/admin_cmd.cpp
@@ -4,6 +4,7 @@
 
 #include <zencore/basicfile.h>
 #include <zencore/filesystem.h>
+#include <zencore/fmtutils.h>
 #include <zencore/logging.h>
 #include <zenhttp/formatters.h>
 #include <zenhttp/httpclient.h>
@@ -28,21 +29,21 @@ ScrubCommand::ScrubCommand()
 
 ScrubCommand::~ScrubCommand() = default;
 
-int
+void
 ScrubCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	m_HostName = ResolveTargetHostSpec(m_HostName);
 
 	if (m_HostName.empty())
 	{
-		throw OptionParseException("unable to resolve server specification");
+		throw OptionParseException("Unable to resolve server specification", m_Options.help());
 	}
 
 	HttpClient Http(m_HostName);
@@ -52,19 +53,11 @@ ScrubCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 	if (HttpClient::Response Response = Http.Post("/admin/scrub"sv, /* headers */ HttpClient::KeyValueMap{}, Params))
 	{
 		ZEN_CONSOLE("Scrub started OK: {}", Response.ToText());
-
-		return 0;
-	}
-	else if (int StatusCode = (int)Response.StatusCode)
-	{
-		ZEN_CONSOLE_ERROR("Scrub start failed: {}: {} ({})", StatusCode, ReasonStringForHttpResultCode(StatusCode), Response.ToText());
 	}
 	else
 	{
-		ZEN_CONSOLE_ERROR("Scrub start failed: {}", Response.ToText());
+		Response.ThrowError("Scrub start failed");
 	}
-
-	return 1;
 }
 
 //////////////////////////////////////////////////////////////////////////
@@ -160,113 +153,115 @@ GcCommand::~GcCommand()
 {
 }
 
-int
+void
 GcCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	m_HostName = ResolveTargetHostSpec(m_HostName);
 
 	if (m_HostName.empty())
 	{
-		throw OptionParseException("unable to resolve server specification");
+		throw OptionParseException("Unable to resolve server specification", m_Options.help());
 	}
 
-	cpr::Parameters Params;
-	Params.Add({"smallobjects", m_SmallObjects ? "true" : "false"});
+	HttpClient::KeyValueMap Params;
+	Params.Entries.insert({"smallobjects", m_SmallObjects ? "true" : "false"});
 	if (m_MaxCacheDuration != 0)
 	{
-		Params.Add({"maxcacheduration", fmt::format("{}", m_MaxCacheDuration)});
+		Params.Entries.insert({"maxcacheduration", fmt::format("{}", m_MaxCacheDuration)});
 	}
 	if (m_DiskSizeSoftLimit != 0)
 	{
-		Params.Add({"disksizesoftlimit", fmt::format("{}", m_DiskSizeSoftLimit)});
+		Params.Entries.insert({"disksizesoftlimit", fmt::format("{}", m_DiskSizeSoftLimit)});
 	}
-	Params.Add({"skipcid", m_SkipCid ? "true" : "false"});
-	Params.Add({"skipdelete", m_SkipDelete ? "true" : "false"});
+	Params.Entries.insert({"skipcid", m_SkipCid ? "true" : "false"});
+	Params.Entries.insert({"skipdelete", m_SkipDelete ? "true" : "false"});
 	if (m_ForceUseGCV1)
 	{
-		throw OptionParseException("usegcv1 is deprecated and can no longer be used");
+		throw OptionParseException("'--usegcv1' is deprecated and can no longer be used", m_Options.help());
 	}
 	if (m_ForceUseGCV2)
 	{
-		Params.Add({"forceusegcv2", "true"});
+		Params.Entries.insert({"forceusegcv2", "true"});
 	}
 	if (m_CompactBlockThreshold)
 	{
-		Params.Add({"compactblockthreshold", fmt::format("{}", m_CompactBlockThreshold)});
+		Params.Entries.insert({"compactblockthreshold", fmt::format("{}", m_CompactBlockThreshold)});
 	}
 	IoHash LowRef = IoHash::Zero;
 	if (!m_ReferenceHashLow.empty())
 	{
 		if (m_ReferenceHashLow.length() != IoHash::StringLength)
 		{
-			throw OptionParseException(fmt::format("reference-low must be a {} character hex string", IoHash::StringLength));
+			throw OptionParseException(fmt::format("'--reference-low' ('{}') is malformed, it must be a {} character hex string",
+												   m_ReferenceHashLow,
+												   IoHash::StringLength),
+									   m_Options.help());
+		}
+		if (!IoHash::TryParse(m_ReferenceHashLow, LowRef))
+		{
+			throw OptionParseException(fmt::format("'--reference-low' ('{}') is malformed", m_ReferenceHashLow), m_Options.help());
 		}
-		LowRef = IoHash::FromHexString(m_ReferenceHashLow);
 	}
 	IoHash HighRef = IoHash::Max;
 	if (!m_ReferenceHashHigh.empty())
 	{
 		if (m_ReferenceHashHigh.length() != IoHash::StringLength)
 		{
-			throw OptionParseException(fmt::format("reference-high must be a {} character hex string", IoHash::StringLength));
+			throw OptionParseException(fmt::format("''--reference-high' ('{}') is malformed, it must be a {} character hex string",
+												   m_ReferenceHashHigh,
+												   IoHash::StringLength),
+									   m_Options.help());
+		}
+		if (!IoHash::TryParse(m_ReferenceHashHigh, HighRef))
+		{
+			throw OptionParseException(fmt::format("'--reference-high' ('{}') is malformed", m_ReferenceHashHigh), m_Options.help());
 		}
-		HighRef = IoHash::FromHexString(m_ReferenceHashHigh);
 	}
 
 	if (HighRef < LowRef)
 	{
-		throw OptionParseException(fmt::format("invalid reference range, reference-high must be higher value than reference-low"));
+		throw OptionParseException(
+			fmt::format("'--reference-high' ('{}') is invalid, it must be a higher value than '--reference-low' ('{}')",
+						m_ReferenceHashHigh,
+						m_ReferenceHashLow),
+			m_Options.help());
 	}
 
 	if (!m_ReferenceHashLow.empty() || !m_ReferenceHashHigh.empty())
 	{
-		Params.Add({"referencehashlow", LowRef.ToHexString()});
-		Params.Add({"referencehashhigh", HighRef.ToHexString()});
+		Params.Entries.insert({"referencehashlow", LowRef.ToHexString()});
+		Params.Entries.insert({"referencehashhigh", HighRef.ToHexString()});
 	}
 
-	Params.Add({"verbose", m_Verbose ? "true" : "false"});
-	Params.Add({"singlethreaded", m_SingleThreaded ? "true" : "false"});
+	Params.Entries.insert({"verbose", m_Verbose ? "true" : "false"});
+	Params.Entries.insert({"singlethreaded", m_SingleThreaded ? "true" : "false"});
 
 	if (m_StoreCacheAttachmentMetaData)
 	{
-		Params.Add({"storecacheattachmentmetadata", m_StoreCacheAttachmentMetaData ? "true" : "false"});
+		Params.Entries.insert({"storecacheattachmentmetadata", m_StoreCacheAttachmentMetaData ? "true" : "false"});
 	}
 	if (m_StoreProjectAttachmentMetaData)
 	{
-		Params.Add({"storeprojectattachmentmetadata", m_StoreProjectAttachmentMetaData ? "true" : "false"});
-	}
-	Params.Add({"enablevalidation", m_EnableValidation ? "true" : "false"});
-
-	cpr::Session Session;
-	Session.SetHeader(cpr::Header{{"Accept", "application/json"}});
-	Session.SetUrl({fmt::format("{}/admin/gc", m_HostName)});
-	Session.SetParameters(Params);
-
-	cpr::Response Result = Session.Post();
-
-	if (zen::IsHttpSuccessCode(Result.status_code))
-	{
-		ZEN_CONSOLE("OK: {}", Result.text);
-		return 0;
+		Params.Entries.insert({"storeprojectattachmentmetadata", m_StoreProjectAttachmentMetaData ? "true" : "false"});
 	}
+	Params.Entries.insert({"enablevalidation", m_EnableValidation ? "true" : "false"});
 
-	if (Result.status_code)
+	HttpClient Http(m_HostName);
+	if (HttpClient::Response Response = Http.Post("/admin/gc"sv, HttpClient::Accept(HttpContentType::kJSON), Params))
 	{
-		ZEN_CONSOLE_ERROR("GC start failed: {}: {} ({})", Result.status_code, Result.reason, Result.text);
+		ZEN_CONSOLE("OK: {}", Response.ToText());
 	}
 	else
 	{
-		ZEN_CONSOLE_ERROR("GC start failed: {}", Result.error.message);
+		Response.ThrowError("GC start failed");
 	}
-
-	return 1;
 }
 
 GcStatusCommand::GcStatusCommand()
@@ -280,49 +275,32 @@ GcStatusCommand::~GcStatusCommand()
 {
 }
 
-int
+void
 GcStatusCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	m_HostName = ResolveTargetHostSpec(m_HostName);
 
 	if (m_HostName.empty())
 	{
-		throw OptionParseException("unable to resolve server specification");
+		throw OptionParseException("Unable to resolve server specification", m_Options.help());
 	}
 
-	cpr::Session Session;
-	Session.SetHeader(cpr::Header{{"Accept", "application/json"}});
-	Session.SetUrl({fmt::format("{}/admin/gc", m_HostName)});
-	if (m_Details)
-	{
-		Session.SetParameters({{"details", "true"}});
-	}
-
-	cpr::Response Result = Session.Get();
-
-	if (zen::IsHttpSuccessCode(Result.status_code))
-	{
-		ZEN_CONSOLE("OK: {}", Result.text);
-		return 0;
-	}
-
-	if (Result.status_code)
+	HttpClient Http(m_HostName);
+	if (HttpClient::Response Response = Http.Get("/admin/gc"sv, HttpClient::Accept(HttpContentType::kJSON)))
 	{
-		ZEN_CONSOLE_ERROR("GC status failed: {}: {} ({})", Result.status_code, Result.reason, Result.text);
+		ZEN_CONSOLE("OK: {}", Response.ToText());
 	}
 	else
 	{
-		ZEN_CONSOLE_ERROR("GC status failed: {}", Result.error.message);
+		Response.ThrowError("Gc status failed");
 	}
-
-	return 1;
 }
 
 GcStopCommand::GcStopCommand()
@@ -335,48 +313,39 @@ GcStopCommand::~GcStopCommand()
 {
 }
 
-int
+void
 GcStopCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	m_HostName = ResolveTargetHostSpec(m_HostName);
 
 	if (m_HostName.empty())
 	{
-		throw OptionParseException("unable to resolve server specification");
+		throw OptionParseException("Unable to resolve server specification", m_Options.help());
 	}
 
-	cpr::Session Session;
-	Session.SetUrl({fmt::format("{}/admin/gc-stop", m_HostName)});
-	cpr::Response Result = Session.Post();
-
-	if (static_cast<HttpResponseCode>(Result.status_code) == HttpResponseCode::Accepted)
-	{
-		ZEN_CONSOLE("OK: {}", "Cancel request accepted");
-		return 0;
-	}
-	else if (zen::IsHttpSuccessCode(Result.status_code))
-	{
-		ZEN_CONSOLE("OK: {}", "No GC running");
-		return 0;
-	}
-
-	if (Result.status_code)
+	HttpClient Http(m_HostName);
+	if (HttpClient::Response Response = Http.Post("/admin/gc-stop"sv, HttpClient::Accept(HttpContentType::kJSON)))
 	{
-		ZEN_CONSOLE_ERROR("GC status failed: {}: {} ({})", Result.status_code, Result.reason, Result.text);
+		if (Response.StatusCode == HttpResponseCode::Accepted)
+		{
+			ZEN_CONSOLE("OK: {}", "Cancel request accepted");
+		}
+		else
+		{
+			ZEN_CONSOLE("OK: {}", "No GC running");
+		}
 	}
 	else
 	{
-		ZEN_CONSOLE_ERROR("GC status failed: {}", Result.error.message);
+		Response.ThrowError("Gc stop failed");
 	}
-
-	return 1;
 }
 
 ////////////////////////////////////////////
@@ -391,7 +360,7 @@ JobCommand::JobCommand()
 
 JobCommand::~JobCommand() = default;
 
-int
+void
 JobCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
@@ -400,14 +369,14 @@ JobCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	m_HostName = ResolveTargetHostSpec(m_HostName);
 
 	if (m_HostName.empty())
 	{
-		throw OptionParseException("unable to resolve server specification");
+		throw OptionParseException("Unable to resolve server specification", m_Options.help());
 	}
 
 	HttpClient Http(m_HostName);
@@ -416,8 +385,7 @@ JobCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 	{
 		if (m_JobId == 0)
 		{
-			throw OptionParseException("Job id must be given");
-			return 1;
+			throw OptionParseException("'--jobid' is required", m_Options.help());
 		}
 	}
 	std::string Url = m_JobId != 0 ? fmt::format("/admin/jobs/{}", m_JobId) : "/admin/jobs";
@@ -431,7 +399,6 @@ JobCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		else
 		{
 			Result.ThrowError("failed cancelling job"sv);
-			return 1;
 		}
 	}
 	else if (HttpClient::Response Result = Http.Get(Url, HttpClient::Accept(ZenContentType::kJSON)))
@@ -441,10 +408,7 @@ JobCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 	else
 	{
 		Result.ThrowError("failed fetching job info"sv);
-		return 1;
 	}
-
-	return 0;
 }
 
 ////////////////////////////////////////////
@@ -479,7 +443,7 @@ LoggingCommand::LoggingCommand()
 
 LoggingCommand::~LoggingCommand() = default;
 
-int
+void
 LoggingCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
@@ -488,14 +452,14 @@ LoggingCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	m_HostName = ResolveTargetHostSpec(m_HostName);
 
 	if (m_HostName.empty())
 	{
-		throw OptionParseException("unable to resolve server specification");
+		throw OptionParseException("Unable to resolve server specification", m_Options.help());
 	}
 
 	HttpClient Http(m_HostName);
@@ -514,8 +478,8 @@ LoggingCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		}
 		else
 		{
-			ZEN_CONSOLE_ERROR("Invalid value for parameter 'cache-write-log'. Use 'enable' or 'disable'");
-			return 1;
+			throw OptionParseException(fmt::format("'--cache-write-log' ('{}') is invalid, use 'enable' or 'disable'", m_CacheWriteLog),
+									   m_Options.help());
 		}
 	}
 
@@ -531,8 +495,8 @@ LoggingCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		}
 		else
 		{
-			ZEN_CONSOLE_ERROR("Invalid value for parameter 'cache-access-log'. Use 'enable' or 'disable'");
-			return 1;
+			throw OptionParseException(fmt::format("'--cache-access-log' ('{}') is invalid, use 'enable' or 'disable'", m_CacheAccessLog),
+									   m_Options.help());
 		}
 	}
 
@@ -548,49 +512,37 @@ LoggingCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 			ZEN_CONSOLE("{}", Result.ToText());
 			const CbObject LogsResponse = Result.AsObject();
 
-			auto CopyLog = [](std::string_view SourceName, std::string_view SourcePath, std::string_view TargetPath) -> bool {
+			auto CopyLog = [](std::string_view SourceName, std::string_view SourcePath, std::string_view TargetPath) {
 				if (SourcePath.empty())
 				{
-					ZEN_CONSOLE_ERROR("Failed to retrieve {} log path", SourceName);
-					return false;
+					throw std::runtime_error(fmt::format("Failed to retrieve {} log path", SourceName));
 				}
 				if (!CopyFile(SourcePath, TargetPath, {}))
 				{
-					ZEN_CONSOLE_ERROR("Failed to copy {} log file {} to output file '{}'", SourceName, SourcePath, TargetPath);
-					return false;
+					throw std::runtime_error(
+						fmt::format("Failed to copy {} log file {} to output file '{}'", SourceName, SourcePath, TargetPath));
 				}
-				return true;
 			};
 
 			if (!m_ServerLogTarget.empty())
 			{
-				if (!CopyLog("server", LogsResponse["Logfile"].AsString(), m_ServerLogTarget))
-				{
-					return 1;
-				}
+				CopyLog("server", LogsResponse["Logfile"].AsString(), m_ServerLogTarget);
 			}
 
 			if (!m_CacheLogTarget.empty())
 			{
-				if (!CopyLog("cache", LogsResponse["cache"].AsObjectView()["Logfile"].AsString(), m_CacheLogTarget))
-				{
-					return 1;
-				}
+				CopyLog("cache", LogsResponse["cache"].AsObjectView()["Logfile"].AsString(), m_CacheLogTarget);
 			}
 			if (!m_HttpLogTarget.empty())
 			{
-				if (!CopyLog("http", LogsResponse["http"].AsObjectView()["Logfile"].AsString(), m_HttpLogTarget))
-				{
-					return 1;
-				}
+				CopyLog("http", LogsResponse["http"].AsObjectView()["Logfile"].AsString(), m_HttpLogTarget);
 			}
 		}
 		else
 		{
 			Result.ThrowError("failed fetching log info"sv);
-			return 1;
 		}
-		return 0;
+		return;
 	}
 	if (HttpClient::Response Result = Http.Post("/admin/logs", HttpClient::KeyValueMap{}, Parameters))
 	{
@@ -599,10 +551,7 @@ LoggingCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 	else
 	{
 		Result.ThrowError("failed setting log info"sv);
-		return 1;
 	}
-
-	return 0;
 }
 
 //////////////////////////////////////////////////////////////////////////
@@ -615,21 +564,21 @@ FlushCommand::FlushCommand()
 
 FlushCommand::~FlushCommand() = default;
 
-int
+void
 FlushCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	m_HostName = ResolveTargetHostSpec(m_HostName);
 
 	if (m_HostName.empty())
 	{
-		throw OptionParseException("unable to resolve server specification");
+		throw OptionParseException("Unable to resolve server specification", m_Options.help());
 	}
 
 	zen::HttpClient Http(m_HostName);
@@ -638,18 +587,12 @@ FlushCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 	{
 		ZEN_CONSOLE("OK: {}", Response.ToText());
 
-		return 0;
-	}
-	else if (int StatusCode = (int)Response.StatusCode)
-	{
-		ZEN_CONSOLE_ERROR("Flush failed: {}: {} ({})", StatusCode, ReasonStringForHttpResultCode(StatusCode), Response.ToText());
+		return;
 	}
 	else
 	{
-		ZEN_CONSOLE_ERROR("Flush failed: {}", Response.ToText());
+		Response.ThrowError("Flush failed");
 	}
-
-	return 1;
 }
 
 //////////////////////////////////////////////////////////////////////////
@@ -693,29 +636,29 @@ TryCopy(const std::filesystem::path& Source, const std::filesystem::path& Target
 	return CopyFile(Source, Target, Options);
 }
 
-int
+void
 CopyStateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 {
 	ZEN_UNUSED(GlobalOptions);
 
 	if (!ParseOptions(argc, argv))
 	{
-		return 0;
+		return;
 	}
 
 	if (m_DataPath.empty())
 	{
-		throw OptionParseException("data path must be given");
+		throw OptionParseException("'--data-path' is required", m_Options.help());
 	}
 
 	if (!IsDir(m_DataPath))
 	{
-		throw OptionParseException("data path must exist");
+		throw std::runtime_error(fmt::format("'--data-path' '{}' must exist", m_DataPath));
 	}
 
 	if (m_TargetPath.empty())
 	{
-		throw OptionParseException("target path must be given");
+		throw OptionParseException("'--target-path' is required", m_Options.help());
 	}
 
 	std::filesystem::path RootManifestPath		 = m_DataPath / "root_manifest";
@@ -723,7 +666,8 @@ CopyStateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 
 	if (!TryCopy(RootManifestPath, TargetRootManifestPath))
 	{
-		throw OptionParseException("data path is invalid, missing root_manifest");
+		throw std::runtime_error(
+			fmt::format("'--data-path' ('{}') is invalid, missing root_manifest at '{}'", m_DataPath, RootManifestPath));
 	}
 
 	std::filesystem::path CachePath		  = m_DataPath / "cache";
@@ -828,8 +772,6 @@ CopyStateCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 			}
 		}
 	}
-
-	return 0;
 }
 
 }  // namespace zen