fix: actions rate limit#164
Open
stay-foolish-forever wants to merge 6 commits into
Open
Conversation
…s workflow - Add version check after OCR installation to verify successful setup - Implement exponential backoff retry logic for GitHub API rate limits - Add delays between individual comment posts to avoid secondary rate limits - GitHub enforces ~80 content-generating requests per minute; spacing calls helps stay under that threshold with 2-second base delay and up to 3 retries
- Add `|| true` to ocr version check for error isolation - Narrow rate limit detection to 429 and 403 with rate-limit message matching, avoiding retries on permission/auth failures - Extract hardcoded delay constants into env-configurable variables (OCR_RETRY_BASE_DELAY, OCR_MAX_RETRIES, OCR_SUCCESS_DELAY, OCR_FAILURE_DELAY) with sensible defaults - Document optional environment variables in workflow header
- Add `ocr version || true` after install for diagnostic logging - Add `api_request_with_retry` function with exponential backoff for 429 and 403 (rate-limit message matching) errors - Respect GitLab `Retry-After` header when present - Extract delay constants into CI/CD-configurable variables (OCR_RETRY_BASE_DELAY, OCR_MAX_RETRIES, OCR_SUCCESS_DELAY, OCR_FAILURE_DELAY) with defaults - Add pacing delays between successful/failed discussion posts - Document optional CI/CD variables in pipeline header comments
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
+286
to
+292
| } else { | ||
| failedCount++; | ||
| failedComments.push({ comment, error: innerE.message }); | ||
| console.log(`Failed to post comment for ${reviewComment.path}: ${innerE.message}`); | ||
| // Still delay after a non-rate-limit failure to pace subsequent requests | ||
| await sleep(FAILURE_DELAY); | ||
| } |
There was a problem hiding this comment.
Bug: When rate-limit retries are exhausted (i.e., isRateLimit === true but attempt === MAX_RETRIES), execution falls into this else branch, which only applies FAILURE_DELAY (default 1000ms). After repeatedly hitting rate limits, a 1-second pause before the next comment's API call is likely insufficient and may perpetuate rate-limit failures.
Additionally, the comment "Still delay after a non-rate-limit failure" is misleading because this branch also handles exhausted rate-limit retries.
Consider distinguishing between the two failure cases: apply a longer delay (e.g., the last computed backoff delay or SUCCESS_DELAY) when rate-limit retries are exhausted, and keep the shorter FAILURE_DELAY only for genuine non-rate-limit errors.
Suggestion:
Suggested change
| } else { | |
| failedCount++; | |
| failedComments.push({ comment, error: innerE.message }); | |
| console.log(`Failed to post comment for ${reviewComment.path}: ${innerE.message}`); | |
| // Still delay after a non-rate-limit failure to pace subsequent requests | |
| await sleep(FAILURE_DELAY); | |
| } | |
| } else { | |
| failedCount++; | |
| failedComments.push({ comment, error: innerE.message }); | |
| console.log(`Failed to post comment for ${reviewComment.path}: ${innerE.message}`); | |
| // After exhausting rate-limit retries, use a longer delay; for other errors, use a shorter pace delay | |
| const postFailDelay = isRateLimit ? RETRY_BASE_DELAY * Math.pow(2, MAX_RETRIES) : FAILURE_DELAY; | |
| await sleep(postFailDelay); | |
| } |
- GitHub Actions: distinguish exhausted rate-limit retries from other errors, apply SUCCESS_DELAY (2s) instead of FAILURE_DELAY (1s) when retries exhausted - GitLab CI: return structured result from api_request_with_retry to differentiate failure types, apply context-aware delays based on rate-limit exhaustion status - Both: prevent perpetuating rate-limit failures by using longer delays after retry exhaustion
Contributor
Author
|
Will perform some further tests before it can be merged. |
|
Thanks for issuing this, hit the same issue today. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add rate-limit handling and version verification to both the GitHub Actions and GitLab CI example workflows. When posting many review comments via API, the original scripts could hit platform rate limits (GitHub's secondary rate limit of ~80 content-creating requests/min, and GitLab's general rate limit on self-hosted instances), causing comment posting failures with no recovery mechanism.
This PR introduces exponential backoff with retry logic, configurable delay constants, and a post-install version check for better observability.
Type of Change
How Has This Been Tested?
make testpasses locallyChecklist
go fmt,go vet)Related Issues
#158