* Retry transient network errors (RemoteDisconnected) in github_utils
`http.client.RemoteDisconnected` is a `ConnectionError` subclass, not a
`urllib.error.URLError`, so it was escaping both except clauses and
producing an unhandled traceback in CI (seen in run 31988833378).
Two changes:
- `_request`: catch `ConnectionError` and re-raise wrapped in `URLError`
so all network failures surface as a single consistent type.
- `github_request`: retry `URLError` with exponential backoff (1 s,
2 s, … capped at 60 s) instead of failing hard immediately, since
RemoteDisconnected / connection resets are almost always transient.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Retry transient network errors (RemoteDisconnected) in github_utils
`http.client.RemoteDisconnected` is a `ConnectionError` subclass, not a
`urllib.error.URLError`, so it was escaping both except clauses and
producing an unhandled traceback in CI (seen in run 31988833378).
Two changes:
- `_request`: catch `ConnectionError` and re-raise wrapped in `URLError`
so all network failures surface as a single consistent type.
- `github_request`: retry `URLError` with exponential backoff (1 s,
2 s, … capped at 60 s) instead of failing hard immediately, since
RemoteDisconnected / connection resets are almost always transient.
Add tests covering the new retry behaviour and the ConnectionError
normalization in `_request`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Clarify connection error test names and explain URLError vs ConnectionError
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Rename test and simplify comments per review
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix line length formatting
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Drop RequestTest class, fold tests into GithubRequestTest
Per review feedback: no need for a separate test class — the two new
tests for connection error retry behaviour sit naturally alongside the
existing GithubRequestTest cases.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Move _request normalization tests into GithubRequestTest
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
* use a single function for GH calls
* check token before loop
* Add pre-flight token/quota check and per-request rate-limit logging to github_utils
- Add _log_token_status(): calls /rate_limit once before the retry loop (no quota
consumed). Fails immediately if: CI env with no token, token rejected (401), or
quota already exhausted (remaining=0). 401 with vs. without token gives distinct
actionable error messages.
- Add _log_rate_limit_headers(): prints limit/used/remaining/reset on every request
attempt, labeled [initial] / [retry N/M]. Skipped on 401 (GitHub omits those
headers on auth failures).
- Remove _is_expired_or_bad_token(): pre-flight check catches bad tokens earlier;
401s in the retry loop fall through to the generic fail-hard with the raw body.
- Update tests: remove IsExpiredOrBadTokenTest (function deleted), add
LogTokenStatusTest covering all _log_token_status failure modes, patch
_log_token_status in GithubRequestTest.setUp so mock responses are not consumed
by the pre-flight call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix stale comments about get_github_json error handling
get_github_json now raises RuntimeError on error responses (rate limiting,
auth failures, etc.) rather than returning an error dict. Update comments
in notification_service.py and check_bad_commit.py that incorrectly described
the old behaviour, while keeping the defensive .get() calls which guard
against unexpected response shapes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* format
* fix
---------
Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
