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

Unhandled exception raised at runtime: 'x-ratelimit-remaining'. #57

Closed
id-tari opened this issue Mar 9, 2023 · 7 comments
Closed

Unhandled exception raised at runtime: 'x-ratelimit-remaining'. #57

id-tari opened this issue Mar 9, 2023 · 7 comments

Comments

@id-tari
Copy link

id-tari commented Mar 9, 2023

Running action v2.
During the first run it has discovered and deleted 147 untagged, 1 week old images.
By the end action throws exception messages:

Unhandled exception raised at runtime: `'x-ratelimit-remaining'`. Please report this at https://github.com/snok/container-retention-policy/issues/new

604 times/log-lines

Unfortunately there are no additional outputs, and action did not exit with error.
Action has finished "successfully".

Initially as test I was trying to delete only untagged, at least 1 week old images.
After initial run I had left 521 untagged image.

2nd run has discovered 70 images for deletion. Tons of exception messages still appear.
3rd run deleted 96 images.
4th run deleted 61 images. etc ...

Even "enable debug output" on action run is not showing anything useful.
I'm not sure how to debug this behavior, will greatly appreciate your ideas. (not a pro python developer)

@sondrelg
Copy link
Member

sondrelg commented Mar 9, 2023

Thanks @id-tari, the error you see is the "catch-all" exception handler here 👍

If I had to guess, I'm assuming the response from github doesn't contain the x-ratelimit-remaining header we're expecting here. We should probably change this code to:

-    if int(response.headers['x-ratelimit-remaining']) == 0:
+    if int(response.headers.get('x-ratelimit-remaining', default=0)) == 0:

That's just my first instinct - could be wrong 🙂

Would you be interested in trying to create a fix? If you're comfortable with it, how you would need to do it is:

  1. Fork this project
  2. Commit the change above
  3. Then in your workflow, to test it works, you can change - uses: snok/container-retention-policy@v2 to - uses: id-tari/container-retention-policy@main and run the workflow, and it should run it with your new code
  4. Submit the PR here with the fix 🙂

If that sounds too complicated, I might have time to fix this later in the week 🙂

@id-tari
Copy link
Author

id-tari commented Mar 13, 2023

Thanks @sondrelg!
I've applied and tried your suggested fix.

Now there is another error:

Unhandled exception raised at runtime: 'x-ratelimit-reset'.

This time it does not make sense to set default value for a key if header is not found.

Something is wrong really wrong, headers and values are missing.

Unfortunately looks like I don't have time to learn asyncio and httpx library.
I hope this will be a trivial issue to fix for somehow who has implemented that.

Docs:
https://docs.github.com/en/rest/rate-limit?apiVersion=2022-11-28
https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limit-http-headers

@sondrelg
Copy link
Member

Sorry, actually if you just do this instead

-    if int(response.headers['x-ratelimit-remaining']) == 0:
+    if int(response.headers.get('x-ratelimit-remaining', default=1)) == 0:

it should work

@id-tari
Copy link
Author

id-tari commented Mar 13, 2023

Yay worked! I'll make a PR from a different acc.

@Pho3niX90
Copy link

Same same. just posting here so there are no duplications.
image

@sondrelg
Copy link
Member

sondrelg commented Aug 3, 2023

Thanks @Pho3niX90. I must have missed the PR for this. If I can I'll take a look and get it merged this weekend 👍

@sondrelg sondrelg mentioned this issue Jun 23, 2024
@sondrelg
Copy link
Member

The latest release redoes rate limiting completely. It should fix this issue, and a few more.

If you have time, the migration guide for v3 is included in the release post 👍 I would greatly appreciate feedback if you have any.

If you run into any issues, please share them in the issue opened for tracking the v3 release ☺️

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

3 participants