Merge pull request #177 from mrc-ide/mrc-5706-error-no-json

Handle error responses without any content type.

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: orderly2
 Title: Orderly Next Generation
-Version: 1.99.33
+Version: 1.99.34
 Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"),
                     email = "[email protected]"),
              person("Robert", "Ashton", role = "aut"),

R/outpack_http_client.R

@@ -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

tests/testthat/helper-outpack-http.R

@@ -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")) {
@@ -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")
   }

tests/testthat/test-outpack-http-client.R

@@ -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")