General modernization of some errors; better handling of HTML error #255
Conversation
| template = "Expected content type {.field application/json}, not \\ | ||
| {.field <<x>>}." | ||
| ), | ||
| "*" = obfuscate(content, first = 197, last = 0) |
There was a problem hiding this comment.
Note: this is where the Could not parse cli {} expression: ... error was coming from (#254). When the content is html, it can contain curly braces (and, indeed, in this case, did), which then confuses cli when it processes the error message bullets.
There is a solution, which is to refer to the {-and-}-containing string from a template:
https://cli.r-lib.org/reference/inline-markup.html#escaping-and-
However, I decided not to do that, but instead to make it much easier to see the full html. Previously, my only direct experience of html errors was from the token info endpoints, which seemed rather obscure and the examples I knew about produced a relatively small amount of html, so it seemed sensible to show a part (and maybe all) of the html in the actual error message.
But the error seen in #254 is actually important for the user (429 too many requests, from googledrive) and involved considerably more html (so a wee little excerpt didn't reveal anything useful).
There was a problem hiding this comment.
This pitfall is also exactly described here:
| gargle_error_message <- function(resp, call = caller_env()) { | ||
| type <- httr::http_type(resp) | ||
| if (type == "text/html") { | ||
| return(gargle_html_error_message(resp)) |
There was a problem hiding this comment.
Instead of letting html trigger an error in check_for_json() inside response_as_json(), I handle that case explicitly here.
| ! Client error: (404) Not Found | ||
| x Expected content type 'application/json', not 'text/html'. | ||
| i See 'VOLATILE_FILE_PATH' for the html error content. | ||
| i Or execute `browseURL("VOLATILE_FILE_PATH")` to view it in your browser. |
There was a problem hiding this comment.
This is a look at the new handling for an error that comes as html. In certain contexts, such as RStudio, the "See 'VOLATILE_FILE_PATH' ..." is a clickable link to open the file. I can't make the browseURL() a clickable link because it calls into a base package. But the user can copy it, paste into the console, and execute without too much trouble.
https://cli.r-lib.org/reference/links.html#security-considerations
Seems weird to have a mix of parent.frame() and caller_env(), so prefer the latter.
Content type might look like `"text/html; charset=UTF-8"` (even though httr seems to just return `"text/html"` in that case)
Start to use more modern features of our error-throwing tools. Do a better job of surfacing error content that comes as HTML, which is gross, but happens.
Fixes #254
(This only deals with errors in the response processing domain. Wiring
callinto the auth part of gargle would be harder and have less payoff, since the vast majority of usage happens viatoken_fetch()which silently swallows errors. So it's not clear if I will ever do that.)