Some error handling improvements in zenhttp

Primarily replaces some exception usage with std::error_code

1 files changed, 79 insertions, 64 deletions

diff --git a/zenhttp/httpsys.cpp b/zenhttp/httpsys.cpp
index 7bb3bbc75..f06460822 100644
--- a/zenhttp/httpsys.cpp
+++ b/zenhttp/httpsys.cpp
@@ -2,6 +2,7 @@
 
 #include "httpsys.h"
 
+#include <zencore/except.h>
 #include <zencore/logging.h>
 #include <zencore/scopeguard.h>
 #include <zencore/string.h>
@@ -62,6 +63,19 @@ class HttpSysServer;
 class HttpSysTransaction;
 class HttpMessageResponseRequest;
 
+//////////////////////////////////////////////////////////////////////////
+
+HttpSysException::HttpSysException(const char* Message, uint32_t Error)
+: HttpServerException(Message, std::error_code(Error, std::system_category()))
+{
+}
+
+HttpSysException::~HttpSysException()
+{
+}
+
+//////////////////////////////////////////////////////////////////////////
+
 using namespace std::literals;
 
 static const uint32_t HashBinary			   = HashStringDjb2("application/octet-stream"sv);
@@ -311,7 +325,7 @@ public:
 	explicit HttpSysRequestHandler(HttpSysTransaction& InRequest) : m_Request(InRequest) {}
 	virtual ~HttpSysRequestHandler() = default;
 
-	virtual void				   IssueRequest()														= 0;
+	virtual void				   IssueRequest(std::error_code& ErrorCode)								= 0;
 	virtual HttpSysRequestHandler* HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTransferred) = 0;
 
 	HttpSysTransaction& Transaction() { return m_Request; }
@@ -333,7 +347,7 @@ struct InitialRequestHandler : public HttpSysRequestHandler
 	InitialRequestHandler(HttpSysTransaction& InRequest);
 	~InitialRequestHandler();
 
-	virtual void				   IssueRequest() override final;
+	virtual void				   IssueRequest(std::error_code& ErrorCode) override final;
 	virtual HttpSysRequestHandler* HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTransferred) override;
 
 	bool	 m_IsInitialRequest		= true;
@@ -344,18 +358,6 @@ struct InitialRequestHandler : public HttpSysRequestHandler
 };
 
 /**
- * @brief Handler used to read complete body
- */
-class HttpPayloadReadRequest : public HttpSysRequestHandler
-{
-public:
-	HttpPayloadReadRequest(HttpSysTransaction& InRequest) : HttpSysRequestHandler(InRequest) {}
-
-	virtual void				   IssueRequest() override;
-	virtual HttpSysRequestHandler* HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTransferred) override;
-};
-
-/**
  * This is the class which request handlers use to interact with the server instance
  */
 
@@ -401,7 +403,7 @@ public:
 											   ULONG_PTR			 NumberOfBytesTransferred,
 											   PTP_IO				 Io);
 
-	void				  IssueInitialRequest();
+	void				  IssueInitialRequest(std::error_code& ErrorCode);
 	PTP_IO				  Iocp();
 	HANDLE				  RequestQueueHandle();
 	inline OVERLAPPED*	  Overlapped() { return &m_HttpOverlapped; }
@@ -411,30 +413,16 @@ public:
 private:
 	OVERLAPPED			   m_HttpOverlapped{};
 	HttpSysServer&		   m_HttpServer;
-	HttpSysRequestHandler* m_HttpHandler{nullptr};	// Tracks which handler is due to handle the next I/O completion event
+	HttpSysRequestHandler* m_CompletionHandler{nullptr};  // Tracks which handler is due to handle the next I/O completion event
 	RwLock				   m_CompletionMutex;
 	InitialRequestHandler  m_InitialHttpHandler{*this};
 };
 
 //////////////////////////////////////////////////////////////////////////
 
-void
-HttpPayloadReadRequest::IssueRequest()
-{
-}
-
-HttpSysRequestHandler*
-HttpPayloadReadRequest::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTransferred)
-{
-	ZEN_UNUSED(IoResult, NumberOfBytesTransferred);
-	return nullptr;
-}
-
-//////////////////////////////////////////////////////////////////////////
-
 /**
  * @brief HTTP request response I/O request handler
- * 
+ *
  * Asynchronously streams out a response to an HTTP request via compound
  * responses from memory or directly from file
  */
@@ -455,7 +443,7 @@ public:
 							   std::span<IoBuffer> Blobs);
 	~HttpMessageResponseRequest();
 
-	virtual void				   IssueRequest() override;
+	virtual void				   IssueRequest(std::error_code& ErrorCode) override final;
 	virtual HttpSysRequestHandler* HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTransferred) override;
 
 	void SuppressResponseBody();
@@ -464,7 +452,7 @@ private:
 	std::vector<HTTP_DATA_CHUNK> m_HttpDataChunks;
 	uint64_t					 m_TotalDataSize = 0;  // Sum of all chunk sizes
 
-	uint16_t m_ResponseCode	   = 0;
+	uint16_t m_ResponseCode		   = 0;
 	uint32_t m_NextDataChunkOffset = 0;	 // This is used for responses where the number of chunks exceed the maximum number for one API call
 	uint32_t m_RemainingChunkCount = 0;
 	bool	 m_IsInitialResponse   = true;
@@ -616,7 +604,7 @@ HttpMessageResponseRequest::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfB
 }
 
 void
-HttpMessageResponseRequest::IssueRequest()
+HttpMessageResponseRequest::IssueRequest(std::error_code& ErrorCode)
 {
 	HttpSysTransaction& Tx		= Transaction();
 	HTTP_REQUEST* const HttpReq = Tx.HttpRequest();
@@ -722,9 +710,9 @@ HttpMessageResponseRequest::IssueRequest()
 
 		CancelThreadpoolIo(Iocp);
 
-		spdlog::error("failed to send HTTP response (error: {}) URL: {}", SendResult, HttpReq->pRawUrl);
+		spdlog::error("failed to send HTTP response (error: {}) URL: {}"sv, SendResult, HttpReq->pRawUrl);
 
-		throw HttpServerException("Failed to send HTTP response", SendResult);
+		ErrorCode = MakeWin32ErrorCode(SendResult);
 	}
 }
 
@@ -763,13 +751,13 @@ HttpSysServer::~HttpSysServer()
 void
 HttpSysServer::Initialize(const wchar_t* UrlPath)
 {
+	m_IsOk = false;
+
 	ULONG Result = HttpCreateServerSession(HTTPAPI_VERSION_2, &m_HttpSessionId, 0);
 
 	if (Result != NO_ERROR)
 	{
-		spdlog::error("Failed to create server session for '{}': {x}", WideToUtf8(UrlPath), Result);
-
-		m_IsOk = false;
+		spdlog::error("Failed to create server session for '{}': {x}"sv, WideToUtf8(UrlPath), Result);
 
 		return;
 	}
@@ -778,7 +766,7 @@ HttpSysServer::Initialize(const wchar_t* UrlPath)
 
 	if (Result != NO_ERROR)
 	{
-		spdlog::error("Failed to create URL group for '{}': {x}", WideToUtf8(UrlPath), Result);
+		spdlog::error("Failed to create URL group for '{}': {x}"sv, WideToUtf8(UrlPath), Result);
 
 		return;
 	}
@@ -789,7 +777,7 @@ HttpSysServer::Initialize(const wchar_t* UrlPath)
 
 	if (Result != NO_ERROR)
 	{
-		spdlog::error("Failed to add base URL to URL group for '{}': {x}", WideToUtf8(UrlPath), Result);
+		spdlog::error("Failed to add base URL to URL group for '{}': {x}"sv, WideToUtf8(UrlPath), Result);
 
 		return;
 	}
@@ -804,7 +792,7 @@ HttpSysServer::Initialize(const wchar_t* UrlPath)
 
 	if (Result != NO_ERROR)
 	{
-		spdlog::error("Failed to create request queue for '{}': {x}", WideToUtf8(UrlPath), Result);
+		spdlog::error("Failed to create request queue for '{}': {x}"sv, WideToUtf8(UrlPath), Result);
 
 		return;
 	}
@@ -816,18 +804,23 @@ HttpSysServer::Initialize(const wchar_t* UrlPath)
 
 	if (Result != NO_ERROR)
 	{
-		spdlog::error("Failed to set server binding property for '{}': {x}", WideToUtf8(UrlPath), Result);
+		spdlog::error("Failed to set server binding property for '{}': {x}"sv, WideToUtf8(UrlPath), Result);
 
 		return;
 	}
 
 	// Create I/O completion port
 
-	m_ThreadPool.CreateIocp(m_RequestQueueHandle, HttpSysTransaction::IoCompletionCallback, /* Context */ this);
+	std::error_code ErrorCode;
+	m_ThreadPool.CreateIocp(m_RequestQueueHandle, HttpSysTransaction::IoCompletionCallback, /* Context */ this, /* out */ ErrorCode);
 
-	if (!m_ThreadPool.Iocp())
+	if (ErrorCode)
+	{
+		spdlog::error("Failed to create IOCP for '{}': {}"sv, WideToUtf8(UrlPath), ErrorCode.message());
+	}
+	else
 	{
-		spdlog::error("Failed to create IOCP for '{}': {x}", WideToUtf8(UrlPath), Result);
+		m_IsOk = true;
 	}
 }
 
@@ -913,7 +906,7 @@ HttpSysServer::OnHandlingRequest()
 void
 HttpSysServer::IssueNewRequestMaybe()
 {
-	if (m_IsShuttingDown)
+	if (m_IsShuttingDown.load(std::memory_order::acquire))
 	{
 		return;
 	}
@@ -925,7 +918,15 @@ HttpSysServer::IssueNewRequestMaybe()
 
 	std::unique_ptr<HttpSysTransaction> Request = std::make_unique<HttpSysTransaction>(*this);
 
-	Request->IssueInitialRequest();
+	std::error_code ec;
+	Request->IssueInitialRequest(ec);
+
+	if (ec)
+	{
+		// No request was actually issued. What is the appropriate response?
+
+		return;
+	}
 
 	// This may end up exceeding the MaxPendingRequests limit, but it's not
 	// really a problem. I'm doing it this way mostly to avoid dealing with
@@ -954,7 +955,7 @@ HttpSysServer::RegisterService(const char* UrlPath, HttpService& Service)
 
 	if (Result != NO_ERROR)
 	{
-		spdlog::error("HttpAddUrlToUrlGroup failed with result {}", Result);
+		spdlog::error("HttpAddUrlToUrlGroup failed with result {}"sv, Result);
 
 		return;
 	}
@@ -980,13 +981,13 @@ HttpSysServer::UnregisterService(const char* UrlPath, HttpService& Service)
 
 	if (Result != NO_ERROR)
 	{
-		spdlog::error("HttpRemoveUrlFromUrlGroup failed with result {}", Result);
+		spdlog::error("HttpRemoveUrlFromUrlGroup failed with result {}"sv, Result);
 	}
 }
 
 //////////////////////////////////////////////////////////////////////////
 
-HttpSysTransaction::HttpSysTransaction(HttpSysServer& Server) : m_HttpServer(Server), m_HttpHandler(&m_InitialHttpHandler)
+HttpSysTransaction::HttpSysTransaction(HttpSysServer& Server) : m_HttpServer(Server), m_CompletionHandler(&m_InitialHttpHandler)
 {
 }
 
@@ -1007,9 +1008,9 @@ HttpSysTransaction::RequestQueueHandle()
 }
 
 void
-HttpSysTransaction::IssueInitialRequest()
+HttpSysTransaction::IssueInitialRequest(std::error_code& ErrorCode)
 {
-	m_InitialHttpHandler.IssueRequest();
+	m_InitialHttpHandler.IssueRequest(ErrorCode);
 }
 
 void
@@ -1045,9 +1046,9 @@ HttpSysTransaction::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTran
 
 	RwLock::ExclusiveLockScope _(m_CompletionMutex);
 
-	bool RequestPending = false;
+	bool IsRequestPending = false;
 
-	if (HttpSysRequestHandler* CurrentHandler = m_HttpHandler)
+	if (HttpSysRequestHandler* CurrentHandler = m_CompletionHandler)
 	{
 		const bool IsInitialRequest = (CurrentHandler == &m_InitialHttpHandler) && m_InitialHttpHandler.IsInitialRequest();
 
@@ -1057,19 +1058,27 @@ HttpSysTransaction::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTran
 			m_HttpServer.OnHandlingRequest();
 		}
 
-		m_HttpHandler = CurrentHandler->HandleCompletion(IoResult, NumberOfBytesTransferred);
+		m_CompletionHandler = CurrentHandler->HandleCompletion(IoResult, NumberOfBytesTransferred);
 
-		if (m_HttpHandler)
+		if (m_CompletionHandler)
 		{
 			try
 			{
-				m_HttpHandler->IssueRequest();
+				std::error_code ec;
+				m_CompletionHandler->IssueRequest(ec);
 
-				RequestPending = true;
+				if (ec)
+				{
+					spdlog::error("IssueRequest() failed {}"sv, ec.message());
+				}
+				else
+				{
+					IsRequestPending = true;
+				}
 			}
 			catch (std::exception& Ex)
 			{
-				spdlog::error("exception caught from IssueRequest(): {}", Ex.what());
+				spdlog::error("exception caught from IssueRequest(): {}"sv, Ex.what());
 
 				// something went wrong, no request is pending
 			}
@@ -1083,14 +1092,16 @@ HttpSysTransaction::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTran
 		}
 	}
 
-	// Ensure new requests are enqueued
+	// Ensure new requests are enqueued as necessary
 	m_HttpServer.IssueNewRequestMaybe();
 
-	if (RequestPending)
+	if (IsRequestPending)
 	{
+		// There is another request pending on this transaction, so it needs to remain valid
 		return Status::kRequestPending;
 	}
 
+	// Transaction done, caller should clean up (delete) this instance
 	return Status::kDone;
 }
 
@@ -1199,7 +1210,7 @@ InitialRequestHandler::~InitialRequestHandler()
 }
 
 void
-InitialRequestHandler::IssueRequest()
+InitialRequestHandler::IssueRequest(std::error_code& ErrorCode)
 {
 	HttpSysTransaction& Tx		= Transaction();
 	PTP_IO				Iocp	= Tx.Iocp();
@@ -1248,10 +1259,14 @@ InitialRequestHandler::IssueRequest()
 
 		// CleanupHttpIoRequest(pIoRequest);
 
-		spdlog::error("HttpReceiveHttpRequest failed, error {:x}", HttpApiResult);
+		ErrorCode = MakeWin32ErrorCode(HttpApiResult);
+
+		spdlog::error("HttpReceiveHttpRequest failed, error {}", ErrorCode.message());
 
 		return;
 	}
+
+	ErrorCode = std::error_code();
 }
 
 HttpSysRequestHandler*