From 85420b85379d42d03c225db91e0eeee4ac77ffde Mon Sep 17 00:00:00 2001
From: Ryan Hendrickson <ryan.hendrickson@alum.mit.edu>
Date: Sun, 20 Apr 2025 14:49:03 -0400
Subject: [PATCH 1/2] libstore: increase retry delay for 429

A 429 (Too Many Requests) error should not be retried after a quarter of
a second; that's just silly. GitHub recommends a minute.
---
 src/libstore/filetransfer.cc | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc
index a917188d9..5a298a658 100644
--- a/src/libstore/filetransfer.cc
+++ b/src/libstore/filetransfer.cc
@@ -399,6 +399,8 @@ struct curlFileTransfer : public FileTransfer
         {
             auto finishTime = std::chrono::steady_clock::now();
 
+            auto retryTimeMs = request.baseRetryTimeMs;
+
             auto httpStatus = getHTTPStatus();
 
             debug("finished %s of '%s'; curl status = %d, HTTP status = %d, body = %d bytes, duration = %.2f s",
@@ -449,10 +451,12 @@ struct curlFileTransfer : public FileTransfer
                 } else if (httpStatus == 401 || httpStatus == 403 || httpStatus == 407) {
                     // Don't retry on authentication/authorization failures
                     err = Forbidden;
-                } else if (httpStatus >= 400 && httpStatus < 500 && httpStatus != 408 && httpStatus != 429) {
+                } else if (httpStatus == 429) {
+                    // 429 means too many requests, so we retry (with a substantially longer delay)
+                    retryTimeMs = 60000;
+                } else if (httpStatus >= 400 && httpStatus < 500 && httpStatus != 408) {
                     // Most 4xx errors are client errors and are probably not worth retrying:
                     //   * 408 means the server timed out waiting for us, so we try again
-                    //   * 429 means too many requests, so we retry (with a delay)
                     err = Misc;
                 } else if (httpStatus == 501 || httpStatus == 505 || httpStatus == 511) {
                     // Let's treat most 5xx (server) errors as transient, except for a handful:
@@ -518,7 +522,7 @@ struct curlFileTransfer : public FileTransfer
                         || writtenToSink == 0
                         || (acceptRanges && encoding.empty())))
                 {
-                    int ms = request.baseRetryTimeMs * std::pow(2.0f, attempt - 1 + std::uniform_real_distribution<>(0.0, 0.5)(fileTransfer.mt19937));
+                    int ms = retryTimeMs * std::pow(2.0f, attempt - 1 + std::uniform_real_distribution<>(0.0, 0.5)(fileTransfer.mt19937));
                     if (writtenToSink)
                         warn("%s; retrying from offset %d in %d ms", exc.what(), writtenToSink, ms);
                     else

From 047f2bc1af17bc173ebe0e7c7f55e4741bc2b131 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 20 Apr 2025 22:42:12 +0200
Subject: [PATCH 2/2] refactor: Extract RETRY_TIME constants in filetransfer

---
 src/libstore/filetransfer.cc                   | 5 ++++-
 src/libstore/include/nix/store/filetransfer.hh | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc
index 5a298a658..8fc4f14f2 100644
--- a/src/libstore/filetransfer.cc
+++ b/src/libstore/filetransfer.cc
@@ -33,6 +33,9 @@ using namespace std::string_literals;
 
 namespace nix {
 
+const unsigned int RETRY_TIME_MS_DEFAULT = 250;
+const unsigned int RETRY_TIME_MS_TOO_MANY_REQUESTS = 60000;
+
 FileTransferSettings fileTransferSettings;
 
 static GlobalConfig::Register rFileTransferSettings(&fileTransferSettings);
@@ -453,7 +456,7 @@ struct curlFileTransfer : public FileTransfer
                     err = Forbidden;
                 } else if (httpStatus == 429) {
                     // 429 means too many requests, so we retry (with a substantially longer delay)
-                    retryTimeMs = 60000;
+                    retryTimeMs = RETRY_TIME_MS_TOO_MANY_REQUESTS;
                 } else if (httpStatus >= 400 && httpStatus < 500 && httpStatus != 408) {
                     // Most 4xx errors are client errors and are probably not worth retrying:
                     //   * 408 means the server timed out waiting for us, so we try again
diff --git a/src/libstore/include/nix/store/filetransfer.hh b/src/libstore/include/nix/store/filetransfer.hh
index f9b1f620f..10c3ec7ef 100644
--- a/src/libstore/include/nix/store/filetransfer.hh
+++ b/src/libstore/include/nix/store/filetransfer.hh
@@ -58,6 +58,8 @@ struct FileTransferSettings : Config
 
 extern FileTransferSettings fileTransferSettings;
 
+extern const unsigned int RETRY_TIME_MS_DEFAULT;
+
 struct FileTransferRequest
 {
     std::string uri;
@@ -67,7 +69,7 @@ struct FileTransferRequest
     bool head = false;
     bool post = false;
     size_t tries = fileTransferSettings.tries;
-    unsigned int baseRetryTimeMs = 250;
+    unsigned int baseRetryTimeMs = RETRY_TIME_MS_DEFAULT;
     ActivityId parentAct;
     bool decompress = true;
     std::optional<std::string> data;