-
Notifications
You must be signed in to change notification settings - Fork 57
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
Conversation
check_installed("vctrs") | ||
check_function2(resp_data, "resp") | ||
|
||
vctrs::list_unchop(lapply(resps, resp_data)) |
There was a problem hiding this comment.
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()
There was a problem hiding this comment.
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 😬
This looks very nice for debugging! |
And polish the helpers for working with lists.
Fixes #357