fix(update): call out GitHub rate limiting/outage on fetch 429 instead of a generic failure
A GitHub-side HTTP 429 during 'hermes update' printed only 'Failed to fetch updates from origin.' — and the curl 'unable to access ... returned error: 429' shape even matched the network-error branch, blaming the user's connection for a GitHub outage. - new _classify_fetch_failure(): 429/rate-limit -> 'GitHub is rate limiting requests or having an outage — try again in 5 minutes'; 5xx -> outage message with githubstatus.com; ordered BEFORE the generic 'unable to access' network check - both fetch-failure sites (update apply + --check) now share the classifier via _print_fetch_failure(), and both always print the first raw stderr line so the wire error stays diagnosable - tests: classifier matrix + E2E against a live local HTTP server returning 429 through real git Fixes #89287
This commit is contained in:
Teknium
+45
-23
@@ -2714,6 +2714,49 @@ def _run_logged_subprocess(cmd, *, cwd=None, env=None):
|
||||
_log_only_write(result.stdout or "")
|
||||
return result
|
||||
|
||||
def _classify_fetch_failure(stderr: str) -> str:
|
||||
"""Map git-fetch stderr to a one-line, user-facing diagnosis.
|
||||
|
||||
Order matters: curl surfaces HTTP failures as
|
||||
``fatal: unable to access '<url>': The requested URL returned error: 429``,
|
||||
so the rate-limit/outage checks must run BEFORE the generic
|
||||
"unable to access" network check or a GitHub 429/5xx gets misreported as a
|
||||
local network problem. The caller always prints the first raw stderr line
|
||||
alongside this diagnosis — the friendly message adds guidance, it never
|
||||
replaces the wire error.
|
||||
"""
|
||||
|
||||
def _has_http_code(*codes: str) -> bool:
|
||||
return any(
|
||||
f"HTTP {code}" in stderr or f"returned error: {code}" in stderr
|
||||
for code in codes
|
||||
)
|
||||
|
||||
if _has_http_code("429") or "rate limit" in stderr.lower():
|
||||
return (
|
||||
"✗ GitHub is rate limiting requests or having an outage (HTTP 429)"
|
||||
" — try again in 5 minutes."
|
||||
)
|
||||
if _has_http_code("500", "502", "503", "504"):
|
||||
return (
|
||||
"✗ GitHub appears to be having an outage — try again in a few"
|
||||
" minutes (https://www.githubstatus.com)."
|
||||
)
|
||||
if "Could not resolve host" in stderr or "unable to access" in stderr:
|
||||
return "✗ Network error — cannot reach the remote repository."
|
||||
if "Authentication failed" in stderr or "could not read Username" in stderr:
|
||||
return "✗ Authentication failed — check your git credentials or SSH key."
|
||||
return "✗ Failed to fetch updates from origin."
|
||||
|
||||
|
||||
def _print_fetch_failure(stderr: str) -> None:
|
||||
"""Print the classified diagnosis plus the first raw stderr line."""
|
||||
stderr = (stderr or "").strip()
|
||||
print(_classify_fetch_failure(stderr))
|
||||
if stderr:
|
||||
print(f" {stderr.splitlines()[0]}")
|
||||
|
||||
|
||||
def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False):
|
||||
"""Implement ``hermes update --check``: fetch and report without installing.
|
||||
|
||||
@@ -2835,15 +2878,7 @@ def _cmd_update_check(branch: str = "main", *, branch_explicit: bool = False):
|
||||
compare_branch = f"origin/{branch}"
|
||||
|
||||
if fetch_result.returncode != 0:
|
||||
stderr = fetch_result.stderr.strip()
|
||||
if "Could not resolve host" in stderr or "unable to access" in stderr:
|
||||
print("✗ Network error — cannot reach the remote repository.")
|
||||
elif "Authentication failed" in stderr or "could not read Username" in stderr:
|
||||
print("✗ Authentication failed — check your git credentials or SSH key.")
|
||||
else:
|
||||
print("✗ Failed to fetch.")
|
||||
if stderr:
|
||||
print(f" {stderr.splitlines()[0]}")
|
||||
_print_fetch_failure(fetch_result.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
# Verify the compare ref actually exists before asking rev-list about it.
|
||||
@@ -4896,20 +4931,7 @@ def _cmd_update_impl(args, gateway_mode: bool):
|
||||
text=True, encoding="utf-8", errors="replace",
|
||||
)
|
||||
if fetch_result.returncode != 0:
|
||||
stderr = fetch_result.stderr.strip()
|
||||
if "Could not resolve host" in stderr or "unable to access" in stderr:
|
||||
print("✗ Network error — cannot reach the remote repository.")
|
||||
print(f" {stderr.splitlines()[0]}" if stderr else "")
|
||||
elif (
|
||||
"Authentication failed" in stderr or "could not read Username" in stderr
|
||||
):
|
||||
print(
|
||||
"✗ Authentication failed — check your git credentials or SSH key."
|
||||
)
|
||||
else:
|
||||
print("✗ Failed to fetch updates from origin.")
|
||||
if stderr:
|
||||
print(f" {stderr.splitlines()[0]}")
|
||||
_print_fetch_failure(fetch_result.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
# Get current branch (returns literal "HEAD" when detached)
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
"""Fetch-failure classification for `hermes update` / `hermes update --check`.
|
||||
|
||||
A GitHub-side HTTP 429 (rate limit / outage) used to be reported as the
|
||||
generic "Failed to fetch updates from origin." — or worse, matched the
|
||||
"unable to access" branch and got called a local network error. The
|
||||
classifier must call out rate limiting / outages explicitly, and the raw
|
||||
stderr line must always be printed alongside the diagnosis.
|
||||
"""
|
||||
|
||||
from hermes_cli import update_cmd
|
||||
|
||||
|
||||
RATE_LIMIT_STDERR = (
|
||||
"error: RPC failed; HTTP 429 curl 22 The requested URL returned error: 429\n"
|
||||
"fatal: expected flush after ref listing"
|
||||
)
|
||||
CURL_429_STDERR = (
|
||||
"fatal: unable to access 'https://github.com/NousResearch/hermes-agent.git/':"
|
||||
" The requested URL returned error: 429"
|
||||
)
|
||||
|
||||
|
||||
class TestClassifyFetchFailure:
|
||||
def test_http_429_rpc_failure_reports_rate_limit(self):
|
||||
msg = update_cmd._classify_fetch_failure(RATE_LIMIT_STDERR)
|
||||
assert "rate limiting" in msg
|
||||
assert "try again in 5 minutes" in msg
|
||||
|
||||
def test_curl_unable_to_access_429_is_rate_limit_not_network(self):
|
||||
# "unable to access" also appears here — 429 must win.
|
||||
msg = update_cmd._classify_fetch_failure(CURL_429_STDERR)
|
||||
assert "rate limiting" in msg
|
||||
assert "Network error" not in msg
|
||||
|
||||
def test_rate_limit_phrase_without_code(self):
|
||||
msg = update_cmd._classify_fetch_failure("fatal: GitHub rate limit exceeded")
|
||||
assert "rate limiting" in msg
|
||||
|
||||
def test_5xx_reports_outage(self):
|
||||
msg = update_cmd._classify_fetch_failure(
|
||||
"fatal: unable to access 'https://github.com/x.git/':"
|
||||
" The requested URL returned error: 503"
|
||||
)
|
||||
assert "outage" in msg
|
||||
assert "githubstatus.com" in msg
|
||||
|
||||
def test_dns_failure_reports_network_error(self):
|
||||
msg = update_cmd._classify_fetch_failure(
|
||||
"fatal: unable to access 'https://github.com/x.git/':"
|
||||
" Could not resolve host: github.com"
|
||||
)
|
||||
assert msg.startswith("✗ Network error")
|
||||
|
||||
def test_auth_failure(self):
|
||||
msg = update_cmd._classify_fetch_failure(
|
||||
"fatal: Authentication failed for 'https://github.com/x.git/'"
|
||||
)
|
||||
assert "Authentication failed" in msg
|
||||
|
||||
def test_unknown_falls_back_to_generic(self):
|
||||
msg = update_cmd._classify_fetch_failure("fatal: something novel")
|
||||
assert msg == "✗ Failed to fetch updates from origin."
|
||||
|
||||
|
||||
class TestPrintFetchFailure:
|
||||
def test_prints_diagnosis_and_first_raw_line(self, capsys):
|
||||
update_cmd._print_fetch_failure(RATE_LIMIT_STDERR)
|
||||
out = capsys.readouterr().out
|
||||
assert "rate limiting" in out
|
||||
assert "HTTP 429" in out
|
||||
# raw first stderr line preserved for diagnosability
|
||||
assert "error: RPC failed" in out
|
||||
|
||||
def test_empty_stderr_prints_only_diagnosis(self, capsys):
|
||||
update_cmd._print_fetch_failure("")
|
||||
out = capsys.readouterr().out.strip().splitlines()
|
||||
assert out == ["✗ Failed to fetch updates from origin."]
|
||||
Reference in New Issue
Block a user