Merge pull request #155 from shawkinsl/user/shawkins/improve_curl_errors

User/shawkins/improve curl errors

Author: mrtazz

.github/CONTRIBUTING.md

@@ -10,11 +10,12 @@
 ## How to run tests
 
 Since most of the tests are actually integration tests you will need to have a
-working docker setup to make the full test suite pass.
+working docker setup to make the full test suite pass. gtest build requires python 2 :(
 
 1. build vendorized gtest: `./utils/build_gtest.sh`
-2. build restclient-cpp: `./autogen.sh && ./configure && make check`
-3. run the unit test suite: `make ci`
+1. build restclient-cpp: `./autogen.sh && ./configure && make check`
+1. ensure you have cpplint available `pip install cpplint`
+1. run the unit test suite: `make ci`
 
 ## Help wanted
 Given that I'm not in a position to maintain compatibility with all the different

README.md

@@ -159,7 +159,7 @@ The connection object stores the curl easy handle in an instance variable and
 uses that for the lifetime of the object. This means curl will [automatically
 reuse connections][curl_keepalive] made with that handle.
 
-## Progress callback
+### Progress callback
 
 Two wrapper functions are provided to setup the progress callback for uploads/downloads. 
 
@@ -175,6 +175,15 @@ conn->SetFileProgressCallback(progressFunc);
 conn->SetFileProgressCallbackData(data);
 ```
 
+## Error handling
+When restclient-cpp encounters an error, generally the error (or "status") code is returned in the `Response` (see
+[Response struct in restclient.h](https://github.com/mrtazz/restclient-cpp/blob/master/include/restclient-cpp/restclient.h)). This error code can be either
+an [HTTP error code](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status), or if a lower-level cURL error was encountered, it may be
+a [CURLCode](https://curl.haxx.se/libcurl/c/libcurl-errors.html). Currently, libcurl only defines 92 error codes, which means
+there is no overlap between cURL error codes and HTTP response codes (which start at 1xx). However, if in the future, libcurl defines more than 99
+error codes, meaning that cURL errors overlap with the HTTP 1xx class of responses, restclient-cpp will return a -1 if the CURLCode is 100 or higher.
+In this case, callers can use `GetInfo().lastRequest.curlCode` to inspect the actual cURL error.
+
 ## Thread Safety
 restclient-cpp leans heavily on libcurl as it aims to provide a thin wrapper
 around it. This means it adheres to the basic level of thread safety [provided

include/restclient-cpp/connection.h

@@ -56,6 +56,12 @@ class Connection {
       *  @var RequestInfo::redirectCount
       *  Member 'redirectCount' contains the number of redirects followed. See
       *  CURLINFO_REDIRECT_COUNT
+      *  @var RequestInfo::curlCode
+      *  Member 'curlCode' contains the cURL code (cast to int). See
+      *  libcurl-errors
+      *  @var RequestInfo::curlError
+      *  Member 'curlError' contains the cURL error as a string, if any. See
+      *  CURLOPT_ERRORBUFFER
       */
     typedef struct {
         double totalTime;
@@ -65,7 +71,13 @@ class Connection {
         double preTransferTime;
         double startTransferTime;
         double redirectTime;
-        int redirectCount;
+        // note: libcurl specifies redirectCount is a long, but cpplint
+        // won't let us use long to match. So we must default to the
+        // largest int type available so as to not allow curl to corrupt
+        // the curlCode in that follows in this struct
+        uint64_t redirectCount;
+        int curlCode;
+        std::string curlError;
       } RequestInfo;
     /**
       *  @struct Info
@@ -231,6 +243,7 @@ class Connection {
     std::string keyPassword;
     std::string uriProxy;
     std::string unixSocketPath;
+    char curlErrorBuf[CURL_ERROR_SIZE];
     RestClient::Response performCurlRequest(const std::string& uri);
 };
 };  // namespace RestClient

include/restclient-cpp/restclient.h

@@ -28,9 +28,9 @@ typedef std::map<std::string, std::string> HeaderFields;
 /** @struct Response
   *  @brief This structure represents the HTTP response data
   *  @var Response::code
-  *  Member 'code' contains the HTTP response code
+  *  Member 'code' contains the HTTP response code, or cURL error code
   *  @var Response::body
-  *  Member 'body' contains the HTTP response body
+  *  Member 'body' contains the HTTP response body, or curl_easy_strerror output
   *  @var Response::headers
   *  Member 'headers' contains the HTTP response headers
   */

source/connection.cc

@@ -359,6 +359,10 @@ RestClient::Connection::performCurlRequest(const std::string& uri) {
     curl_easy_setopt(this->curlHandle, CURLOPT_HTTPAUTH, CURLAUTH_BASIC);
     curl_easy_setopt(this->curlHandle, CURLOPT_USERPWD, authString.c_str());
   }
+  /** set error buffer */
+  curl_easy_setopt(this->curlHandle, CURLOPT_ERRORBUFFER,
+                   this->curlErrorBuf);
+
   /** set user agent */
   curl_easy_setopt(this->curlHandle, CURLOPT_USERAGENT,
                    this->GetUserAgent().c_str());
@@ -441,26 +445,22 @@ RestClient::Connection::performCurlRequest(const std::string& uri) {
   }
 
   res = curl_easy_perform(this->curlHandle);
+  this->lastRequest.curlCode = res;
   if (res != CURLE_OK) {
-    switch (res) {
-      case CURLE_OPERATION_TIMEDOUT:
-        ret.code = res;
-        ret.body = "Operation Timeout.";
-        break;
-      case CURLE_SSL_CERTPROBLEM:
-        ret.code = res;
-        ret.body = curl_easy_strerror(res);
-        break;
-      default:
-        ret.body = "Failed to query.";
-        ret.code = -1;
+    int retCode = res;
+    if (retCode > 99) {
+      retCode = -1;
     }
+    ret.code = retCode;
+    ret.body = curl_easy_strerror(res);
   } else {
     int64_t http_code = 0;
     curl_easy_getinfo(this->curlHandle, CURLINFO_RESPONSE_CODE, &http_code);
     ret.code = static_cast<int>(http_code);
   }
 
+  this->lastRequest.curlError = std::string(this->curlErrorBuf);
+
   curl_easy_getinfo(this->curlHandle, CURLINFO_TOTAL_TIME,
                     &this->lastRequest.totalTime);
   curl_easy_getinfo(this->curlHandle, CURLINFO_NAMELOOKUP_TIME,

test/test_connection.cc

@@ -46,8 +46,8 @@ TEST_F(ConnectionTest, TestFailForInvalidCA)
   conn->SetCAInfoFilePath("non-existent file");
   RestClient::Response res = conn->get("/get");
 
-  EXPECT_EQ("Failed to query.", res.body);
-  EXPECT_EQ(-1, res.code);
+  EXPECT_EQ("Problem with the SSL CA cert (path? access rights?)", res.body);
+  EXPECT_EQ(77, res.code);
 }
 
 TEST_F(ConnectionTest, TestDefaultUserAgent)
@@ -104,6 +104,22 @@ TEST_F(ConnectionTest, TestSSLCert)
   EXPECT_EQ(58, res.code);
 }
 
+TEST_F(ConnectionTest, TestCurlError)
+{
+	auto cancelCallback = [](void* pData, double downloadTotal, double downloaded, double uploadTotal, double uploaded) -> int {
+    // abort connection at first progress callback
+    return 1;
+	};
+	conn->SetFileProgressCallback(cancelCallback);
+	conn->SetFileProgressCallbackData(NULL);
+
+  RestClient::Response res = conn->get("/get");
+  int errorCode = conn->GetInfo().lastRequest.curlCode;
+
+  EXPECT_EQ(42, res.code);
+  EXPECT_EQ(42, errorCode);
+}
+
 TEST_F(ConnectionTest, TestSetHeaders)
 {
   RestClient::HeaderFields headers;
@@ -191,7 +207,8 @@ TEST_F(ConnectionTest, TestFollowRedirectLimited)
   EXPECT_EQ(302, res.code);
   conn->FollowRedirects(true, 1);
   res = conn->get("/redirect/2");
-  EXPECT_EQ(-1, res.code);
+  // 47 = CURLE_TOO_MANY_REDIRECTS 
+  EXPECT_EQ(47, res.code);
   conn->FollowRedirects(true, 2);
   res = conn->get("/redirect/2");
   EXPECT_EQ(200, res.code);
@@ -278,6 +295,7 @@ TEST_F(ConnectionTest, TestInvalidProxy)
 {
   conn->SetProxy("127.0.0.1:666");
   RestClient::Response res = conn->get("/get");
-  EXPECT_EQ("Failed to query.", res.body);
-  EXPECT_EQ(-1, res.code);
+  EXPECT_EQ("Couldn't connect to server", res.body);
+  // 7 = CURLE_COULDNT_CONNECT
+  EXPECT_EQ(7, res.code);
 }

test/test_restclient.cc

@@ -51,7 +51,8 @@ TEST_F(RestClientTest, TestRestClientDELETEFailureCode)
 {
   std::string u = "http://nonexistent";
   RestClient::Response res = RestClient::del(u);
-  EXPECT_EQ(-1, res.code);
+  // 6 = CURLE_COULDNT_RESOLVE_HOST 
+  EXPECT_EQ(6, res.code);
 }
 
 TEST_F(RestClientTest, TestRestClientDELETEHeaders)
@@ -89,8 +90,9 @@ TEST_F(RestClientTest, TestRestClientGETFailureCode)
 {
   std::string u = "http://nonexistent";
   RestClient::Response res = RestClient::get(u);
-  EXPECT_EQ("Failed to query.", res.body);
-  EXPECT_EQ(-1, res.code);
+  EXPECT_EQ("Couldn't resolve host name", res.body);
+  // 6 = CURLE_COULDNT_RESOLVE_HOST 
+  EXPECT_EQ(6, res.code);
 }
 
 TEST_F(RestClientTest, TestRestClientGETHeaders)
@@ -123,7 +125,8 @@ TEST_F(RestClientTest, TestRestClientPOSTFailureCode)
 {
   std::string u = "http://nonexistent";
   RestClient::Response res = RestClient::post(u, "text/text", "data");
-  EXPECT_EQ(-1, res.code);
+  // 6 = CURLE_COULDNT_RESOLVE_HOST 
+  EXPECT_EQ(6, res.code);
 }
 
 TEST_F(RestClientTest, TestRestClientPOSTHeaders)
@@ -156,7 +159,8 @@ TEST_F(RestClientTest, TestRestClientPUTFailureCode)
 {
   std::string u = "http://nonexistent";
   RestClient::Response res = RestClient::put(u, "text/text", "data");
-  EXPECT_EQ(-1, res.code);
+  // 6 = CURLE_COULDNT_RESOLVE_HOST 
+  EXPECT_EQ(6, res.code);
 }
 
 TEST_F(RestClientTest, TestRestClientPUTHeaders)
@@ -189,7 +193,8 @@ TEST_F(RestClientTest, TestRestClientPATCHFailureCode)
 {
   std::string u = "http://nonexistent";
   RestClient::Response res = RestClient::patch(u, "text/text", "data");
-  EXPECT_EQ(-1, res.code);
+  // 6 = CURLE_COULDNT_RESOLVE_HOST 
+  EXPECT_EQ(6, res.code);
 }
 
 TEST_F(RestClientTest, TestRestClientPATCHHeaders)
@@ -212,7 +217,8 @@ TEST_F(RestClientTest, TestRestClientOPTIONSFailureCode)
 {
   std::string u = "http://nonexistent";
   RestClient::Response res = RestClient::options(u);
-  EXPECT_EQ(-1, res.code);
+  // 6 = CURLE_COULDNT_RESOLVE_HOST 
+  EXPECT_EQ(6, res.code);
 }
 
 TEST_F(RestClientTest, TestRestClientOPTIONSHeaders)