Merge pull request #125 from jhunkeler/retry-download

Add a retry loop to download()

7 files changed, 99 insertions, 49 deletions

diff --git a/README.md b/README.md
index c1a490d..a8c72d6 100644
--- a/README.md
+++ b/README.md
@@ -200,6 +200,9 @@ stasis mydelivery.ini
 | STASIS_JF_CLIENT_CERT_CERT_PATH | Path to OpenSSL cert files                                              | 
 | STASIS_JF_CLIENT_CERT_KEY_PATH  | OpenSSL key file (in cert path)                                         | 
 | STASIS_JF_REPO                  | Artifactory "generic" repository to write to                            | 
+| STASIS_DOWNLOAD_TIMEOUT         | Number of seconds before timing out a remote file download              |
+| STASIS_DOWNLOAD_RETRY_MAX       | Number of retries before giving up on a remote file download            |
+| STASIS_DOWNLOAD_RETRY_SECONDS   | Number of seconds to wait before retrying a remote file download        |
 
 ## Main configuration (stasis.ini)
 
@@ -283,17 +286,17 @@ Environment variables exported are _global_ to all programs executed by stasis.
 
 Sections starting with `test:` will be used during the testing phase of the stasis pipeline. Where the value of `name` following the colon is an arbitrary value, and only used for reporting which test-run is executing. Section names must be unique.
 
-| Key          | Type    | Purpose                                                  | Required |
-|--------------|---------|----------------------------------------------------------|----------|
+| Key          | Type    | Purpose                                                     | Required |
+|--------------|---------|-------------------------------------------------------------|----------|
 | disable      | Boolean | Disable `script` execution (`script_setup` always executes) | N        |
-| parallel     | Boolean | Execute test block in parallel (default) or sequentially | N        |
-| timeout      | String  | Kill test script after `n[hms]`                          | N        |
-| build_recipe | String  | Git repository path to package's conda recipe            | N        |
-| repository   | String  | Git repository path or URL to clone                      | Y        |
-| version      | String  | Git commit or tag to check out                           | Y        |
-| runtime      | List    | Export environment variables specific to test context    | Y        |
-| script_setup | List    | Body of a shell script that will install dependencies    | N        |
-| script       | List    | Body of a shell script that will execute the tests       | Y        |
+| parallel     | Boolean | Execute test block in parallel (default) or sequentially    | N        |
+| timeout      | String  | Kill test script after `n[hms]`                             | N        |
+| build_recipe | String  | Git repository path to package's conda recipe               | N        |
+| repository   | String  | Git repository path or URL to clone                         | Y        |
+| version      | String  | Git commit or tag to check out                              | Y        |
+| runtime      | List    | Export environment variables specific to test context       | Y        |
+| script_setup | List    | Body of a shell script that will install dependencies       | N        |
+| script       | List    | Body of a shell script that will execute the tests          | Y        |
 
 ### deploy:artifactory:_name_
 
diff --git a/src/lib/core/artifactory.c b/src/lib/core/artifactory.c
index eedaf43..d5457e7 100644
--- a/src/lib/core/artifactory.c
+++ b/src/lib/core/artifactory.c
@@ -61,9 +61,10 @@ int artifactory_download_cli(char *dest,
     }
 
     sprintf(path + strlen(path), "/%s", remote_filename);
-    long fetch_status = download(url, path, NULL);
-    if (HTTP_ERROR(fetch_status) || fetch_status < 0) {
-        fprintf(stderr, "%s: download failed: %s\n", __FUNCTION__, url);
+    char *errmsg = NULL;
+    long fetch_status = download(url, path, &errmsg);
+    if (HTTP_ERROR(fetch_status)) {
+        SYSERROR("download failed: %s: %s\n", errmsg, url);
         return -1;
     }
     chmod(path, 0755);
diff --git a/src/lib/core/download.c b/src/lib/core/download.c
index b021860..817e576 100644
--- a/src/lib/core/download.c
+++ b/src/lib/core/download.c
@@ -13,45 +13,85 @@ size_t download_writer(void *fp, size_t size, size_t nmemb, void *stream) {
 long download(char *url, const char *filename, char **errmsg) {
     long http_code = -1;
     char user_agent[20];
-    sprintf(user_agent, "stasis/%s", VERSION);
+    snprintf(user_agent, sizeof(user_agent), "stasis/%s", VERSION);
+
     long timeout = 30L;
-    char *timeout_str = getenv("STASIS_DOWNLOAD_TIMEOUT");
+    const char *timeout_str = getenv("STASIS_DOWNLOAD_TIMEOUT");
+    if (timeout_str) {
+        timeout = strtol(timeout_str, NULL, 10);
+        if (timeout <= 0L) {
+            timeout = 1L;
+        }
+    }
+
+    ssize_t max_retries = 5;
+    const char *max_retries_str = getenv("STASIS_DOWNLOAD_RETRY_MAX");
+    if (max_retries_str) {
+        max_retries = strtol(max_retries_str, NULL, 10);
+        if (max_retries <= 0) {
+            max_retries = 1;
+        }
+    }
+
+    ssize_t max_retry_seconds = 3;
+    const char *max_retry_seconds_str = getenv("STASIS_DOWNLOAD_RETRY_SECONDS");
+    if (max_retry_seconds_str) {
+        max_retry_seconds = strtol(max_retry_seconds_str, NULL, 10);
+        if (max_retry_seconds < 0) {
+            max_retry_seconds = 0;
+        }
+    }
+
 
     curl_global_init(CURL_GLOBAL_ALL);
     CURL *c = curl_easy_init();
-    curl_easy_setopt(c, CURLOPT_URL, url);
-    curl_easy_setopt(c, CURLOPT_WRITEFUNCTION, download_writer);
-    FILE *fp = fopen(filename, "wb");
-    if (!fp) {
-        return -1;
-    }
+    for (ssize_t retry = 0; retry < max_retries; retry++) {
+        if (retry) {
+            fprintf(stderr, "[RETRY %zu/%zu] %s: %s\n", retry + 1, max_retries, *errmsg, url);
+        }
+        curl_easy_setopt(c, CURLOPT_URL, url);
+        curl_easy_setopt(c, CURLOPT_WRITEFUNCTION, download_writer);
+        FILE *fp = fopen(filename, "wb");
+        if (!fp) {
+            return -1;
+        }
 
-    curl_easy_setopt(c, CURLOPT_VERBOSE, 0L);
-    curl_easy_setopt(c, CURLOPT_FOLLOWLOCATION, 1L);
-    curl_easy_setopt(c, CURLOPT_USERAGENT, user_agent);
-    curl_easy_setopt(c, CURLOPT_NOPROGRESS, 0L);
-    curl_easy_setopt(c, CURLOPT_WRITEDATA, fp);
+        curl_easy_setopt(c, CURLOPT_VERBOSE, 0L);
+        curl_easy_setopt(c, CURLOPT_FOLLOWLOCATION, 1L);
+        curl_easy_setopt(c, CURLOPT_USERAGENT, user_agent);
+        curl_easy_setopt(c, CURLOPT_NOPROGRESS, 0L);
+        curl_easy_setopt(c, CURLOPT_WRITEDATA, fp);
+        curl_easy_setopt(c, CURLOPT_CONNECTTIMEOUT, timeout);
 
-    if (timeout_str) {
-        timeout = strtol(timeout_str, NULL, 10);
-    }
-    curl_easy_setopt(c, CURLOPT_CONNECTTIMEOUT, timeout);
-
-    SYSDEBUG("curl_easy_perform(): \n\turl=%s\n\tfilename=%s\n\tuser agent=%s\n\ttimeout=%zu", url, filename, user_agent, timeout);
-    CURLcode curl_code = curl_easy_perform(c);
-    SYSDEBUG("curl status code: %d", curl_code);
-    if (curl_code != CURLE_OK) {
-        if (!*errmsg) {
-            *errmsg = strdup(curl_easy_strerror(curl_code));
-        } else {
-            strncpy(*errmsg, curl_easy_strerror(curl_code), strlen(curl_easy_strerror(curl_code) + 1));
+        SYSDEBUG("curl_easy_perform(): \n\turl=%s\n\tfilename=%s\n\tuser agent=%s\n\ttimeout=%zu", url, filename, user_agent, timeout);
+        CURLcode curl_code = curl_easy_perform(c);
+        SYSDEBUG("curl status code: %d", curl_code);
+
+        if (curl_code != CURLE_OK) {
+            const size_t errmsg_maxlen = 256;
+            if (!*errmsg) {
+                *errmsg = calloc(errmsg_maxlen, sizeof(char));
+            }
+            snprintf(*errmsg, errmsg_maxlen, "%s", curl_easy_strerror(curl_code));
+            curl_easy_reset(c);
+            fclose(fp);
+            sleep(max_retry_seconds);
+            continue;
+        }
+
+        // Data written. Clean up.
+        fclose(fp);
+
+        if (CURLE_OK && *errmsg) {
+            // Retry loop succeeded, no error
+            *errmsg[0] = '\0';
         }
-        goto failed;
+
+        curl_easy_getinfo(c, CURLINFO_RESPONSE_CODE, &http_code);
+        SYSDEBUG("HTTP code: %li", http_code);
+
+        break;
     }
-    curl_easy_getinfo(c, CURLINFO_RESPONSE_CODE, &http_code);
-    failed:
-    SYSDEBUG("HTTP code: %li", http_code);
-    fclose(fp);
     curl_easy_cleanup(c);
     curl_global_cleanup();
     return http_code;
diff --git a/src/lib/core/strlist.c b/src/lib/core/strlist.c
index 5655da9..a0db5f3 100644
--- a/src/lib/core/strlist.c
+++ b/src/lib/core/strlist.c
@@ -92,8 +92,11 @@ int strlist_append_file(struct StrList *pStrList, char *_path, ReaderFn *readerF
         }
         close(fd);
         filename = strdup(tempfile);
-        long http_code = download(path, filename, NULL);
+        char *errmsg = NULL;
+        long http_code = download(path, filename, &errmsg);
         if (HTTP_ERROR(http_code)) {
+            SYSERROR("%s: %s", errmsg, filename);
+            guard_free(errmsg);
             retval = -1;
             goto fatal;
         }
diff --git a/src/lib/delivery/delivery_conda.c b/src/lib/delivery/delivery_conda.c
index 8974ae8..191d93f 100644
--- a/src/lib/delivery/delivery_conda.c
+++ b/src/lib/delivery/delivery_conda.c
@@ -26,9 +26,12 @@ int delivery_get_conda_installer(struct Delivery *ctx, char *installer_url) {
     sprintf(script_path, "%s/%s", ctx->storage.tmpdir, installer);
     if (access(script_path, F_OK)) {
         // Script doesn't exist
-        long fetch_status = download(installer_url, script_path, NULL);
+        char *errmsg = NULL;
+        long fetch_status = download(installer_url, script_path, &errmsg);
         if (HTTP_ERROR(fetch_status) || fetch_status < 0) {
             // download failed
+            SYSERROR("download failed: %s: %s\n", errmsg, installer_url);
+            guard_free(errmsg);
             return -1;
         }
     } else {
diff --git a/tests/test_download.c b/tests/test_download.c
index 714e614..31e9792 100644
--- a/tests/test_download.c
+++ b/tests/test_download.c
@@ -33,7 +33,6 @@ void test_download() {
         }
         STASIS_ASSERT(http_code == tc[i].http_code, "expecting non-error HTTP code");
 
-        //char **data = file_readlines(filename, 0, 100, NULL);
         char *data = stasis_testing_read_ascii(filename);
         if (http_code >= 0) {
             STASIS_ASSERT(data != NULL, "data should not be null");
diff --git a/tests/test_strlist.c b/tests/test_strlist.c
index ce38ff6..47722c0 100644
--- a/tests/test_strlist.c
+++ b/tests/test_strlist.c
@@ -115,6 +115,7 @@ void test_strlist_append_file() {
     const char *local_filename = "test_strlist_append_file.txt";
 
     struct testcase tc[] = {
+            {.origin = "https://this-will-never-work.tld/remote.txt", .expected = (const char *[]){NULL}},
             {.origin = "https://ssb.stsci.edu/jhunk/stasis_test/test_strlist_append_file_from_remote.txt", .expected = expected},
             {.origin = local_filename, .expected = expected},
     };
@@ -141,10 +142,10 @@ void test_strlist_append_file() {
             const char *left;
             const char *right;
             left = strlist_item(list, z);
-            right = expected[z];
+            right = tc[i].expected[z];
             STASIS_ASSERT(strcmp(left, right) == 0, "file content is different than expected");
         }
-        STASIS_ASSERT(strcmp_array((const char **) list->data, expected) == 0, "file contents does not match expected values");
+        STASIS_ASSERT(strcmp_array((const char **) list->data, tc[i].expected) == 0, "file contents does not match expected values");
         guard_strlist_free(&list);
     }
 }