Better error message in req_retry()

NEWS.md

@@ -1,5 +1,8 @@
 # httr2 (development version)
 
+* `req_retry()` gives a clearer error if `after` returns the wrong type of 
+  value (#385).
+
 * `req_template()` now works when you have a bare `:` in a template that
   uses "uri" style (#389).
 

R/req-retries.R

@@ -110,15 +110,29 @@ retry_backoff <- function(req, i) {
   req_policy_call(req, "retry_backoff", list(i), default = backoff_default)
 }
 
-retry_after <- function(req, resp, i) {
+retry_after <- function(req, resp, i, error_call = caller_env()) {
   after <- req_policy_call(req, "retry_after", list(resp), default = resp_retry_after)
+
+  # TODO: apply this idea to all callbacks
+  if (!is_number_or_na(after)) {
+    not <- obj_type_friendly(after)
+    cli::cli_abort(
+      "The {.code after} callback to {.fn req_retry} must return a single number or NA, not {not}.",
+      call = error_call
+    )
+  }
+
   if (is.na(after)) {
     retry_backoff(req, i)
   } else {
     after
   }
 }
 
+is_number_or_na <- function(x) {
+  (is.numeric(x) && length(x) == 1) || identical(x, NA)
+}
+
 # Helpers -----------------------------------------------------------------
 
 # Exponential backoff with full-jitter, capped to 60s wait

tests/testthat/_snaps/req-retries.md

@@ -0,0 +1,8 @@
+# useful message if `after` wrong
+
+    Code
+      req_perform(req)
+    Condition
+      Error in `req_perform()`:
+      ! The `after` callback to `req_retry()` must return a single number or NA, not a <httr2_response> object.
+

tests/testthat/test-req-retries.R

@@ -55,3 +55,23 @@ test_that("missing retry-after uses backoff", {
 
   expect_equal(retry_after(req, response(429), 1), 10)
 })
+
+test_that("useful message if `after` wrong", {
+  req <- request_test() %>%
+    req_retry(
+      is_transient = function(resp) TRUE,
+      after = function(resp) resp
+    )
+
+  expect_snapshot(req_perform(req), error = TRUE)
+})
+
+test_that("is_number_or_na implemented correctly", {
+  expect_equal(is_number_or_na(1), TRUE)
+  expect_equal(is_number_or_na(NA_real_), TRUE)
+  expect_equal(is_number_or_na(NA), TRUE)
+
+  expect_equal(is_number_or_na(1:2), FALSE)
+  expect_equal(is_number_or_na(numeric()), FALSE)
+  expect_equal(is_number_or_na("x"), FALSE)
+})