Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 51 additions & 18 deletions backend/app/services/intelligence/github/api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,22 +90,11 @@ async def fetch_repos_raw(
)
if resp.status_code == 404:
raise GitHubUserNotFoundError(username)
# レート制限(403 + 残量 0 / 429)は共通ヘルパで RetryableError に変換する
_raise_if_rate_limited(resp)
if resp.status_code == 403:
# GitHub は rate limit でも 403 を返すため、ヘッダで判別する
if _is_rate_limited(resp):
retry_after = _retry_after_from_github(resp)
logger.warning(
"GitHub API rate limit hit (retry_after=%s)", retry_after,
)
raise RetryableError(
"GitHub API rate limit", retry_after=retry_after,
)
# レート制限でない 403 は権限エラー等の恒久障害
raise NonRetryableError(f"GitHub API 403 Forbidden: {resp.text[:200]}")
if resp.status_code == 429:
retry_after = _retry_after_from_github(resp)
raise RetryableError(
"GitHub API 429 Too Many Requests", retry_after=retry_after,
)
if resp.status_code in _RETRYABLE_STATUS_CODES:
raise RetryableError(f"GitHub API {resp.status_code}")
if 400 <= resp.status_code < 500:
Expand All @@ -121,9 +110,35 @@ async def fetch_repos_raw(


def _is_rate_limited(response: httpx.Response) -> bool:
"""GitHub のレスポンスがレート制限起因かを判定する。"""
remaining = response.headers.get("x-ratelimit-remaining")
return remaining == "0"
"""GitHub のレスポンスがレート制限起因かを判定する。

GitHub は API レート制限を超過すると **403** を返す(残量ヘッダで判別する)。
二次的な abuse 検出などで **429** を返すこともあり、こちらは残量に依らずレート制限扱い。
"""
if response.status_code == 429:
return True
if response.status_code == 403:
return response.headers.get("x-ratelimit-remaining") == "0"
return False


def _raise_if_rate_limited(response: httpx.Response) -> None:
"""レスポンスがレート制限(403 + 残量 0 / 429)なら ``RetryableError`` を raise する。

GitHub の全 fetch 経路で共通のレート制限ハンドリング。レート制限は特定リポの問題では
なくトークン単位のグローバルなスロットリングのため、``([], partial)`` として黙って
握り込むと後続リポも同様に空収集になり、証跡が静かに欠ける。``retry_after`` 付きで
raise し直し、リセット窓を待ってタスク全体を再試行させる(``fetch_repos_raw`` と同方針)。
レート制限でなければ何もしない(呼び出し側が他ステータスを処理する)。
"""
if _is_rate_limited(response):
retry_after = _retry_after_from_github(response)
logger.warning(
"GitHub API rate limit hit (status=%s, retry_after=%s)",
response.status_code,
retry_after,
)
raise RetryableError("GitHub API rate limit", retry_after=retry_after)


def _retry_after_from_github(response: httpx.Response) -> float | None:
Expand Down Expand Up @@ -155,8 +170,11 @@ async def fetch_languages(
return {}
try:
resp = await client.get(f"/repos/{owner}/{repo}/languages")
# レート制限は握り込まず RetryableError で連携全体をリトライさせる(#485)
_raise_if_rate_limited(resp)
if resp.status_code == 403:
logger.warning("Rate limit on languages for %s/%s", owner, repo)
# レート制限でない 403(ブロック等)は言語情報を欠いたまま best-effort 継続
logger.warning("Languages fetch forbidden for %s/%s", owner, repo)
return {}
resp.raise_for_status()
return resp.json()
Expand All @@ -183,6 +201,10 @@ async def fetch_repo_tree(
加え、tree 取得自体が失敗した場合(非200 / 不正レスポンス / ``httpx.HTTPError``)も
「依存ゼロ」と「走査不能」を区別するため ``True`` を返す(D9(d))。不正 owner/repo は
実在リポではなく走査対象ですらないため ``([], False)`` とする。

ただしレート制限(403 + 残量 0 / 429)は 1 リポの部分走査ではなくトークン単位の
グローバルなスロットリングのため、partial として握り込まず ``RetryableError`` を raise し、
リセット窓を待って連携タスク全体を再試行させる(#485。``fetch_repos_raw`` と同方針)。
"""
if not _is_valid_owner_repo(owner, repo):
return [], False
Expand All @@ -192,7 +214,16 @@ async def fetch_repo_tree(
f"/repos/{owner}/{repo}/git/trees/{branch}",
params={"recursive": "1"},
)
# レート制限は「1 リポの部分走査」ではなくトークン単位のスロットリングのため、
# partial として握り込まず RetryableError で連携全体をリトライさせる(#485)。
_raise_if_rate_limited(resp)
if resp.status_code != 200:
logger.warning(
"Git tree fetch returned %s for %s/%s (partial)",
resp.status_code,
owner,
repo,
)
return [], True
data = resp.json()
if not isinstance(data, dict):
Expand Down Expand Up @@ -228,6 +259,8 @@ async def fetch_repo_file(
f"/repos/{owner}/{repo}/contents/{path}",
headers={"Accept": "application/vnd.github.raw+json"},
)
# レート制限は握り込まず RetryableError で連携全体をリトライさせる(#485)
_raise_if_rate_limited(resp)
if resp.status_code != 200:
return None
return resp.text
Expand Down
86 changes: 85 additions & 1 deletion backend/tests/test_github_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@
from unittest.mock import AsyncMock, MagicMock

import httpx
from app.services.intelligence.github.api_client import fetch_repo_tree
import pytest
from app.services.intelligence.github.api_client import (
fetch_languages,
fetch_repo_file,
fetch_repo_tree,
)
from app.services.tasks.exceptions import RetryableError


def _run(coro):
Expand All @@ -23,12 +29,24 @@ def _run(coro):
def _client_with_tree(tree, truncated=False, status_code=200):
"""Trees API レスポンスを返す AsyncClient モックを生成する。"""
resp = MagicMock(status_code=status_code)
resp.headers = {}
resp.json = MagicMock(return_value={"tree": tree, "truncated": truncated})
client = MagicMock()
client.get = AsyncMock(return_value=resp)
return client


def _client_with_status(status_code, headers=None):
"""任意ステータス・ヘッダのレスポンスを返す AsyncClient モックを生成する。"""
resp = MagicMock(status_code=status_code)
resp.headers = headers or {}
resp.json = MagicMock(return_value={})
resp.text = ""
client = MagicMock()
client.get = AsyncMock(return_value=resp)
return client


def test_returns_all_blob_paths():
"""blob の相対パスをすべて返し、ディレクトリ(tree)は除外すること。"""
tree = [
Expand Down Expand Up @@ -74,3 +92,69 @@ def test_invalid_owner_repo_returns_empty():
result = _run(fetch_repo_tree(client, "../evil", "repo", "main"))
assert result == ([], False)
client.get.assert_not_called()


def test_rate_limited_403_raises_retryable():
"""レート制限の 403(X-RateLimit-Remaining:0)は partial 握り込みではなく
RetryableError を raise し、連携全体をリトライ経路へ乗せること(#485)。"""
client = _client_with_status(
403, headers={"x-ratelimit-remaining": "0", "retry-after": "42"}
)
with pytest.raises(RetryableError) as exc:
_run(fetch_repo_tree(client, "u", "repo", "main"))
assert exc.value.retry_after == 42


def test_429_raises_retryable():
"""429 Too Many Requests も RetryableError(retry_after 付き)で raise すること(#485)。"""
client = _client_with_status(429, headers={"retry-after": "30"})
with pytest.raises(RetryableError) as exc:
_run(fetch_repo_tree(client, "u", "repo", "main"))
assert exc.value.retry_after == 30


def test_genuine_403_returns_partial():
"""レート制限でない 403(残量あり = 権限エラー等)はリトライさせず
従来どおり partial 扱い([], True)で返すこと(#485)。"""
client = _client_with_status(403, headers={"x-ratelimit-remaining": "57"})
assert _run(fetch_repo_tree(client, "u", "repo", "main")) == ([], True)


def test_languages_rate_limited_raises_retryable():
"""fetch_languages もレート制限 403 を {} で握り込まず RetryableError を raise すること
(同一ホットパスの兄弟 fetch も #485 で統一)。"""
client = _client_with_status(403, headers={"x-ratelimit-remaining": "0"})
with pytest.raises(RetryableError):
_run(fetch_languages(client, "u", "repo"))


def test_languages_genuine_403_returns_empty():
"""レート制限でない 403 は言語情報を欠いたまま {} で best-effort 継続すること。"""
client = _client_with_status(403, headers={"x-ratelimit-remaining": "57"})
assert _run(fetch_languages(client, "u", "repo")) == {}


def test_repo_file_rate_limited_403_raises_retryable():
"""fetch_repo_file もレート制限 403 を None で握り込まず RetryableError を raise すること
(同一ホットパスの兄弟 fetch も #485 で統一)。"""
client = _client_with_status(
403, headers={"x-ratelimit-remaining": "0", "retry-after": "42"}
)
with pytest.raises(RetryableError) as exc:
_run(fetch_repo_file(client, "u", "repo", "requirements.txt"))
assert exc.value.retry_after == 42


def test_repo_file_429_raises_retryable():
"""fetch_repo_file の 429 も RetryableError(retry_after 付き)で raise すること(#485)。"""
client = _client_with_status(429, headers={"retry-after": "30"})
with pytest.raises(RetryableError) as exc:
_run(fetch_repo_file(client, "u", "repo", "requirements.txt"))
assert exc.value.retry_after == 30


def test_repo_file_genuine_403_returns_none():
"""レート制限でない 403(残量あり = 権限エラー等)はリトライさせず
従来どおり None(当該 manifest をスキップ)で best-effort 継続すること(#485)。"""
client = _client_with_status(403, headers={"x-ratelimit-remaining": "57"})
assert _run(fetch_repo_file(client, "u", "repo", "requirements.txt")) is None
Loading