silence warnings in CprHttpClient if request was aborted (#591)

* silence warnings in CprHttpClient if request was aborted
* refactor CprHttpClient

1 files changed, 202 insertions, 197 deletions

diff --git a/src/zenhttp/clients/httpclientcpr.cpp b/src/zenhttp/clients/httpclientcpr.cpp
index 27cffd912..ca4d03dd4 100644
--- a/src/zenhttp/clients/httpclientcpr.cpp
+++ b/src/zenhttp/clients/httpclientcpr.cpp
@@ -105,10 +105,67 @@ AsCprBody(const IoBuffer& Obj)
 	return cpr::Body((const char*)Obj.GetData(), Obj.GetSize());
 }
 
+static bool
+ShouldRetry(const cpr::Response& Response)
+{
+	switch (Response.error.code)
+	{
+		case cpr::ErrorCode::OK:
+			break;
+		case cpr::ErrorCode::INTERNAL_ERROR:
+		case cpr::ErrorCode::NETWORK_RECEIVE_ERROR:
+		case cpr::ErrorCode::NETWORK_SEND_FAILURE:
+		case cpr::ErrorCode::OPERATION_TIMEDOUT:
+			return true;
+		default:
+			return false;
+	}
+	switch ((HttpResponseCode)Response.status_code)
+	{
+		case HttpResponseCode::RequestTimeout:
+		case HttpResponseCode::TooManyRequests:
+		case HttpResponseCode::InternalServerError:
+		case HttpResponseCode::BadGateway:
+		case HttpResponseCode::ServiceUnavailable:
+		case HttpResponseCode::GatewayTimeout:
+			return true;
+		default:
+			return false;
+	}
+};
+
+static std::pair<std::string, std::string>
+HeaderContentType(ZenContentType ContentType)
+{
+	return std::make_pair("Content-Type", std::string(MapContentTypeToString(ContentType)));
+}
+
 //////////////////////////////////////////////////////////////////////////
 
-static HttpClient::Response
-ResponseWithPayload(std::string_view SessionId, cpr::Response& HttpResponse, const HttpResponseCode WorkResponseCode, IoBuffer&& Payload)
+CprHttpClient::CprHttpClient(std::string_view		   BaseUri,
+							 const HttpClientSettings& Connectionsettings,
+							 std::function<bool()>&&   CheckIfAbortFunction)
+: HttpClientBase(BaseUri, Connectionsettings, std::move(CheckIfAbortFunction))
+{
+}
+
+CprHttpClient::~CprHttpClient()
+{
+	ZEN_TRACE_CPU("CprHttpClient::~CprHttpClient");
+	m_SessionLock.WithExclusiveLock([&] {
+		for (auto CprSession : m_Sessions)
+		{
+			delete CprSession;
+		}
+		m_Sessions.clear();
+	});
+}
+
+HttpClient::Response
+CprHttpClient::ResponseWithPayload(std::string_view		  SessionId,
+								   cpr::Response&		  HttpResponse,
+								   const HttpResponseCode WorkResponseCode,
+								   IoBuffer&&			  Payload)
 {
 	// This ends up doing a memcpy, would be good to get rid of it by streaming results
 	// into buffer directly
@@ -121,9 +178,14 @@ ResponseWithPayload(std::string_view SessionId, cpr::Response& HttpResponse, con
 		ResponseBuffer.SetContentType(ContentType);
 	}
 
-	if (!IsHttpSuccessCode(WorkResponseCode) && WorkResponseCode != HttpResponseCode::NotFound)
+	const bool Quiet = m_CheckIfAbortFunction && m_CheckIfAbortFunction();
+
+	if (!Quiet)
 	{
-		ZEN_WARN("HttpClient request failed (session: {}): {}", SessionId, HttpResponse);
+		if (!IsHttpSuccessCode(WorkResponseCode) && WorkResponseCode != HttpResponseCode::NotFound)
+		{
+			ZEN_WARN("HttpClient request failed (session: {}): {}", SessionId, HttpResponse);
+		}
 	}
 
 	return HttpClient::Response{.StatusCode		 = WorkResponseCode,
@@ -134,16 +196,21 @@ ResponseWithPayload(std::string_view SessionId, cpr::Response& HttpResponse, con
 								.ElapsedSeconds	 = HttpResponse.elapsed};
 }
 
-static HttpClient::Response
-CommonResponse(std::string_view SessionId, cpr::Response&& HttpResponse, IoBuffer&& Payload = {})
+HttpClient::Response
+CprHttpClient::CommonResponse(std::string_view SessionId, cpr::Response&& HttpResponse, IoBuffer&& Payload)
 {
 	const HttpResponseCode WorkResponseCode = HttpResponseCode(HttpResponse.status_code);
 	if (HttpResponse.error)
 	{
-		if (HttpResponse.error.code != cpr::ErrorCode::OPERATION_TIMEDOUT &&
-			HttpResponse.error.code != cpr::ErrorCode::CONNECTION_FAILURE && HttpResponse.error.code != cpr::ErrorCode::REQUEST_CANCELLED)
+		const bool Quiet = m_CheckIfAbortFunction && m_CheckIfAbortFunction();
+		if (!Quiet)
 		{
-			ZEN_WARN("HttpClient client failure (session: {}): {}", SessionId, HttpResponse);
+			if (HttpResponse.error.code != cpr::ErrorCode::OPERATION_TIMEDOUT &&
+				HttpResponse.error.code != cpr::ErrorCode::CONNECTION_FAILURE &&
+				HttpResponse.error.code != cpr::ErrorCode::REQUEST_CANCELLED)
+			{
+				ZEN_WARN("HttpClient client failure (session: {}): {}", SessionId, HttpResponse);
+			}
 		}
 
 		// Client side failure code
@@ -176,37 +243,8 @@ CommonResponse(std::string_view SessionId, cpr::Response&& HttpResponse, IoBuffe
 	}
 }
 
-static bool
-ShouldRetry(const cpr::Response& Response)
-{
-	switch (Response.error.code)
-	{
-		case cpr::ErrorCode::OK:
-			break;
-		case cpr::ErrorCode::INTERNAL_ERROR:
-		case cpr::ErrorCode::NETWORK_RECEIVE_ERROR:
-		case cpr::ErrorCode::NETWORK_SEND_FAILURE:
-		case cpr::ErrorCode::OPERATION_TIMEDOUT:
-			return true;
-		default:
-			return false;
-	}
-	switch ((HttpResponseCode)Response.status_code)
-	{
-		case HttpResponseCode::RequestTimeout:
-		case HttpResponseCode::TooManyRequests:
-		case HttpResponseCode::InternalServerError:
-		case HttpResponseCode::BadGateway:
-		case HttpResponseCode::ServiceUnavailable:
-		case HttpResponseCode::GatewayTimeout:
-			return true;
-		default:
-			return false;
-	}
-};
-
-static bool
-ValidatePayload(cpr::Response& Response, std::unique_ptr<detail::TempPayloadFile>& PayloadFile)
+bool
+CprHttpClient::ValidatePayload(cpr::Response& Response, std::unique_ptr<detail::TempPayloadFile>& PayloadFile)
 {
 	ZEN_TRACE_CPU("ValidatePayload");
 	IoBuffer ResponseBuffer = (Response.text.empty() && PayloadFile) ? PayloadFile->BorrowIoBuffer()
@@ -286,19 +324,16 @@ ValidatePayload(cpr::Response& Response, std::unique_ptr<detail::TempPayloadFile
 	return true;
 }
 
-static cpr::Response
-DoWithRetry(
-	std::string_view							 SessionId,
-	std::function<cpr::Response()>&&			 Func,
-	uint8_t										 RetryCount,
-	std::function<bool()>&						 CheckIfAbortFunction,
-	std::function<bool(cpr::Response& Result)>&& Validate = [](cpr::Response&) { return true; })
+cpr::Response
+CprHttpClient::DoWithRetry(std::string_view								SessionId,
+						   std::function<cpr::Response()>&&				Func,
+						   std::function<bool(cpr::Response& Result)>&& Validate)
 {
 	uint8_t		  Attempt = 0;
 	cpr::Response Result  = Func();
-	while (Attempt < RetryCount)
+	while (Attempt < m_ConnectionSettings.RetryCount)
 	{
-		if (CheckIfAbortFunction && CheckIfAbortFunction())
+		if (m_CheckIfAbortFunction && m_CheckIfAbortFunction())
 		{
 			return Result;
 		}
@@ -315,24 +350,29 @@ DoWithRetry(
 		}
 		Sleep(100 * (Attempt + 1));
 		Attempt++;
-		ZEN_INFO("{} Attempt {}/{}", CommonResponse(SessionId, std::move(Result)).ErrorMessage("Retry"), Attempt, RetryCount + 1);
+		const bool Quiet = m_CheckIfAbortFunction && m_CheckIfAbortFunction();
+		if (!Quiet)
+		{
+			ZEN_INFO("{} Attempt {}/{}",
+					 CommonResponse(SessionId, std::move(Result), {}).ErrorMessage("Retry"),
+					 Attempt,
+					 m_ConnectionSettings.RetryCount + 1);
+		}
 		Result = Func();
 	}
 	return Result;
 }
 
-static cpr::Response
-DoWithRetry(std::string_view						  SessionId,
-			std::function<cpr::Response()>&&		  Func,
-			std::unique_ptr<detail::TempPayloadFile>& PayloadFile,
-			uint8_t									  RetryCount,
-			std::function<bool()>&					  CheckIfAbortFunction)
+cpr::Response
+CprHttpClient::DoWithRetry(std::string_view							 SessionId,
+						   std::function<cpr::Response()>&&			 Func,
+						   std::unique_ptr<detail::TempPayloadFile>& PayloadFile)
 {
 	uint8_t		  Attempt = 0;
 	cpr::Response Result  = Func();
-	while (Attempt < RetryCount)
+	while (Attempt < m_ConnectionSettings.RetryCount)
 	{
-		if (CheckIfAbortFunction && CheckIfAbortFunction())
+		if (m_CheckIfAbortFunction && m_CheckIfAbortFunction())
 		{
 			return Result;
 		}
@@ -349,39 +389,19 @@ DoWithRetry(std::string_view						  SessionId,
 		}
 		Sleep(100 * (Attempt + 1));
 		Attempt++;
-		ZEN_INFO("{} Attempt {}/{}", CommonResponse(SessionId, std::move(Result)).ErrorMessage("Retry"), Attempt, RetryCount + 1);
+		const bool Quiet = m_CheckIfAbortFunction && m_CheckIfAbortFunction();
+		if (!Quiet)
+		{
+			ZEN_INFO("{} Attempt {}/{}",
+					 CommonResponse(SessionId, std::move(Result), {}).ErrorMessage("Retry"),
+					 Attempt,
+					 m_ConnectionSettings.RetryCount + 1);
+		}
 		Result = Func();
 	}
 	return Result;
 }
 
-static std::pair<std::string, std::string>
-HeaderContentType(ZenContentType ContentType)
-{
-	return std::make_pair("Content-Type", std::string(MapContentTypeToString(ContentType)));
-}
-
-//////////////////////////////////////////////////////////////////////////
-
-CprHttpClient::CprHttpClient(std::string_view		   BaseUri,
-							 const HttpClientSettings& Connectionsettings,
-							 std::function<bool()>&&   CheckIfAbortFunction)
-: HttpClientBase(BaseUri, Connectionsettings, std::move(CheckIfAbortFunction))
-{
-}
-
-CprHttpClient::~CprHttpClient()
-{
-	ZEN_TRACE_CPU("CprHttpClient::~CprHttpClient");
-	m_SessionLock.WithExclusiveLock([&] {
-		for (auto CprSession : m_Sessions)
-		{
-			delete CprSession;
-		}
-		m_Sessions.clear();
-	});
-}
-
 //////////////////////////////////////////////////////////////////////////
 
 CprHttpClient::Session
@@ -566,16 +586,15 @@ CprHttpClient::Put(std::string_view Url, const IoBuffer& Payload, const KeyValue
 
 	return CommonResponse(
 		m_SessionId,
-		DoWithRetry(
-			m_SessionId,
-			[&]() {
-				Session Sess = AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
-				Sess->SetBody(AsCprBody(Payload));
-				Sess->UpdateHeader({HeaderContentType(Payload.GetContentType())});
-				return Sess.Put();
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+		DoWithRetry(m_SessionId,
+					[&]() {
+						Session Sess =
+							AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
+						Sess->SetBody(AsCprBody(Payload));
+						Sess->UpdateHeader({HeaderContentType(Payload.GetContentType())});
+						return Sess.Put();
+					}),
+		{});
 }
 
 CprHttpClient::Response
@@ -584,20 +603,18 @@ CprHttpClient::Put(std::string_view Url, const KeyValueMap& Parameters)
 	ZEN_TRACE_CPU("CprHttpClient::Put");
 
 	return CommonResponse(m_SessionId,
-						  DoWithRetry(
-							  m_SessionId,
-							  [&]() {
-								  Session Sess = AllocSession(m_BaseUri,
-															  Url,
-															  m_ConnectionSettings,
-															  {{"Content-Length", "0"}},
-															  Parameters,
-															  m_SessionId,
-															  GetAccessToken());
-								  return Sess.Put();
-							  },
-							  m_ConnectionSettings.RetryCount,
-							  m_CheckIfAbortFunction));
+						  DoWithRetry(m_SessionId,
+									  [&]() {
+										  Session Sess = AllocSession(m_BaseUri,
+																	  Url,
+																	  m_ConnectionSettings,
+																	  {{"Content-Length", "0"}},
+																	  Parameters,
+																	  m_SessionId,
+																	  GetAccessToken());
+										  return Sess.Put();
+									  }),
+						  {});
 }
 
 CprHttpClient::Response
@@ -613,12 +630,11 @@ CprHttpClient::Get(std::string_view Url, const KeyValueMap& AdditionalHeader, co
 					AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, Parameters, m_SessionId, GetAccessToken());
 				return Sess.Get();
 			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction,
-			[](cpr::Response& Result) {
+			[this](cpr::Response& Result) {
 				std::unique_ptr<detail::TempPayloadFile> NoTempFile;
 				return ValidatePayload(Result, NoTempFile);
-			}));
+			}),
+		{});
 }
 
 CprHttpClient::Response
@@ -628,14 +644,13 @@ CprHttpClient::Head(std::string_view Url, const KeyValueMap& AdditionalHeader)
 
 	return CommonResponse(
 		m_SessionId,
-		DoWithRetry(
-			m_SessionId,
-			[&]() {
-				Session Sess = AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
-				return Sess.Head();
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+		DoWithRetry(m_SessionId,
+					[&]() {
+						Session Sess =
+							AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
+						return Sess.Head();
+					}),
+		{});
 }
 
 CprHttpClient::Response
@@ -645,14 +660,13 @@ CprHttpClient::Delete(std::string_view Url, const KeyValueMap& AdditionalHeader)
 
 	return CommonResponse(
 		m_SessionId,
-		DoWithRetry(
-			m_SessionId,
-			[&]() {
-				Session Sess = AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
-				return Sess.Delete();
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+		DoWithRetry(m_SessionId,
+					[&]() {
+						Session Sess =
+							AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
+						return Sess.Delete();
+					}),
+		{});
 }
 
 CprHttpClient::Response
@@ -662,15 +676,13 @@ CprHttpClient::Post(std::string_view Url, const KeyValueMap& AdditionalHeader, c
 
 	return CommonResponse(
 		m_SessionId,
-		DoWithRetry(
-			m_SessionId,
-			[&]() {
-				Session Sess =
-					AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, Parameters, m_SessionId, GetAccessToken());
-				return Sess.Post();
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+		DoWithRetry(m_SessionId,
+					[&]() {
+						Session Sess =
+							AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, Parameters, m_SessionId, GetAccessToken());
+						return Sess.Post();
+					}),
+		{});
 }
 
 CprHttpClient::Response
@@ -707,9 +719,8 @@ CprHttpClient::Post(std::string_view Url, const IoBuffer& Payload, ZenContentTyp
 				}
 				Sess->SetBody(AsCprBody(Payload));
 				return Sess.Post();
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+			}),
+		{});
 }
 
 CprHttpClient::Response
@@ -719,17 +730,16 @@ CprHttpClient::Post(std::string_view Url, CbObject Payload, const KeyValueMap& A
 
 	return CommonResponse(
 		m_SessionId,
-		DoWithRetry(
-			m_SessionId,
-			[&]() {
-				Session Sess = AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
-
-				Sess->SetBody(AsCprBody(Payload));
-				Sess->UpdateHeader({HeaderContentType(ZenContentType::kCbObject)});
-				return Sess.Post();
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+		DoWithRetry(m_SessionId,
+					[&]() {
+						Session Sess =
+							AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
+
+						Sess->SetBody(AsCprBody(Payload));
+						Sess->UpdateHeader({HeaderContentType(ZenContentType::kCbObject)});
+						return Sess.Post();
+					}),
+		{});
 }
 
 CprHttpClient::Response
@@ -745,25 +755,24 @@ CprHttpClient::Post(std::string_view Url, const CompositeBuffer& Payload, ZenCon
 
 	return CommonResponse(
 		m_SessionId,
-		DoWithRetry(
-			m_SessionId,
-			[&]() {
-				Session Sess = AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
-				Sess->UpdateHeader({HeaderContentType(ContentType)});
-
-				detail::CompositeBufferReadStream Reader(Payload, 512u * 1024u);
-				auto							  ReadCallback = [this, &Reader](char* buffer, size_t& size, intptr_t) {
-					 if (m_CheckIfAbortFunction && m_CheckIfAbortFunction())
-					 {
-						 return false;
-					 }
-					 size = Reader.Read(buffer, size);
-					 return true;
-				};
-				return Sess.Post(cpr::ReadCallback(gsl::narrow<cpr::cpr_off_t>(Payload.GetSize()), ReadCallback));
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+		DoWithRetry(m_SessionId,
+					[&]() {
+						Session Sess =
+							AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
+						Sess->UpdateHeader({HeaderContentType(ContentType)});
+
+						detail::CompositeBufferReadStream Reader(Payload, 512u * 1024u);
+						auto							  ReadCallback = [this, &Reader](char* buffer, size_t& size, intptr_t) {
+							 if (m_CheckIfAbortFunction && m_CheckIfAbortFunction())
+							 {
+								 return false;
+							 }
+							 size = Reader.Read(buffer, size);
+							 return true;
+						};
+						return Sess.Post(cpr::ReadCallback(gsl::narrow<cpr::cpr_off_t>(Payload.GetSize()), ReadCallback));
+					}),
+		{});
 }
 
 CprHttpClient::Response
@@ -799,9 +808,8 @@ CprHttpClient::Upload(std::string_view Url, const IoBuffer& Payload, const KeyVa
 				}
 				Sess->SetBody(AsCprBody(Payload));
 				return Sess.Put();
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+			}),
+		{});
 }
 
 CprHttpClient::Response
@@ -811,25 +819,24 @@ CprHttpClient::Upload(std::string_view Url, const CompositeBuffer& Payload, ZenC
 
 	return CommonResponse(
 		m_SessionId,
-		DoWithRetry(
-			m_SessionId,
-			[&]() {
-				Session Sess = AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
-				Sess->UpdateHeader({HeaderContentType(ContentType)});
-
-				detail::CompositeBufferReadStream Reader(Payload, 512u * 1024u);
-				auto							  ReadCallback = [this, &Reader](char* buffer, size_t& size, intptr_t) {
-					 if (m_CheckIfAbortFunction && m_CheckIfAbortFunction())
-					 {
-						 return false;
-					 }
-					 size = Reader.Read(buffer, size);
-					 return true;
-				};
-				return Sess.Put(cpr::ReadCallback(gsl::narrow<cpr::cpr_off_t>(Payload.GetSize()), ReadCallback));
-			},
-			m_ConnectionSettings.RetryCount,
-			m_CheckIfAbortFunction));
+		DoWithRetry(m_SessionId,
+					[&]() {
+						Session Sess =
+							AllocSession(m_BaseUri, Url, m_ConnectionSettings, AdditionalHeader, {}, m_SessionId, GetAccessToken());
+						Sess->UpdateHeader({HeaderContentType(ContentType)});
+
+						detail::CompositeBufferReadStream Reader(Payload, 512u * 1024u);
+						auto							  ReadCallback = [this, &Reader](char* buffer, size_t& size, intptr_t) {
+							 if (m_CheckIfAbortFunction && m_CheckIfAbortFunction())
+							 {
+								 return false;
+							 }
+							 size = Reader.Read(buffer, size);
+							 return true;
+						};
+						return Sess.Put(cpr::ReadCallback(gsl::narrow<cpr::cpr_off_t>(Payload.GetSize()), ReadCallback));
+					}),
+		{});
 }
 
 CprHttpClient::Response
@@ -1066,9 +1073,7 @@ CprHttpClient::Download(std::string_view Url, const std::filesystem::path& TempF
 			   }
 			   return Response;
 		   },
-		   PayloadFile,
-		   m_ConnectionSettings.RetryCount,
-		   m_CheckIfAbortFunction);
+		   PayloadFile);
 
 	return CommonResponse(m_SessionId, std::move(Response), PayloadFile ? PayloadFile->DetachToIoBuffer() : IoBuffer{});
 }