Skip to content

Clean up resp_parse() #90

@jonthegeek

Description

@jonthegeek

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.

Metadata

Metadata

Labels

No labels
No labels

Type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions