Skip to content

Commit 2134e18

Browse files
authored
Only attempt to close responses (#818)
Fixes #817
1 parent fb371f4 commit 2134e18

5 files changed

Lines changed: 28 additions & 8 deletions

File tree

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# httr2 (development version)
22

3+
* `req_perform_connection()` no longer errors with `no applicable method for 'close' applied to an object of class "c('httr2_failure', 'httr2_error', 'rlang_error', 'error', 'condition')` (#817).
34
* Refactor `url_modify()` to better retain exact formatting of URL components
45
that are not modified. (#788, #794)
56

R/req-error.R

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,11 @@ error_body <- function(req, resp, call = caller_env()) {
112112
}
113113

114114
capture_curl_error <- function(code, call = caller_env()) {
115-
resp <- tryCatch(
116-
code,
115+
tryCatch(
116+
{
117+
code
118+
NULL
119+
},
117120
error = function(err) curl_cnd(err, call = call)
118121
)
119122
}

R/req-perform-connection.R

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ req_perform_connection <- function(
8181
retry_check_breaker(req, tries)
8282
sys_sleep(delay, "for retry backoff")
8383

84-
if (!is.null(resp)) {
84+
if (is_response(resp)) {
8585
close(resp)
8686
}
8787
resp <- req_perform_connection1(req, handle, blocking = blocking)
@@ -143,14 +143,11 @@ req_perform_connection1 <- function(req, handle, blocking = TRUE) {
143143
err <- capture_curl_error({
144144
conn <- curl::curl(req$url, handle = handle)
145145
# Must open the stream in order to initiate the connection
146-
withCallingHandlers(
147-
open(conn, "rbf", blocking = blocking),
148-
warning = \(cnd) tryInvokeRestart("muffleWarning"),
149-
error = \(cnd) close(conn)
150-
)
146+
suppressWarnings(open(conn, "rbf", blocking = blocking))
151147
body <- StreamingBody$new(conn)
152148
})
153149
if (is_error(err)) {
150+
close(conn)
154151
return(err)
155152
}
156153

tests/testthat/_snaps/req-perform-connection.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@
2121
Caused by error in `open()`:
2222
! Failed to connect
2323

24+
# correclty reports curl error with retries (#817)
25+
26+
Code
27+
req_perform_connection(req)
28+
Condition
29+
Error in `req_perform_connection()`:
30+
! Failed to perform HTTP request.
31+
Caused by error in `open()`:
32+
! Failed to connect
33+
2434
# validates its input
2535

2636
Code

tests/testthat/test-req-perform-connection.R

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,15 @@ test_that("curl errors become errors", {
7070
expect_null(last_response())
7171
})
7272

73+
test_that("correclty reports curl error with retries (#817)", {
74+
local_mocked_bindings(open = function(...) abort("Failed to connect"))
75+
76+
req <- request("http://127.0.0.1") |>
77+
req_retry(max_tries = 2, retry_on_failure = TRUE, backoff = \(i) 0)
78+
79+
expect_snapshot(req_perform_connection(req), error = TRUE)
80+
})
81+
7382
test_that("mocking works", {
7483
req_200 <- request("https://ok")
7584
req_404 <- request("https://notok")

0 commit comments

Comments
 (0)