Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Record requests in responses and errors #359

Merged
merged 5 commits into from
Oct 27, 2023
Merged

Record requests in responses and errors #359

merged 5 commits into from
Oct 27, 2023

Conversation

hadley
Copy link
Member

@hadley hadley commented Oct 26, 2023

And polish the helpers for working with lists.

Fixes #357

@hadley hadley requested a review from mgirlich October 26, 2023 18:14
@hadley hadley changed the title Card requests in responses and errors Record requests in responses and errors Oct 26, 2023
check_installed("vctrs")
check_function2(resp_data, "resp")

vctrs::list_unchop(lapply(resps, resp_data))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe resps_data() should check that all responses were successful. And I get this beauty when applying to an error:

devtools::load_all("~/GitHub/httr2/")
#> ℹ Loading httr2
reqs <- list(
  request(example_url()) |> req_template("/status/:status", status = 404),
  request("INVALID")
)
resps <- req_perform_parallel(reqs)

resps |> resps_data(\(resp) resp_body_json(resp))
#> Error in `env_has()` at httr2/R/resp-body.R:82:2:
#> ! `env` must be an environment, not `NULL`.
#> Backtrace:
#>     ▆
#>  1. └─httr2::resps_data(resps, function(resp) resp_body_json(resp))
#>  2.   ├─vctrs::list_unchop(lapply(resps, resp_data)) at httr2/R/iterate-responses.R:62:2
#>  3.   └─base::lapply(resps, resp_data)
#>  4.     └─global FUN(X[[i]], ...)
#>  5.       └─httr2::resp_body_json(resp)
#>  6.         └─rlang::env_has(env = resp$cache) at httr2/R/resp-body.R:82:2
#>  7.           └─rlang:::check_environment(env)
#>  8.             └─rlang:::stop_input_type(...)
#>  9.               └─rlang::abort(message, ..., call = call, arg = arg)

Created on 2023-10-27 with reprex v2.0.2

This suggests that we should be more careful with the caching in resp_body_json() and resp_body_xml()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops, no, that's just because check_response(resp) should come before the cache lookup, not after 😬

@mgirlich
Copy link
Collaborator

This looks very nice for debugging!

@hadley hadley merged commit 8ecb2c4 into main Oct 27, 2023
7 of 12 checks passed
@hadley hadley deleted the response-request branch October 27, 2023 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store request in response and errors
2 participants