-
Notifications
You must be signed in to change notification settings - Fork 85
Closed
Description
This is issue is the same error as #277 but in its minimal form.
I stumbled upon this error while trying to do some mocking in testing with httr2::response().
> httr2::resp_body_json(httr2::response())
Error in `if (!is.null(suffix) && endsWith(content_type, suffix)) ...`:
! missing value where TRUE/FALSE needed
Run `rlang::last_trace()` to see where the error occurred.
> rlang::last_trace()
<error/rlang_error>
Error in `if (!is.null(suffix) && endsWith(content_type, suffix)) ...`:
! missing value where TRUE/FALSE needed
---
Backtrace:
▆
1. └─httr2::resp_body_json(httr2::response())
2. └─httr2:::check_content_type(...)I don't think the error is very clear to understand is the call is really allowed or not.
Simpler reprex:
httr2:::check_content_type(httr2::response(), "application/json", suffix = "+json")In there,
Lines 130 to 138 in 7962bcd
| content_type <- resp_content_type(resp) | |
| if (content_type %in% types) { | |
| return() | |
| } | |
| # https://datatracker.ietf.org/doc/html/rfc6838#section-4.2.8 | |
| if (!is.null(suffix) && endsWith(content_type, suffix)) { | |
| return() | |
| } |
content_type can be NA_character_Lines 101 to 107 in d789827
| resp_content_type <- function(resp) { | |
| if (resp_header_exists(resp, "content-type")) { | |
| parse_media(resp_header(resp, "content-type"))$type | |
| } else { | |
| NA_character_ | |
| } | |
| } |
and this will make endsWith() returns a NA
endsWith(NA_character_, "+json")
#> [1] NAwhich is not good in the if() call and with &&
#277 was fixed by #278 but regarding the specific cache usage.
I believe here the error should be clearer. Maybe is.na(content_types) should be checked also when !is.null(suffix) to handle this specific content_types value, and throw useful error.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels