MXS-2208 Address review comments

Also report errors in more detailed manner.

maxutils/maxbase/include/maxbase/http.hh

@@ -69,6 +69,13 @@ struct Config
 
 struct Result
 {
+    enum
+    {
+        ERROR                = -1, // Some non-specific error occurred.
+        COULDNT_RESOLVE_HOST = -2, // The specified host cold not be resolved.
+        OPERATION_TIMEDOUT   = -3  // The operation timed out.
+    };
+
     int                                code = 0; // HTTP response code
     std::string                        body;     // Response body
     std::map<std::string, std::string> headers;  // Headers attached to the response
@@ -80,6 +87,9 @@ struct Result
  * @param url     The URL to GET.
  * @param config  The config to use.
  *
+ * @note The @c url is assumed to be escaped in case it contain arguments
+ *       that must be escaped.
+ *
  * @return A @c Result.
  */
 Result get(const std::string& url, const Config& config = Config());
@@ -92,6 +102,9 @@ Result get(const std::string& url, const Config& config = Config());
  * @param password  Password for the user.
  * @param config    The config to use.
  *
+ * @note The @c url is assumed to be escaped in case it contain arguments
+ *       that must be escaped, but @c user and @c pass will always be escaped.
+ *
  * @return A @c Result.
  */
 Result get(const std::string& url,
@@ -104,6 +117,9 @@ Result get(const std::string& url,
  * @param urls    The URLs to GET.
  * @param config  The config to use.
  *
+ * @note The @c urls are assumed to be escaped in case they contain arguments
+ *       that must be escaped.
+ *
  * @return A @c Result.
  */
 std::vector<Result> get(const std::vector<std::string>& urls,
@@ -117,6 +133,9 @@ std::vector<Result> get(const std::vector<std::string>& urls,
  * @param password  Password for the user.
  * @param config    The config to use.
  *
+ * @note The @c urls are assumed to be escaped in case they contain arguments
+ *       that must be escaped, but @c user and @c pass will always be escaped.
+ *
  * @return Vector of @c Results, as many as there were @c urls.
  */
 std::vector<Result> get(const std::vector<std::string>& urls,
@@ -259,6 +278,9 @@ const char* to_string(Async::status_t status);
  * @param urls      The URLs to GET.
  * @param config    The config to use.
  *
+ * @note The @c urls are assumed to be escaped in case they contain arguments
+ *       that must be escaped.
+ *
  * @return An Async instance using which the operation can be performed.
  */
 Async get_async(const std::vector<std::string>& urls,
@@ -272,6 +294,9 @@ Async get_async(const std::vector<std::string>& urls,
  * @param password  Password for the user.
  * @param config    The config to use.
  *
+ * @note The @c urls are assumed to be escaped in case they contains arguments
+ *       that must be escaped, but @c user and @c pass will always be escaped.
+ *
  * @return An Async instance using which the operation can be performed.
  */
 Async get_async(const std::vector<std::string>& urls,

maxutils/maxbase/src/http.cc

@@ -44,6 +44,24 @@ static struct THIS_UNIT
 using namespace mxb;
 using namespace mxb::http;
 
+int translate_curl_code(CURLcode code)
+{
+    switch (code)
+    {
+    case CURLE_OK:
+        return 0;
+
+    case CURLE_COULDNT_RESOLVE_HOST:
+        return Result::COULDNT_RESOLVE_HOST;
+
+    case CURLE_OPERATION_TIMEDOUT:
+        return Result::OPERATION_TIMEDOUT;
+
+    default:
+        return Result::ERROR;
+    }
+}
+
 template<class T>
 inline int checked_curl_setopt(CURL* pCurl, CURLoption option, T value)
 {
@@ -108,8 +126,8 @@ CURL* get_easy_curl(const std::string& url,
     if (pCurl)
     {
         checked_curl_setopt(pCurl, CURLOPT_NOSIGNAL, 1);
-        checked_curl_setopt(pCurl, CURLOPT_CONNECTTIMEOUT, config.connect_timeout_s); // For connection phase
+        checked_curl_setopt(pCurl, CURLOPT_CONNECTTIMEOUT, config.connect_timeout_s);// For connection phase
-        checked_curl_setopt(pCurl, CURLOPT_TIMEOUT, config.timeout_s);                // For data transfer phase
+        checked_curl_setopt(pCurl, CURLOPT_TIMEOUT, config.timeout_s);               // For data transfer phase
         checked_curl_setopt(pCurl, CURLOPT_ERRORBUFFER, pErrbuf);
         checked_curl_setopt(pCurl, CURLOPT_WRITEFUNCTION, write_callback);
         checked_curl_setopt(pCurl, CURLOPT_WRITEDATA, &pRes->body);
@@ -296,10 +314,8 @@ public:
             mxb_assert(!true);
             break;
 
-        default:
+        case Async::PENDING:
             {
-                mxb_assert(m_status == Async::PENDING);
-
                 fd_set fdread;
                 fd_set fdwrite;
                 fd_set fdexcep;
@@ -367,12 +383,15 @@ public:
                             Result* pResult = context.pResult;
                             Errbuf* pErrbuf = context.pErrbuf;
 
-                            long code;
+                            if (pMsg->data.result == CURLE_OK)
-                            curl_easy_getinfo(pCurl, CURLINFO_RESPONSE_CODE, &code);
-                            pResult->code = code;
-
-                            if ((code == 0) && pResult->body.empty())
                             {
+                                long code;
+                                curl_easy_getinfo(pCurl, CURLINFO_RESPONSE_CODE, &code);
+                                pResult->code = code;
+                            }
+                            else
+                            {
+                                pResult->code = translate_curl_code(pMsg->data.result);
                                 pResult->body = pErrbuf->data();
                             }
 
@@ -506,15 +525,20 @@ Result get(const std::string& url, const std::string& user, const std::string& p
     CURL* pCurl = get_easy_curl(url, user, password, config, &res, errbuf);
     mxb_assert(pCurl);
 
-    if (curl_easy_perform(pCurl) == CURLE_OK)
+    CURLcode code = curl_easy_perform(pCurl);
+
+    switch (code)
     {
-        long code = 0; // needs to be a long
+    case CURLE_OK:
-        curl_easy_getinfo(pCurl, CURLINFO_RESPONSE_CODE, &code);
+        {
-        res.code = code;
+            long code = 0; // needs to be a long
-    }
+            curl_easy_getinfo(pCurl, CURLINFO_RESPONSE_CODE, &code);
-    else
+            res.code = code;
-    {
+        }
-        res.code = -1;
+        break;
+
+    default:
+        res.code = translate_curl_code(code);
         res.body = errbuf;
     }
 
@@ -534,16 +558,18 @@ vector<Result> get(const std::vector<std::string>& urls,
 {
     Async http = get_async(urls, user, password, config);
 
-    while (http.perform() == Async::PENDING)
+    long timeout_ms = (config.connect_timeout_s + config.timeout_s) * 1000;
+    long max_wait_ms = timeout_ms / 10;
+
+    long wait_ms = 10;
+    while (http.perform(wait_ms) == Async::PENDING)
     {
-        long ms = http.wait_no_more_than();
+        wait_ms = http.wait_no_more_than();
 
-        if (ms > 100)
+        if (wait_ms > max_wait_ms)
         {
-            ms = 100;
+            wait_ms = max_wait_ms;
         }
-
-        std::this_thread::sleep_for(std::chrono::milliseconds(ms));
     }
 
     vector<Result> results(http.results());

maxutils/maxbase/src/test/test_http.cc

@@ -60,7 +60,8 @@ int check_results(const vector<string>& urls,
         auto& res = results[i];
         bool expected_success = expected_successes[i];
 
-        cout << url << " responded with: " << res.code << endl;
+        cout << url << " responded with: " << res.code
+             << (res.code < 0 ? (", " + res.body) : "") << endl;
 
         if (expected_success)
         {