Fixup

cpp/src/defaults.cpp

@@ -168,9 +168,13 @@ defaults::defaults()
   }
   // Determine the default value of `http_max_attempts`
   {
-    const ssize_t env = getenv_or("KVIKIO_HTTP_MAX_ATTEMPTS", 3);
+    const ssize_t env         = getenv_or("KVIKIO_HTTP_MAX_ATTEMPTS", 3);
+    auto upperbound_exclusive = sizeof(int);
     if (env <= 0) {
       throw std::invalid_argument("KVIKIO_HTTP_MAX_ATTEMPTS has to be a positive integer");
+    } else if (env >= upperbound_exclusive) {
+      throw std::invalid_argument("KVIKIO_HTTP_MAX_ATTEMPTS must be less than " +
+                                  std::to_string(upperbound_exclusive));
     }
     _http_max_attempts = env;
   }
@@ -251,7 +255,13 @@ std::size_t defaults::http_max_attempts() { return instance()->_http_max_attempt
 
 void defaults::http_max_attempts_reset(std::size_t attempts)
 {
-  if (attempts == 0) { throw std::invalid_argument("attempts must be a positive integer"); }
+  auto upperbound_exclusive = sizeof(int);
+  if (attempts == 0) {
+    throw std::invalid_argument("attempts must be a positive integer");
+  } else if (attempts >= upperbound_exclusive) {
+    throw std::invalid_argument("attempts must be less than " +
+                                std::to_string(upperbound_exclusive));
+  }
   instance()->_http_max_attempts = attempts;
 }
 

cpp/src/shim/libcurl.cpp

@@ -17,6 +17,7 @@
 #include <chrono>
 #include <cstring>
 #include <functional>
+#include <iostream>
 #include <memory>
 #include <sstream>
 #include <stdexcept>
@@ -125,8 +126,6 @@ void CurlHandle::perform()
   auto& http_status_codes = kvikio::defaults::http_status_codes();
 
   while (attempt_count <= http_max_attempts) {
-    std::stringstream ss;
-
     auto err = curl_easy_perform(handle());
     curl_easy_getinfo(handle(), CURLINFO_RESPONSE_CODE, &http_code);
 
@@ -136,21 +135,24 @@ void CurlHandle::perform()
       // Retry only if one of the specified status codes is returned
       // TODO: Parse the Retry-After header, if it exists.
       // TODO: configurable maximum wait.
-      ss << "HTTP " << http_code << std::endl;
       if (attempt_count == http_max_attempts) {
-        ss << "Max attempts reached." << std::endl;
+        std::stringstream ss;
+        ss << "kvikio http_max_attempts_reached. attempts=" << http_max_attempts
+           << " reason=" << http_code;
         throw std::runtime_error(ss.str());
       } else {
         int backoff_delay = base_delay * (1 << attempt_count);
         int delay         = std::max(1, backoff_delay);
 
         attempt_count++;
-        ss << "Retrying. after=" << delay << " attempt=" << attempt_count
-           << " http_max_attempts=" << http_max_attempts << std::endl;
+        std::cout << "Retrying. reason=" << http_code << " after=" << delay
+                  << " attempt=" << attempt_count << " http_max_attempts=" << http_max_attempts
+                  << std::endl;
         std::this_thread::sleep_for(std::chrono::milliseconds(delay));
       }
     } else if (err != CURLE_OK) {
       std::string msg(_errbuf);  // We can do this because we always initialize `_errbuf` as empty.
+      std::stringstream ss;
       ss << "curl_easy_perform() error near " << _source_file << ":" << _source_line;
       if (msg.empty()) {
         ss << "(" << curl_easy_strerror(err) << ")";

python/kvikio/tests/test_http_io.py

@@ -176,7 +176,7 @@ def test_retry_http_503_ok(tmpdir, xp):
             f.read(b)
 
 
-def test_retry_http_503_fails(tmpdir, xp):
+def test_retry_http_503_fails(tmpdir, xp, capfd):
     with LocalHttpServer(
         tmpdir,
         max_lifetime=60,
@@ -187,10 +187,21 @@ def test_retry_http_503_fails(tmpdir, xp):
         a.tofile(tmpdir / "a")
         b = xp.empty_like(a)
 
-        with pytest.raises(RuntimeError, match="503"):
+        with pytest.raises(RuntimeError) as m:
             with kvikio.RemoteFile.open_http(f"{server.url}/a") as f:
                 f.read(b)
 
+        assert m.match("kvikio http_max_attempts_reached")
+        assert m.match("attempts=3")
+        assert m.match("reason=503")
+        captured = capfd.readouterr()
+
+        records = captured.out.strip().split("\n")
+        assert len(records) == 2
+        assert (
+            records[0] == "Retrying. reason=503 after=200 attempt=2 http_max_attempts=3"
+        )
+
 
 def test_set_http_status_code(tmpdir, xp):
     with LocalHttpServer(