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

Surfacing GH Rate Limit Errors #1076

Closed
10 tasks done
aj-codecov opened this issue Jan 22, 2024 · 8 comments
Closed
10 tasks done

Surfacing GH Rate Limit Errors #1076

aj-codecov opened this issue Jan 22, 2024 · 8 comments
Assignees
Labels
epic this label is used to mark issues as epics in discovery The design, product, and specifications require refinement P1: will do priority 7-9
Milestone

Comments

@aj-codecov
Copy link

aj-codecov commented Jan 22, 2024

Problem to Solve
When users hit a rate limit error in GH they don't necessarily know that it is happening and their Codecov experience is degraded because of it - we need to surface the errors and possible solutions when this issue is encountered.

Proposed Solution
When Codecov encounters a GitHub rate limit error

  1. Update PR comment to show a message - "Unable to calculate coverage because GitHub rate limit has been exceeded. Please ensure the Codecov App (https://github.com/apps/codecov) is set up on this repo "
  2. On the UI --> Commits tab, show a message in the "Coverage" and "Patch" columns - "Unable to calculate coverage because GitHub rate limit has been exceeded. Please ensure the Codecov App (https://github.com/apps/codecov) is set up on this repo"
  3. On the pulls tab, show a message in the "coverage on Head", "patch" and "change from base" columns - "Unable to calculate coverage because GitHub rate limit has been exceeded. Please ensure the Codecov App (https://github.com/apps/codecov) is set up on this repo"

image
image

WIP: designs

Applications

Preview Give feedback
  1. rohitvinnakota-codecov
  2. rohitvinnakota-codecov
  3. rohitvinnakota-codecov
@github-project-automation github-project-automation bot moved this to Idea/Deprioritized for the quarter in Codecov's Roadmap Jan 22, 2024
@katia-sentry katia-sentry added this to the Q1'24 milestone Jan 31, 2024
@katia-sentry katia-sentry added the epic this label is used to mark issues as epics label Jan 31, 2024
@katia-sentry katia-sentry added the P0: must do priority 10 label Jan 31, 2024
@codecovdesign codecovdesign added the in discovery The design, product, and specifications require refinement label Feb 6, 2024
@codecovdesign
Copy link
Contributor

  • Are there any potential fixes users/orgs are able to do?
  • How about the checks in PR comments - how are these affected?

WIP: designs

@rohan-at-sentry
Copy link

rohan-at-sentry commented Feb 22, 2024

@codecovdesign here are the answers

The only path forward is to ensure orgs have the Github bot set and are uploading using the token. Technically they can retry, but the rate limit is typically set for an hour (worst case they have to wait 60 min) so it's probably not worth letting them know to retry.

With shelter rolling out gradually, there's a scenario where we would end up showing the comment after rate is available again for everyone affected. But that is probably medium term.

PR comments would almost never show up when rate limited today
Checks - can sometimes show up if the admin has set up Codecov check as required.

I think for a first pass, showing the message in the UI and check should be sufficient. (the design is a good approximation of how we want the experience to be)

@rohan-at-sentry
Copy link

rohan-at-sentry commented Feb 22, 2024

Eventually, with shelter available for 100% of orgs , I wonder if we will have an opportunity to say something like "We had trouble receiving this report due to GH rate limit, we will get it eventually"

@codecovdesign
Copy link
Contributor

codecovdesign commented Feb 29, 2024

related reported issue of ux, in scenario not logged in:

Is the “No Files covered…” expected here? I see the number 3 on “Files changed” and expected to see them below. I am on a rather slow internet connection at the moment (on a plane), so maybe that’s it?

no feedback appears to user and/or incorrect that "no files to show" vs likely rate limit issue
image (13)

@codecovdesign
Copy link
Contributor

review with @adrian-codecov 3/7

first principles

  • what are rate limits?
    • as it relates to GH, our system release on themfor a lot that we do. namely and importantly, when there is upload we're given commit id/hash, but not the whole story. when uploads begin processing 1) upload ingestion, 2) processing, 3) finalizing report aggregation (all in worker). 99% calling gh to fill in blank of pr author, pr upon an upload (to confirm); in this moment we there may be a limiting factor reported from GH.
      • there are a couple places where we detect the rate limit, but not a clear guideline we follow. that is to say any endpoint that calls GH, the first step would be there is a widespread detection. If we don't know that is happening, first step is to investigate these touch points and add trigger points to show the rate limit.
  • what causes rate limit
  • what triggers the rate limit events
    • pr comment vs app, how align messaging
    • how to trigger removal of rate limitation
  • what are the potential fixes for users
    • github app is necessary, for further investigation

action items

here are a couple places where we detect the rate limit, but not a clear guidline we follow. that is to say any endpoint that calls GH, the first step would be there is a widespread detection. If we don't know that is happening, first step is to investigate these touch points and add trigger points to show the rate limit.

further follow up on any loose ends around GitHub app requirements, are there alternatives to consider

something too look at is rest/graphql relationships

@codecovdesign
Copy link
Contributor

sync w/ @adrian-codecov @RulaKhaled

  • general overview - WIP investigation
  • scope of work to general rate limits in pr comment, but also there is syncing (could be later)
    • differentiation: this could happen in other place (such as upload area, app, repo page)
  • looking at alternative way of notifying system of rate errors, at the least the web app should be ok (unaffected by rate limit)
    • clarify: we know when rate limits are happening; notably when we are limited and looking to notify user in PR comment we have to send another request
    • implications here are that the pr comment may not show the error comment

@rohan-at-sentry rohan-at-sentry moved this from Idea/Deprioritized for the quarter to In Progress in Codecov's Roadmap Apr 22, 2024
@rohan-at-sentry rohan-at-sentry added the deprioritized Work is deprioritized for current quarter label Apr 29, 2024
@katia-sentry katia-sentry modified the milestones: Q1'24, Q2'24 Apr 30, 2024
@katia-sentry katia-sentry added P1: will do priority 7-9 and removed deprioritized Work is deprioritized for current quarter P0: must do priority 10 labels Apr 30, 2024
@Adal3n3
Copy link

Adal3n3 commented Jun 8, 2024

Making a note here: When codecov-comments bot hits a rate limit, instead of telling human go to the doc, we should tell them "Please ensure the Codecov App is set up on this repo" or "Please install Codecov App"

@codecovdesign
Copy link
Contributor

@codecovdesign todo: add repo rate limit scenario

@codecov-hooky codecov-hooky bot closed this as completed Sep 18, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Codecov's Roadmap Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic this label is used to mark issues as epics in discovery The design, product, and specifications require refinement P1: will do priority 7-9
Projects
Status: Done
Development

No branches or pull requests

8 participants