Skip to content

Commit

Permalink
Handle error responses without any content type.
Browse files Browse the repository at this point in the history
In some circumstances, Packit returns an HTTP error with no body and no
Content-Type header. In those cases, httr2 returns NA as the
content type.

#166 added a check to make sure the content type is JSON before decoding
the error, however the check did not handle NA values properly and gave
a `missing value where TRUE/FALSE needed`.

This now uses `identical` instead of `==` to compare the content type
string against the expected string.
  • Loading branch information
plietar committed Sep 5, 2024
1 parent 08a2f60 commit 06cf405
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
2 changes: 1 addition & 1 deletion R/outpack_http_client.R
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ http_client_handle_error <- function(response) {
## for that and then reauthenticate; that requires that a callback
## is passed through here too.
if (httr2::resp_is_error(response)) {
if (httr2::resp_content_type(response) == "application/json") {
if (identical(httr2::resp_content_type(response), "application/json")) {
res <- httr2::resp_body_json(response)
## I am seeing Packit returning an element 'error' not a list of
## errors
Expand Down
4 changes: 3 additions & 1 deletion tests/testthat/helper-outpack-http.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ mock_headers <- function(...) {

mock_response <- function(content, status = 200L, wrap = TRUE,
download = NULL) {
headers <- mock_headers()
if (is.raw(content)) {
headers <- mock_headers("content-type" = "application/octet-stream")
} else if (inherits(content, "json")) {
Expand All @@ -18,6 +17,9 @@ mock_response <- function(content, status = 200L, wrap = TRUE,
} else if (is.character(content)) {
headers <- mock_headers("content-type" = "text/plain")
content <- c(writeBin(content, raw()), as.raw(0L))
} else if (is.na(content)) {
headers <- mock_headers()
content <- raw(0)
} else {
stop("Unhandled mock response type")
}
Expand Down
12 changes: 12 additions & 0 deletions tests/testthat/test-outpack-http-client.R
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ test_that("handle plain text errors", {
})


test_that("handle empty errors", {
local_mock_response(NA, status = 503L, wrap = FALSE)

cl <- outpack_http_client$new("http://example.com", NULL)
err <- expect_error(cl$request("path"), "Service Unavailable")

expect_s3_class(err, "outpack_http_client_error")
expect_equal(err$code, 503)
expect_null(err$errors)
})


test_that("can use the client to make requests", {
skip_if_not_installed("mockery")

Expand Down

0 comments on commit 06cf405

Please sign in to comment.