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

Change the default failure in parse_http_date() to NA with class Date #544

Merged
merged 3 commits into from
Nov 22, 2018

Conversation

shrektan
Copy link
Contributor

@shrektan shrektan commented Oct 10, 2018

The default value of failure argument in parse_http_date() should be NA with class Date.

Otherwise, print.response() may throw an unfriendly error.

On a Chinese-language Windows machine, I use plumber to build an REST API. The API returns the header$date in Chinese format by default, like "周三, 10 十月 2018 1:19:13 GMT".

parse_http_date() will fail to parse it correctly and fall back to the default value of `failure``.

However, the default value of failure is set to NA (the logical value). Whenever I try to print the response, R will throw error saying:

Error in prettyNum(.Internal(format(x, trim, digits, nsmall, width, 3L, : invalid 'trim' argument

I believe the cause is

cat(" Date: ", format(x$date, "%Y-%m-%d %H:%M"), "\n", sep = "")

After this PR, the result can be printed successfully now.

Response [http://127.0.0.1:8001/reLoad]
  Date: NA
  Status: 200
  Content-Type: application/json
  Size: 2 B

…th class Date

Otherwise, print.response() may throw an unfriendly error.
@shrektan shrektan changed the title The default failure in parse_http_date() NA with class Date Change the default failure in parse_http_date() to NA with class Date Oct 10, 2018
@hadley
Copy link
Member

hadley commented Nov 22, 2018

Thanks!

@shrektan shrektan deleted the na-with-class-date branch November 22, 2018 00:58
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 this pull request may close these issues.

2 participants