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

fromJSON is.na check is incomplete #211

Open
struckma opened this issue Nov 7, 2017 · 3 comments
Open

fromJSON is.na check is incomplete #211

struckma opened this issue Nov 7, 2017 · 3 comments

Comments

@struckma
Copy link

struckma commented Nov 7, 2017

Thank you for your great contribution to the R world, I am using jsonlite on a regular basis and it is a great tool. Nevertheless, I found a minor issue in the error reporting of fromJSON, which is slightly confusing in my oppinion, and you may change this easily, if you agree:

The function fromJSON checks, if input is NA (jsonlite_1.5):

if (is.character(txt) && length(txt) == 1 && nchar(txt, type="bytes") < 1000 && !validate(txt)) {

This works, as long as the input is not NA. Usually, NA is logical, so the line above will treat the situation:
if (!is.character(txt) && !inherits(txt, "connection")) {

However, if I call fromJSON with a NA which was casted to a character, it won't (error messages translated from German, may not be excatly the original terms):

> fromJSON(NA)
Error: Argument 'txt' must be a JSON string, URL or file.
> fromJSON(as.character(NA))
Error in if (is.character(txt) && length(txt) == 1 && nchar(txt, type = "bytes") <  : 
  Missing value, where TRUE/FALSE expected

What I would expect, is the following behaviour:

> fromJSON(NA)
Error: Argument 'txt' must be a JSON string, URL or file.
> fromJSON(as.character(NA))
Error: Argument 'txt' must be a JSON string, URL or file.

So, you should amend

if (!is.character(txt) && !inherits(txt, "connection")) {
by a NA check, something like:

  if (is.null(txt) || is.na(txt) || (!is.character(txt) && !inherits(txt, "connection"))) {

I also added is.null, since this causes similar confusing error messages.
Feel free to use my suggested code, if you like, I would be happy to have clearer error messages in future. Thank you again

Stephan

@MichaelChirico
Copy link
Contributor

I just got bit by this again. I also realize I'm not sure I ever filed a PR on this commit from 1.5 years ago:

MichaelChirico@7e41e30

@kieran-mace
Copy link

Getting this issue too.

@francisbarton
Copy link

francisbarton commented Jul 29, 2022

I'm getting this too.
In my case the cause of the issue was a single invalid character (some Unicode issue I think) in the source data provided by an API, which httr2::resp_body_json() would fail to read without an error. (resp_body_json uses jsonlite I believe).

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 a pull request may close this issue.

4 participants