Public Access
fix: add retry logic to TeaCLI for transient HTTP errors (502/503/504/429)
This commit is contained in:
+58
-20
@@ -40,21 +40,35 @@ Usage::
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
import logging
|
||||||
import shutil
|
import shutil
|
||||||
import subprocess # nosec B404
|
import subprocess # nosec B404
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import click
|
import click
|
||||||
|
from tenacity import (
|
||||||
|
before_sleep_log,
|
||||||
|
retry,
|
||||||
|
retry_if_exception_type,
|
||||||
|
stop_after_attempt,
|
||||||
|
wait_exponential,
|
||||||
|
)
|
||||||
|
|
||||||
from devx.config import GITEA_API_URL
|
from devx.config import GITEA_API_URL, MAX_RETRIES, RETRY_BACKOFF_BASE, RETRY_STATUS_CODES
|
||||||
from devx.i18n import _
|
from devx.i18n import _
|
||||||
from devx.tokens import get_ci_token
|
from devx.tokens import get_ci_token
|
||||||
|
|
||||||
|
logger = logging.getLogger("gitea_cli")
|
||||||
|
|
||||||
|
|
||||||
class TeaCLIError(Exception):
|
class TeaCLIError(Exception):
|
||||||
"""Raised when a tea CLI command fails."""
|
"""Raised when a tea CLI command fails."""
|
||||||
|
|
||||||
|
|
||||||
|
class _TransientTeaError(TeaCLIError):
|
||||||
|
"""Tea CLI error caused by a transient HTTP status (502/503/504/429)."""
|
||||||
|
|
||||||
|
|
||||||
def configure_tea_login(login_name: str = "devx") -> None:
|
def configure_tea_login(login_name: str = "devx") -> None:
|
||||||
"""Configure tea CLI login from CI_GITEA_API_TOKEN and DEVX_GITEA_API_URL.
|
"""Configure tea CLI login from CI_GITEA_API_TOKEN and DEVX_GITEA_API_URL.
|
||||||
|
|
||||||
@@ -140,6 +154,10 @@ class TeaCLI:
|
|||||||
def _run(self, args: list[str], json_output: bool = True) -> str:
|
def _run(self, args: list[str], json_output: bool = True) -> str:
|
||||||
"""Run a tea command and return stdout.
|
"""Run a tea command and return stdout.
|
||||||
|
|
||||||
|
Retries up to ``MAX_RETRIES`` times on transient HTTP errors
|
||||||
|
(502/503/504/429) detected in stderr/stdout, with exponential
|
||||||
|
backoff. Non-transient errors fail immediately.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
args: Command arguments (without the leading ``tea``).
|
args: Command arguments (without the leading ``tea``).
|
||||||
json_output: If True, append ``--output json`` to the command.
|
json_output: If True, append ``--output json`` to the command.
|
||||||
@@ -148,30 +166,50 @@ class TeaCLI:
|
|||||||
stdout as a string.
|
stdout as a string.
|
||||||
|
|
||||||
Raises:
|
Raises:
|
||||||
TeaCLIError: If the command fails.
|
TeaCLIError: If the command fails after retries are exhausted.
|
||||||
"""
|
"""
|
||||||
cmd = [self._tea, *args]
|
cmd = [self._tea, *args]
|
||||||
if json_output:
|
if json_output:
|
||||||
cmd.extend(["--output", "json"])
|
cmd.extend(["--output", "json"])
|
||||||
|
|
||||||
|
def _execute() -> str:
|
||||||
|
try:
|
||||||
|
result = subprocess.run( # nosec B603
|
||||||
|
cmd,
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
except FileNotFoundError as e:
|
||||||
|
raise TeaCLIError(f"tea binary not found ('{self._tea}'). Install tea or add it to PATH.") from e
|
||||||
|
if result.returncode != 0:
|
||||||
|
parts = [
|
||||||
|
f"tea command failed (rc={result.returncode}): {' '.join(args)}",
|
||||||
|
f"stdout: {result.stdout.strip()}" if result.stdout.strip() else "",
|
||||||
|
f"stderr: {result.stderr.strip()}" if result.stderr.strip() else "",
|
||||||
|
]
|
||||||
|
msg = "\n".join(p for p in parts if p)
|
||||||
|
combined = f"{result.stdout} {result.stderr}".lower()
|
||||||
|
if any(str(code) in combined for code in RETRY_STATUS_CODES):
|
||||||
|
raise _TransientTeaError(msg)
|
||||||
|
raise TeaCLIError(msg)
|
||||||
|
return result.stdout.strip()
|
||||||
|
|
||||||
|
retry_decorator = retry(
|
||||||
|
stop=stop_after_attempt(MAX_RETRIES),
|
||||||
|
wait=wait_exponential(
|
||||||
|
multiplier=RETRY_BACKOFF_BASE,
|
||||||
|
min=RETRY_BACKOFF_BASE,
|
||||||
|
max=RETRY_BACKOFF_BASE**MAX_RETRIES,
|
||||||
|
),
|
||||||
|
retry=retry_if_exception_type(_TransientTeaError),
|
||||||
|
before_sleep=before_sleep_log(logger, logging.WARNING),
|
||||||
|
reraise=True,
|
||||||
|
)
|
||||||
try:
|
try:
|
||||||
result = subprocess.run( # nosec B603
|
return retry_decorator(_execute)()
|
||||||
cmd,
|
except _TransientTeaError as e:
|
||||||
capture_output=True,
|
raise TeaCLIError(str(e)) from e
|
||||||
text=True,
|
|
||||||
check=False,
|
|
||||||
)
|
|
||||||
except FileNotFoundError as e:
|
|
||||||
raise TeaCLIError(f"tea binary not found ('{self._tea}'). Install tea or add it to PATH.") from e
|
|
||||||
if result.returncode != 0:
|
|
||||||
# tea writes some errors to stdout (for example, "no available
|
|
||||||
# login"), so include both stdout and stderr for debugging.
|
|
||||||
parts = [
|
|
||||||
f"tea command failed (rc={result.returncode}): {' '.join(args)}",
|
|
||||||
f"stdout: {result.stdout.strip()}" if result.stdout.strip() else "",
|
|
||||||
f"stderr: {result.stderr.strip()}" if result.stderr.strip() else "",
|
|
||||||
]
|
|
||||||
raise TeaCLIError("\n".join(p for p in parts if p))
|
|
||||||
return result.stdout.strip()
|
|
||||||
|
|
||||||
def _run_raw(self, args: list[str]) -> str:
|
def _run_raw(self, args: list[str]) -> str:
|
||||||
"""Run a tea command without JSON output and return stdout."""
|
"""Run a tea command without JSON output and return stdout."""
|
||||||
|
|||||||
@@ -7,7 +7,13 @@ from unittest.mock import MagicMock, patch
|
|||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from devx.gitea_cli import TeaCLI, TeaCLIError, _extract_issue_number, _extract_pr_number, configure_tea_login
|
from devx.gitea_cli import (
|
||||||
|
TeaCLI,
|
||||||
|
TeaCLIError,
|
||||||
|
_extract_issue_number,
|
||||||
|
_extract_pr_number,
|
||||||
|
configure_tea_login,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class TestExtractIssueNumber:
|
class TestExtractIssueNumber:
|
||||||
@@ -119,6 +125,46 @@ class TestTeaCLIRun:
|
|||||||
cmd = mock_run.call_args[0][0]
|
cmd = mock_run.call_args[0][0]
|
||||||
assert "--output" not in cmd
|
assert "--output" not in cmd
|
||||||
|
|
||||||
|
def test_run_retries_on_502(self) -> None:
|
||||||
|
"""Transient 502 errors should be retried, then succeed."""
|
||||||
|
cli = TeaCLI(tea_bin="/fake/tea")
|
||||||
|
fail_result = MagicMock(returncode=1, stdout="", stderr="502 Bad Gateway")
|
||||||
|
success_result = MagicMock(returncode=0, stdout='[{"id": 1}]', stderr="")
|
||||||
|
with patch("subprocess.run", side_effect=[fail_result, success_result]) as mock_run:
|
||||||
|
with patch("tenacity.nap.time.sleep"):
|
||||||
|
output = cli._run(["labels", "list"])
|
||||||
|
assert output == '[{"id": 1}]'
|
||||||
|
assert mock_run.call_count == 2
|
||||||
|
|
||||||
|
def test_run_retries_on_503_then_fails(self) -> None:
|
||||||
|
"""If all retries are exhausted on 503, raise TeaCLIError."""
|
||||||
|
cli = TeaCLI(tea_bin="/fake/tea")
|
||||||
|
fail_result = MagicMock(returncode=1, stdout="", stderr="503 Service Unavailable")
|
||||||
|
with patch("subprocess.run", return_value=fail_result):
|
||||||
|
with patch("tenacity.nap.time.sleep"):
|
||||||
|
with pytest.raises(TeaCLIError, match="503"):
|
||||||
|
cli._run(["issues", "create"])
|
||||||
|
# MAX_RETRIES=3, so 3 attempts total
|
||||||
|
|
||||||
|
def test_run_no_retry_on_non_transient_error(self) -> None:
|
||||||
|
"""Non-transient errors (e.g. auth) should fail immediately without retry."""
|
||||||
|
cli = TeaCLI(tea_bin="/fake/tea")
|
||||||
|
fail_result = MagicMock(returncode=1, stdout="", stderr="auth error")
|
||||||
|
with patch("subprocess.run", return_value=fail_result) as mock_run:
|
||||||
|
with pytest.raises(TeaCLIError, match="auth error"):
|
||||||
|
cli._run(["labels", "list"])
|
||||||
|
assert mock_run.call_count == 1
|
||||||
|
|
||||||
|
def test_run_retries_on_429_in_stdout(self) -> None:
|
||||||
|
"""429 rate limit in stdout should trigger retry."""
|
||||||
|
cli = TeaCLI(tea_bin="/fake/tea")
|
||||||
|
fail_result = MagicMock(returncode=1, stdout="429 Too Many Requests", stderr="")
|
||||||
|
success_result = MagicMock(returncode=0, stdout="ok", stderr="")
|
||||||
|
with patch("subprocess.run", side_effect=[fail_result, success_result]):
|
||||||
|
with patch("tenacity.nap.time.sleep"):
|
||||||
|
output = cli._run(["releases", "create"])
|
||||||
|
assert output == "ok"
|
||||||
|
|
||||||
|
|
||||||
class TestRepoArg:
|
class TestRepoArg:
|
||||||
def test_with_repo_arg(self) -> None:
|
def test_with_repo_arg(self) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user