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

Github can return api rate limit for ip. #321

Closed
3 tasks
compico opened this issue Jun 5, 2022 · 6 comments
Closed
3 tasks

Github can return api rate limit for ip. #321

compico opened this issue Jun 5, 2022 · 6 comments
Assignees

Comments

@compico
Copy link

compico commented Jun 5, 2022

Github can return api rate limit for ip.

I installed noisetorch service according to the instructions https://github.com/noisetorch/NoiseTorch/wiki/Start-automatically-with-Systemd
By my mistake, the program restarted endlessly. As a result I got a softban in github api.

GET https://api.github.com/repos/noisetorch/NoiseTorch/releases?per_page=1&page=1

{
    "message" : "API rate limit exceeded for ip 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://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting"
}

Noisetorch Error:

Reading JSON for latest_release failed json: cannot unmarshal object into Go value of type main.github_releases
panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
main.getLatestRelease()
	noisetorch/update.go:199 +0x477
main.init()
	noisetorch/update.go:77 +0x2e

Originally posted by @compico in #316 (comment)

  • Add in github_releases struct field message and documentation_url.
  • Add reading headers http request for caching
    x-ratelimit-limit
    x-ratelimit-remaining
    x-ratelimit-reset
    x-ratelimit-used
  • Add caching body in function getLatestRelease() to limit the number of requests.
@stasadev
Copy link

stasadev commented Jun 5, 2022

@Technetium1
Copy link

Welcome to steal any of the logic for my GitHub release update check here: https://github.com/Technetium1/ChocolateyUpdate/blob/40b481391d2dfe98f17e734549c1ff8a16665138/ChocolateyUpdate.py#L108

@AXDOOMER
Copy link
Collaborator

AXDOOMER commented Jun 5, 2022

I've pushed an update so that being rate limited does not result in a crash. I'll will be merged when @ZyanKLee comes back.

@AXDOOMER AXDOOMER self-assigned this Jun 5, 2022
@compico
Copy link
Author

compico commented Jun 5, 2022

NoiseTorch/update.go

Lines 185 to 194 in 68991f8

body, readErr := ioutil.ReadAll(res.Body)
if readErr != nil {
log.Fatal(readErr)
}
respBytes := []byte(body)
var latest_release github_releases
err = json.Unmarshal(respBytes, &latest_release)

Here is a useless declaration of a variable respBytes.
Can be shortened to

	body, readErr := ioutil.ReadAll(res.Body)
	if readErr != nil {
		log.Fatal(readErr)
	}

	var latest_release github_releases

	err = json.Unmarshal(body, &latest_release)

@AXDOOMER
Copy link
Collaborator

AXDOOMER commented Jun 5, 2022

Thank you for your suggestions @compico and @stasadev. I'll do some refactoring once #317 is merged.

@AXDOOMER
Copy link
Collaborator

AXDOOMER commented Jun 6, 2022

Crash fixed in #317 which has just been merged.

As for the rate limit, Github allows 50 API requests per hour per IP, so I think it's enough. I took note of your suggestions and will apply them very soon.

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

No branches or pull requests

4 participants