From 76ab8621887f1499deb067982dd512acdf99538b Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Fri, 15 Mar 2024 18:41:02 -0400
Subject: [PATCH] curl: Also map HTTP errors for retries
When we added the retry logic, the intention here was definitely
to do it not just for network errors but also e.g. HTTP 500s and
the like.
xref https://pagure.io/releng/issue/11439
where we rather painfully debugged that this was missing.
---
src/libostree/ostree-fetcher-curl.c | 18 +++++++++---------
src/libostree/ostree-fetcher-soup.c | 2 +-
src/libostree/ostree-fetcher-soup3.c | 2 +-
src/libostree/ostree-fetcher-util.c | 7 ++++---
src/libostree/ostree-fetcher-util.h | 2 +-
5 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c
index e9b9672b..d6902893 100644
--- a/src/libostree/ostree-fetcher-curl.c
+++ b/src/libostree/ostree-fetcher-curl.c
@@ -322,6 +322,8 @@ check_multi_info (OstreeFetcher *fetcher)
req = g_task_get_task_data (task);
+ gboolean retry_all = (!is_file && req->fetcher->opt_retry_all);
+
if (req->caught_write_error)
g_task_return_error (task, g_steal_pointer (&req->caught_write_error));
else if (curlres != CURLE_OK)
@@ -337,12 +339,9 @@ check_multi_info (OstreeFetcher *fetcher)
/* When it is not a file, we want to retry the request.
* We accomplish that by using G_IO_ERROR_TIMED_OUT.
*/
- gboolean opt_retry_all = req->fetcher->opt_retry_all;
- int g_io_error_code
- = (is_file || !opt_retry_all) ? G_IO_ERROR_FAILED : G_IO_ERROR_TIMED_OUT;
- g_task_return_new_error (task, G_IO_ERROR, g_io_error_code,
- "While fetching %s: [%u] %s", eff_url, curlres,
- curl_easy_strerror (curlres));
+ g_task_return_new_error (
+ task, G_IO_ERROR, retry_all ? G_IO_ERROR_TIMED_OUT : G_IO_ERROR,
+ "While fetching %s: [%u] %s", eff_url, curlres, curl_easy_strerror (curlres));
_ostree_fetcher_journal_failure (req->fetcher->remote_name, eff_url,
curl_easy_strerror (curlres));
}
@@ -362,12 +361,13 @@ check_multi_info (OstreeFetcher *fetcher)
if (!is_file && !(response >= 200 && response < 300) && response != 304)
{
- GIOErrorEnum giocode = _ostree_fetcher_http_status_code_to_io_error (response);
+ GIOErrorEnum giocode
+ = _ostree_fetcher_http_status_code_to_io_error (response, retry_all);
if (req->idx + 1 == req->mirrorlist->len)
{
- g_autofree char *response_msg
- = g_strdup_printf ("Server returned HTTP %lu", response);
+ g_autofree char *response_msg = g_strdup_printf (
+ "While fetching %s: Server returned HTTP %lu", eff_url, response);
g_task_return_new_error (task, G_IO_ERROR, giocode, "%s", response_msg);
if (req->fetcher->remote_name
&& !((req->flags & OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT) > 0
diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c
index e75e72bf..b68c4fd1 100644
--- a/src/libostree/ostree-fetcher-soup.c
+++ b/src/libostree/ostree-fetcher-soup.c
@@ -1094,7 +1094,7 @@ on_request_sent (GObject *object, GAsyncResult *result, gpointer user_data)
#endif
break;
default:
- code = _ostree_fetcher_http_status_code_to_io_error (msg->status_code);
+ code = _ostree_fetcher_http_status_code_to_io_error (msg->status_code, FALSE);
break;
}
diff --git a/src/libostree/ostree-fetcher-soup3.c b/src/libostree/ostree-fetcher-soup3.c
index 6de5c1ed..fa71b363 100644
--- a/src/libostree/ostree-fetcher-soup3.c
+++ b/src/libostree/ostree-fetcher-soup3.c
@@ -621,7 +621,7 @@ on_request_sent (GObject *object, GAsyncResult *result, gpointer user_data)
{
g_autofree char *uristring
= g_uri_to_string (soup_message_get_uri (request->message));
- GIOErrorEnum code = _ostree_fetcher_http_status_code_to_io_error (status);
+ GIOErrorEnum code = _ostree_fetcher_http_status_code_to_io_error (status, FALSE);
{
g_autofree char *errmsg = g_strdup_printf ("Server returned status %u: %s", status,
soup_status_get_phrase (status));
diff --git a/src/libostree/ostree-fetcher-util.c b/src/libostree/ostree-fetcher-util.c
index b797bee7..7ca9f07d 100644
--- a/src/libostree/ostree-fetcher-util.c
+++ b/src/libostree/ostree-fetcher-util.c
@@ -224,7 +224,7 @@ _ostree_fetcher_should_retry_request (const GError *error, guint n_retries_remai
* a #GIOErrorEnum. This will return %G_IO_ERROR_FAILED if the status code is
* unknown or otherwise unhandled. */
GIOErrorEnum
-_ostree_fetcher_http_status_code_to_io_error (guint status_code)
+_ostree_fetcher_http_status_code_to_io_error (guint status_code, gboolean should_retry)
{
switch (status_code)
{
@@ -235,8 +235,9 @@ _ostree_fetcher_http_status_code_to_io_error (guint status_code)
case 408: /* SOUP_STATUS_REQUEST_TIMEOUT */
return G_IO_ERROR_TIMED_OUT;
case 500: /* SOUP_STATUS_INTERNAL_SERVER_ERROR */
- return G_IO_ERROR_BUSY;
+ /* retries are always mapped to timeouts, see similar logic in the curl error handling */
+ return should_retry ? G_IO_ERROR_TIMED_OUT : G_IO_ERROR_BUSY;
default:
- return G_IO_ERROR_FAILED;
+ return should_retry ? G_IO_ERROR_TIMED_OUT : G_IO_ERROR_FAILED;
}
}
diff --git a/src/libostree/ostree-fetcher-util.h b/src/libostree/ostree-fetcher-util.h
index 97233e1f..ef5ad657 100644
--- a/src/libostree/ostree-fetcher-util.h
+++ b/src/libostree/ostree-fetcher-util.h
@@ -57,7 +57,7 @@ void _ostree_fetcher_journal_failure (const char *remote_name, const char *url,
gboolean _ostree_fetcher_should_retry_request (const GError *error, guint n_retries_remaining);
-GIOErrorEnum _ostree_fetcher_http_status_code_to_io_error (guint status_code);
+GIOErrorEnum _ostree_fetcher_http_status_code_to_io_error (guint status_code, gboolean retry_all);
G_END_DECLS
--
2.43.0