Don't half-handle cURL error codes (closes #153)

If cURL returns some error code, just return that to the caller as
ret.code. This works because CURLE codes currently stop at 92 (i.e. they
do not overlap with HTTP 1xx codes).

If a caller notices a code 92 or below, they can inspect ret.body for
some rough analysis of the error. If that is not good enough, they can
also inspect GetInfo().lastRequest.curlError to see the output that curl
put in the curl error buffer (which is for some reason different than
curl_easy_strerror output).

Author: Spencatro

.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

@@ -71,7 +71,11 @@ class Connection {
         double preTransferTime;
         double startTransferTime;
         double redirectTime;
-        long 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;

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

@@ -362,7 +362,7 @@ RestClient::Connection::performCurlRequest(const std::string& uri) {
   /** 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());
@@ -447,19 +447,12 @@ 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);

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)