Issues found
1. Bug: .resps_combine() silently drops its own discard() result
File: R/resp_parse.R, line 87
.resps_combine <- function(resps_parsed) {
purrr::discard(resps_parsed, is.null) # <-- result not assigned!
if (inherits(resps_parsed[[1]], "raw")) {
...
purrr::discard() returns a new list; it doesn't mutate in place. The filtered result is thrown away, so NULL elements (from empty pages via resp_tidy_json) are never removed. This will cause vctrs::list_unchop() to error when any paginated page returns no data.
Fix: Assign the result back: resps_parsed <- purrr::discard(resps_parsed, is.null)
2. Bug: .resps_combine() errors on empty input
After fixing issue 1, if all responses are NULL (or all were failures filtered by resps_successes()), resps_parsed will be an empty list and resps_parsed[[1]] will throw a subscript-out-of-bounds error.
Fix: Add an early return for empty input:
.resps_combine <- function(resps_parsed) {
resps_parsed <- purrr::discard(resps_parsed, is.null)
if (!length(resps_parsed)) {
return(NULL)
}
...
3. Missing code chunk in vignette
File: vignettes/nectar.Rmd, between lines 28-30
The section "Preparing a request with req_prepare()" describes setting up a basic request with rows = 10, select, .multi, and cursor = "*", but there is no code chunk showing it. The text at line 27 ends with a colon ("...to trigger cursor-based pagination:") and then jumps directly to the "Authentication" section header on line 30.
Fix: Add the missing introductory req_prepare() code chunk (without auth/tidy/pagination) between lines 28 and 30.
4. Stale response_parser parameter documentation
File: R/aaa-shared_params.R, lines 73-76
The @param response_parser docstring says the default is httr2::resp_body_json(), but the actual default in resp_parse.httr2_response and resp_parse.list is resp_tidy.
Fix: Update the documentation to reflect the actual default and describe what resp_tidy does.
5. Add test coverage for the above fixes
- Test that
.resps_combine() handles NULL elements (from empty pages).
- Test that
.resps_combine() / resp_parse() handles the all-NULL / all-empty case gracefully.
Issues found
1. Bug:
.resps_combine()silently drops its owndiscard()resultFile:
R/resp_parse.R, line 87purrr::discard()returns a new list; it doesn't mutate in place. The filtered result is thrown away, so NULL elements (from empty pages viaresp_tidy_json) are never removed. This will causevctrs::list_unchop()to error when any paginated page returns no data.Fix: Assign the result back:
resps_parsed <- purrr::discard(resps_parsed, is.null)2. Bug:
.resps_combine()errors on empty inputAfter fixing issue 1, if all responses are NULL (or all were failures filtered by
resps_successes()),resps_parsedwill be an empty list andresps_parsed[[1]]will throw a subscript-out-of-bounds error.Fix: Add an early return for empty input:
3. Missing code chunk in vignette
File:
vignettes/nectar.Rmd, between lines 28-30The section "Preparing a request with
req_prepare()" describes setting up a basic request withrows = 10,select,.multi, andcursor = "*", but there is no code chunk showing it. The text at line 27 ends with a colon ("...to trigger cursor-based pagination:") and then jumps directly to the "Authentication" section header on line 30.Fix: Add the missing introductory
req_prepare()code chunk (without auth/tidy/pagination) between lines 28 and 30.4. Stale
response_parserparameter documentationFile:
R/aaa-shared_params.R, lines 73-76The
@param response_parserdocstring says the default ishttr2::resp_body_json(), but the actual default inresp_parse.httr2_responseandresp_parse.listisresp_tidy.Fix: Update the documentation to reflect the actual default and describe what
resp_tidydoes.5. Add test coverage for the above fixes
.resps_combine()handles NULL elements (from empty pages)..resps_combine()/resp_parse()handles the all-NULL / all-empty case gracefully.