go-github: rate limit exceeded: funky error message

The error message for “rate limit exceeded” has both a period and a semicolon in it which looks weird.

GET https://api.github.com/repos/golang/go/issues?direction=desc&page=1&per_page=100&sort=updated&state=all: 403 API rate limit of 5000 
still exceeded until 2017-04-21 08:38:57 -0700 PDT, not making remote request.; rate reset 
in 26m8.032328588s

We could also probably round the duration to the nearest second (if greater than a minute) or tenth of a second (if less than a minute) to avoid 8 useless units of precision after the decimal.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Comments: 19 (11 by maintainers)

Commits related to this issue

Most upvoted comments

I’m okay with omitting leading zeros, but I think it’s probably better to keep the ones in middle. E.g.:

  • 5m54s
  • 25m07s
  • 19s

My opinion on this is not strong. That said, I’d prefer something with a simpler implementation, rather than a very complex custom one.

Thanks, I’ll start working on this and update this thread with the final fix (whichever happens to balance complexity and readability)

Since we’re already adding custom code to control how the duration is formatted, would it be reasonable to implement something like:

[rate limit was reset 121m22s ago]

If this message is intended for humans, then the leading 0s seem verbose to me. I don’t think the caller would try to parse this message anyway (they’d use members in RateLimitError.Rate).

Regarding precision, I agree that being consistent is better.

  • 5m54s
  • 25m7s
  • 19s

This is a matter of opinion, so I’m alright with swinging either way.

Roger that.

Proposed error message(s):

a. rate reset duration <1m

GET https://api.github.com/users: 403 API rate limit of 5000 still exceeded until 2017-04-21 08:38:57 -0700 PDT, not making remote request. [Rate reset in 8.3s]

b. rate reset duration >1m

GET https://api.github.com/users: 403 API rate limit of 5000 still exceeded until 2017-04-21 08:38:57 -0700 PDT, not making remote request. [Rate reset in 9m50s]

c. unauthenticated user

GET https://api.github.com/users: 403 API rate limit exceeded for xxx.xxx.xxx.xxx. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.) [Rate reset in 58m41s]

I’m proposing square brackets for [Rate reset in 58m41s] since the message from Github in © already has a parenthesized snippet. That would make it similar to the generic error message:

GET https://api.github.com/user: 401 Bad credentials []

Thanks for the guidance, Dmitri.

When doing that, we should consider what a real response from GitHub API says when over rate limit.

Here’s the response from the Github API (from their documentation):

HTTP/1.1 403 Forbidden
Date: Tue, 20 Aug 2013 14:50:41 GMT
Status: 403 Forbidden
X-RateLimit-Limit: 60
X-RateLimit-Remaining: 0
X-RateLimit-Reset: 1377013266
{
   "message": "API rate limit exceeded for xxx.xxx.xxx.xxx. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
   "documentation_url": "https://developer.github.com/v3/#rate-limiting"
}

X-RateLimit-Reset is in UTC epoch seconds and Github leaves it to the application to format and display it.

We should also consider the other error messages, such as ErrorResponse.Error, to make sure things are consistent.

Here’s a sample generic error message;

GET https://api.github.com/user: 401 Bad credentials []

The extra [] at the end would presumably dump the ‘errors’ array from an API response JSON like this:

{
  "message": "Validation Failed",
  "errors": [  //< This array would be dumped
    {
      "resource": "Issue",
      "field": "title",
      "code": "missing_field"
    }
  ]
}