Skip to content

Commit

Permalink
Merge pull request #14 from conbench/revert-13-more-verbose-error
Browse files Browse the repository at this point in the history
Revert "Provide better errors when conbench is unreachable"
  • Loading branch information
boshek authored Sep 19, 2023
2 parents 5a8bfff + 699cb5c commit 14b6e65
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 32 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: conbenchcoms
Title: An API wrapper for conbench communications
Version: 0.0.5
Version: 0.0.4
Authors@R: c(
person("Jonathan", "Keane", , "[email protected]", role = c("aut", "ctb"),
comment = c(ORCID = "0000-0001-7087-9776")),
Expand Down
3 changes: 0 additions & 3 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# conbenchcoms 0.0.5
* display more verbose errors when the Conbench API returns an error

# conbenchcoms 0.0.4
* add threshold endpoints to conbenchcoms

Expand Down
30 changes: 8 additions & 22 deletions R/session.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,20 @@
#' @return the response
#' @export
conbench_perform <- function(data, ...) {
# Make an initial call to see if we need to authenticate
# if session is already here, then we can use that
resp <- data |>
req_error(is_error = function(resp) FALSE) |>
req_headers(cookie = .conbench_session$cookie) |>
req_perform(...)

# Authenticate if we need to
if (resp_status(resp) == 401L) auth_conbench()
# TODO: is this status too narrow?
if (resp_status(resp) == 401L) {
auth_conbench()

# Run the request again with better error handling
resp <- data |>
req_error(is_error = function(resp) TRUE, body = error_body) |>
req_headers(cookie = .conbench_session$cookie) |>
req_perform(...)
resp <- data |>
req_headers(cookie = .conbench_session$cookie) |>
req_perform(...)
}

resp
}
Expand All @@ -42,13 +42,6 @@ auth_conbench <- function() {
.conbench_session$cookie <- resp_header(resp, "set-cookie")
}

error_body <- function(resp) {
method <- indent(glue::glue("{resp['method']} {resp['url']}"))
status_code <- indent(glue::glue("Status code: {resp[['status_code']]}"))
indented_message <- indent(resp_body_string(resp))
glue::glue("Request details:\n{method}\n\nResponse details:\n{status_code}\n---\n\n{indented_message}")
}

get_config <- function() {
path <- Sys.getenv("DOT_CONBENCH", ".conbench")
creds_list <- list(
Expand All @@ -71,13 +64,6 @@ get_config <- function() {
creds
}

indent <- function(message) {
lines <- unlist(strsplit(message, "\n")) # Split the message into lines
indent <- paste(rep(" ", 2), collapse = "") # Create the indentation string
indented_lines <- paste(indent, lines, sep = "") # Add indentation
paste(indented_lines, collapse = "\n") # Combine the lines with newline characters
}

.conbench_session <- new.env(parent = emptyenv())


6 changes: 2 additions & 4 deletions man/compare.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions tests/testthat/test-session.R
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ test_that("get_config works to three env vars even when dot_conbench is present"
expect_identical(set3envs$password, "starr")
})

test_that("get_config errors when it has no environment variables", {
test_that("get_config errors when it has no environment variables",{
expect_error(
test_get_config_env_vars(
list(
Expand All @@ -40,7 +40,6 @@ with_mock_dir(test_path("not-logged-in"), {
# This is still a 401 becuase httptest2 only has one possible response for users
expect_identical(resp_status(resp), 401L)
expect_identical(.conbench_session$cookie, "REDACTED")

})
})

Expand Down

0 comments on commit 14b6e65

Please sign in to comment.