Repository navigation
Conversation
_retry_with_backoff compared the error code to the string '403', but the Calendar API returns it as a number, so rate-limited requests were never retried. Compare the number, also accept 429 (the same rateLimitExceeded reason per the Calendar API error guide), and re-raise on the last attempt so callers get the error instead of None. Fixes insanum#289.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Automatic Review: Kody reviews this PR automatically on every push. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
Business Rules ValidationTask: 211940595 - RetryWithBackoff Status: Compliant Note: No structured acceptance criteria were provided. Validation is based on the issue description in FULL_TASK_CONTEXT. FindingsINFO: Reported code-type mismatch is correctedRequirement: "you check against '403' (string) while you should check to 403 (int) I believe" (issue description) INFO: Last-attempt behavior is made explicitRequirement: "I couldn't get upload an ics file due to rateLimitExceeded. Even though you are using backing off. It appears though, it doesn't work in your program" (issue description) Requirements Verified
Analysis performed by Kodus AI Business Rules Validator
|
A 403 or 429 body with no 'errors' list made _retry_with_backoff raise TypeError instead of HttpError. Default the missing list so such errors are re-raised unchanged, and test a 429 RESOURCE_EXHAUSTED body.
| if error.get('code') in (403, 429) and \ | ||
| (error.get('errors') or [{}])[0].get('reason') \ | ||
| in ['rateLimitExceeded', 'userRateLimitExceeded'] and \ |
There was a problem hiding this comment.
Revises an earlier Kody suggestion on gcalcli/gcal.py:145 (2026-10-17 15:42 UTC).
The retry decision depends on the legacy errors[0].reason field, so a rate-limit response carrying its reason in the newer status/details form (e.g. 429 with "status": "RESOURCE_EXHAUSTED" and no errors list) is re-raised on the first attempt, causing gcalcli to fail imports with "Failed to import event" (gcal.py:1674-1698) and the test at tests/test_gcalcli.py:462-473 to enshrine that behavior. Treat the HTTP status or status field as an additional rate-limit signal: OR (error.get('errors') or [{}])[0].get('reason') in [...] with error.get('status') == 'RESOURCE_EXHAUSTED' (or e.resp.status == 429), and scan all errors entries instead of only index 0.
Prompt for LLM
File gcalcli/gcal.py:
Line 146 to 148:
**Revises an earlier Kody suggestion** on `gcalcli/gcal.py:145` (2026-10-17 15:42 UTC).
The retry decision depends on the legacy `errors[0].reason` field, so a rate-limit response carrying its reason in the newer `status`/`details` form (e.g. 429 with `"status": "RESOURCE_EXHAUSTED"` and no `errors` list) is re-raised on the first attempt, causing gcalcli to fail imports with "Failed to import event" (gcal.py:1674-1698) and the test at tests/test_gcalcli.py:462-473 to enshrine that behavior. Treat the HTTP status or `status` field as an additional rate-limit signal: OR `(error.get('errors') or [{}])[0].get('reason') in [...]` with `error.get('status') == 'RESOURCE_EXHAUSTED'` (or `e.resp.status == 429`), and scan all `errors` entries instead of only index 0.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Fixes #289.
_retry_with_backoffcompares the error code to the string'403', but the Calendar API returns it as a number ("code": 403), so the check never matches and a rate-limited request fails on its first attempt. This shows up when importing more than a few dozen events: some of them fail with "Rate Limit Exceeded" (also reported in #833).Changes:
rateLimitExceededreason (Calendar API error guide).return None. That path could not be reached before; with retries working,ImportICSwould call.get()onNonewhen a request was still rate-limited aftermax_retriesattempts.Testing:
pytest68 passed andruff checkis clean; the four new rate-limit tests fail on main. On a real calendar, importing the same 39 already-present events twice, 4.5.1 lost 4 and 12 events to rate limiting and the patched version lost 0 and 0.Summary
Fixes
_retry_with_backoffso that it actually retries rate-limit errors instead of falling through to an immediate re-raise.Problem
The retry helper compared the API error
codeagainst the string'403', while the Calendar API returns the code as a number. As a result, the rate-limit branch was never taken and rate-limited requests were raised immediately rather than being retried with backoff. The helper also unconditionally indexed intoerror['errors'], which could fail on error bodies that do not include anerrorslist.Changes
gcalcli/gcal.py403and429, so rate-limit detection works with the actual API responses.403and429responses, matching the reasonsrateLimitExceededanduserRateLimitExceeded.HttpErrorrather than a silentNone.errorslist is now null-safe, so error bodies without anerrorslist are re-raised unchanged instead of causing an indexing failure.tests/test_gcalcli.pyHttpErrorfactory and aFailingRequesttest double that raises a configurable number of times before succeeding.403/429with the rate-limit reasons), re-raising after the last allowed attempt, not retrying unrelated errors (e.g.409 duplicate), and raising immediately for responses that have noerrorslist.Known limitation (from review)
Rate-limit responses that omit the
errorslist (for example429withstatus: RESOURCE_EXHAUSTED) are still not retried and are re-raised immediately. This behavior is currently asserted by a test, so the fix does not yet cover the newer Google error format for those cases.