# HG changeset patch
# User Alain Mazy <am@osimis.io>
# Date 1666176014 -7200
# Node ID 9d51c000e91a101914cf032580473abeba887b83
# Parent  edefb278cb774eb52e74e33cf789868cb0f8d8ef
more verbose HTTPClient errors + avoid duplicate logging of same error

diff -r edefb278cb77 -r 9d51c000e91a NEWS
--- a/NEWS	Wed Oct 19 08:57:34 2022 +0200
+++ b/NEWS	Wed Oct 19 12:40:14 2022 +0200
@@ -7,6 +7,7 @@
 * C-Store SCU now gives priority to the preferred TransferSyntax proposed by the receiving SCP
   instead of Orthanc own AcceptedTransferSyntaxes.
 * Made the default SQLite DB more robust wrt future updates like adding new columns in DB.
+* Made the HTTP Client errors more verbose by including the url in the logs.
 
 REST API
 --------
diff -r edefb278cb77 -r 9d51c000e91a OrthancFramework/Sources/HttpClient.cpp
--- a/OrthancFramework/Sources/HttpClient.cpp	Wed Oct 19 08:57:34 2022 +0200
+++ b/OrthancFramework/Sources/HttpClient.cpp	Wed Oct 19 12:40:14 2022 +0200
@@ -55,9 +55,6 @@
     }
     else
     {
-      LOG(ERROR) << "Error code " << static_cast<int>(code)
-                 << " in libcurl: " << curl_easy_strerror(code)
-                 << " while accessing url: " << url;
       *status = 0;
       return code;
     }
@@ -102,6 +99,24 @@
     return code;
   }
 
+  static CURLcode CheckCode(CURLcode code, const std::string& url)
+  {
+    if (code == CURLE_NOT_BUILT_IN)
+    {
+      throw OrthancException(ErrorCode_InternalError,
+                             "Your libcurl does not contain a required feature, "
+                             "please recompile Orthanc with -DUSE_SYSTEM_CURL=OFF");
+    }
+
+    if (code != CURLE_OK)
+    {
+      throw OrthancException(ErrorCode_NetworkProtocol,
+                             "libCURL error: " + std::string(curl_easy_strerror(code)) + " while accessing " + url);
+    }
+
+    return code;
+  }
+
 
   // RAII pattern around a "curl_slist"
   class HttpClient::CurlHeaders : public boost::noncopyable
@@ -1064,7 +1079,7 @@
       CLOG(INFO, HTTP) << "cURL status code: " << code;
     }
 
-    CheckCode(code);
+    CheckCode(code, url_);  // throws on HTTP error
 
     if (status == 0)
     {
diff -r edefb278cb77 -r 9d51c000e91a OrthancFramework/Sources/OrthancException.cpp
--- a/OrthancFramework/Sources/OrthancException.cpp	Wed Oct 19 08:57:34 2022 +0200
+++ b/OrthancFramework/Sources/OrthancException.cpp	Wed Oct 19 12:40:14 2022 +0200
@@ -31,7 +31,8 @@
 {
   OrthancException::OrthancException(const OrthancException& other) : 
     errorCode_(other.errorCode_),
-    httpStatus_(other.httpStatus_)
+    httpStatus_(other.httpStatus_),
+    logged_(false)
   {
     if (other.details_.get() != NULL)
     {
@@ -41,7 +42,8 @@
 
   OrthancException::OrthancException(ErrorCode errorCode) : 
     errorCode_(errorCode),
-    httpStatus_(ConvertErrorCodeToHttpStatus(errorCode))
+    httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)),
+    logged_(false)
   {
   }
 
@@ -50,6 +52,7 @@
                                      bool log) :
     errorCode_(errorCode),
     httpStatus_(ConvertErrorCodeToHttpStatus(errorCode)),
+    logged_(log),
     details_(new std::string(details))
   {
 #if ORTHANC_ENABLE_LOGGING == 1
@@ -63,7 +66,8 @@
   OrthancException::OrthancException(ErrorCode errorCode,
                                      HttpStatus httpStatus) :
     errorCode_(errorCode),
-    httpStatus_(httpStatus)
+    httpStatus_(httpStatus),
+    logged_(false)
   {
   }
 
@@ -73,6 +77,7 @@
                                      bool log) :
     errorCode_(errorCode),
     httpStatus_(httpStatus),
+    logged_(log),
     details_(new std::string(details))
   {
 #if ORTHANC_ENABLE_LOGGING == 1
@@ -114,4 +119,10 @@
       return details_->c_str();
     }
   }
+
+  bool OrthancException::HasBeenLogged() const
+  {
+    return logged_;
+  }
+
 }
diff -r edefb278cb77 -r 9d51c000e91a OrthancFramework/Sources/OrthancException.h
--- a/OrthancFramework/Sources/OrthancException.h	Wed Oct 19 08:57:34 2022 +0200
+++ b/OrthancFramework/Sources/OrthancException.h	Wed Oct 19 12:40:14 2022 +0200
@@ -38,6 +38,7 @@
 
     ErrorCode  errorCode_;
     HttpStatus httpStatus_;
+    bool       logged_;    // has the exception already been logged ?  (to avoid double logs)
 
     // New in Orthanc 1.5.0
     std::unique_ptr<std::string>  details_;
@@ -68,5 +69,7 @@
     bool HasDetails() const;
 
     const char* GetDetails() const;
+
+    bool HasBeenLogged() const;
   };
 }
diff -r edefb278cb77 -r 9d51c000e91a OrthancServer/Plugins/Engine/PluginsManager.cpp
--- a/OrthancServer/Plugins/Engine/PluginsManager.cpp	Wed Oct 19 08:57:34 2022 +0200
+++ b/OrthancServer/Plugins/Engine/PluginsManager.cpp	Wed Oct 19 12:40:14 2022 +0200
@@ -183,7 +183,10 @@
         // This service provider has failed
         if (e.GetErrorCode() != ErrorCode_UnknownResource)  // This error code is valid in plugins
         {
-          LOG(ERROR) << "Exception while invoking plugin service " << service << ": " << e.What();
+          if (!e.HasBeenLogged())
+          {
+            LOG(ERROR) << "Exception while invoking plugin service " << service << ": " << e.What();
+          }
         }
 
         return static_cast<OrthancPluginErrorCode>(e.GetErrorCode());