By default, don't retry on curl failures (#545)

Since they're not usually resolvable just by waiting, and it's better to give a clear error.

NEWS.md

@@ -1,5 +1,6 @@
 # httr2 (development version)
 
+* `req_retry()` no longer treates low-level HTTP failures the same way as transient errors by default. You can return to the previous behaviour with `retry_on_error = TRUE`.
 * `req_perform_iterative()` is no longer experimental.
 * New `req_cookie_set()` allows you to set client side cookies (#369).
 * `req_body_file()` no longer leaks a connection if the response doesn't complete succesfully (#534).

R/req-perform.R

@@ -125,19 +125,16 @@ req_perform <- function(
     )
     req_completed(req_prep)
 
-    if (is_error(resp)) {
+    if (retry_is_transient(req, resp)) {
       tries <- tries + 1
-      delay <- retry_backoff(req, tries)
+      delay <- retry_after(req, resp, tries)
+      signal(class = "httr2_retry", tries = tries, delay = delay)
     } else if (!reauth && resp_is_invalid_oauth_token(req, resp)) {
       reauth <- TRUE
       req <- auth_oauth_sign(req, TRUE)
       req_prep <- req_prepare(req)
       handle <- req_handle(req_prep)
       delay <- 0
-    } else if (retry_is_transient(req, resp)) {
-      tries <- tries + 1
-      delay <- retry_after(req, resp, tries)
-      signal(class = "httr2_retry", tries = tries, delay = delay)
     } else {
       # done
       break

R/req-retries.R

@@ -4,17 +4,17 @@
 #' `req_retry()` alters [req_perform()] so that it will automatically retry
 #' in the case of failure. To activate it, you must specify either the total
 #' number of requests to make with `max_tries` or the total amount of time
-#' to spend with `max_seconds`. Then `req_perform()` will retry if:
+#' to spend with `max_seconds`. Then `req_perform()` will retry if the error is
+#' "transient", i.e. it's an HTTP error that can be resolved by waiting. By
+#' default, 429 and 503 statuses are treated as transient, but if the API you
+#' are wrapping has other transient status codes (or conveys transient-ness
+#' with some other property of the response), you can override the default
+#' with `is_transient`.
 #'
-#' * Either the HTTP request or HTTP response doesn't complete successfully
-#'   leading to an error from curl, the lower-level library that httr2 uses to
-#'   perform HTTP request. This occurs, for example, if your wifi is down.
-#'
-#' * The error is "transient", i.e. it's an HTTP error that can be resolved
-#'   by waiting. By default, 429 and 503 statuses are treated as transient,
-#'   but if the API you are wrapping has other transient status codes (or
-#'   conveys transient-ness with some other property of the response), you can
-#'   override the default with `is_transient`.
+#' Additionally, if you set `retry_on_failure = TRUE`, the request will retry
+#' if either the HTTP request or HTTP response doesn't complete successfully
+#' leading to an error from curl, the lower-level library that httr2 uses to
+#' perform HTTP request. This occurs, for example, if your wifi is down.
 #'
 #' It's a bad idea to immediately retry a request, so `req_perform()` will
 #' wait a little before trying again:
@@ -42,6 +42,8 @@
 #' @param is_transient A predicate function that takes a single argument
 #'   (the response) and returns `TRUE` or `FALSE` specifying whether or not
 #'   the response represents a transient error.
+#' @param retry_on_failure Treat low-level failures as if they are
+#'   transient errors, and can be retried.
 #' @param backoff A function that takes a single argument (the number of failed
 #'   attempts so far) and returns the number of seconds to wait.
 #' @param after A function that takes a single argument (the response) and
@@ -80,16 +82,19 @@
 req_retry <- function(req,
                       max_tries = NULL,
                       max_seconds = NULL,
+                      retry_on_failure = FALSE,
                       is_transient = NULL,
                       backoff = NULL,
                       after = NULL) {
   check_request(req)
   check_number_whole(max_tries, min = 2, allow_null = TRUE)
   check_number_whole(max_seconds, min = 0, allow_null = TRUE)
+  check_bool(retry_on_failure)
 
   req_policies(req,
     retry_max_tries = max_tries,
     retry_max_wait = max_seconds,
+    retry_on_failure = retry_on_failure,
     retry_is_transient = as_callback(is_transient, 1, "is_transient"),
     retry_backoff = as_callback(backoff, 1, "backoff"),
     retry_after = as_callback(after, 1, "after")
@@ -106,6 +111,10 @@ retry_max_seconds <- function(req) {
 }
 
 retry_is_transient <- function(req, resp) {
+  if (is_error(resp)) {
+    return(req$policies$retry_on_failure %||% FALSE)
+  }
+
   req_policy_call(req, "retry_is_transient", list(resp),
     default = function(resp) resp_status(resp) %in% c(429, 503)
   )
@@ -116,6 +125,10 @@ retry_backoff <- function(req, i) {
 }
 
 retry_after <- function(req, resp, i, error_call = caller_env()) {
+  if (is_error(resp)) {
+    return(retry_backoff(req, i))
+  }
+
   after <- req_policy_call(req, "retry_after", list(resp), default = resp_retry_after)
 
   # TODO: apply this idea to all callbacks

R/resp-headers.R

@@ -134,6 +134,8 @@ resp_encoding <- function(resp) {
 #' resp <- response(headers = "Retry-After: Mon, 20 Sep 2025 21:44:05 UTC")
 #' resp |> resp_retry_after()
 resp_retry_after <- function(resp) {
+  check_response(resp)
+
   # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
   val <- resp_header(resp, "Retry-After")
   if (is.null(val)) {

tests/testthat/_snaps/req-retries.md

@@ -18,4 +18,9 @@
     Condition
       Error in `req_retry()`:
       ! `max_seconds` must be a whole number or `NULL`, not the string "x".
+    Code
+      req_retry(req, retry_on_failure = "x")
+    Condition
+      Error in `req_retry()`:
+      ! `retry_on_failure` must be `TRUE` or `FALSE`, not the string "x".
 

tests/testthat/test-req-perform.R

@@ -54,6 +54,16 @@ test_that("persistent HTTP errors only get single attempt", {
   expect_equal(cnd$n, 1)
 })
 
+test_that("don't retry curl errors by default", {
+  req <- request("") %>% req_retry(max_tries = 2)
+  expect_error(req_perform(req), class = "httr2_failure")
+
+  # But can opt-in to it
+  req <- request("") %>% req_retry(max_tries = 2, retry_on_failure = TRUE)
+  cnd <- catch_cnd(req_perform(req), "httr2_retry")
+  expect_equal(cnd$tries, 1)
+})
+
 test_that("can retry a transient error", {
   req <- local_app_request(function(req, res) {
     i <- res$app$locals$i %||% 1
@@ -163,7 +173,7 @@ test_that("can retrieve last request and response", {
 })
 
 test_that("can last response is NULL if it fails", {
-  req <- request("frooble") %>% req_timeout(0.1)
+  req <- request("")
   try(req_perform(req), silent = TRUE)
 
   expect_equal(last_request(), req)

tests/testthat/test-req-retries.R

@@ -72,6 +72,7 @@ test_that("validates its inputs", {
   expect_snapshot(error = TRUE, {
     req_retry(req, max_tries = 1)
     req_retry(req, max_seconds = "x")
+    req_retry(req, retry_on_failure = "x")
   })
 })