From 5addbc51072a92a96e04bcdf62f6c2542b12cb45 Mon Sep 17 00:00:00 2001 From: "Terry.Mao" Date: Sat, 3 Oct 2026 23:22:58 +0800 Subject: [PATCH 1/2] =?UTF-8?q?fix(github-api):=20=E5=A2=9E=E5=8A=A0?= =?UTF-8?q?=E8=AF=B7=E6=B1=82=E8=B6=85=E6=97=B6=E5=92=8C=E9=87=8D=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../aicodingflow-tests/test_post_pr_review.py | 47 +++++++++++++++++++ .github/scripts/post_pr_review.py | 26 +++++++--- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/.github/aicodingflow-tests/test_post_pr_review.py b/.github/aicodingflow-tests/test_post_pr_review.py index e2ca9ac..052a35d 100644 --- a/.github/aicodingflow-tests/test_post_pr_review.py +++ b/.github/aicodingflow-tests/test_post_pr_review.py @@ -6,6 +6,7 @@ import os import tempfile import unittest +import urllib.error from pathlib import Path from unittest import mock @@ -16,6 +17,52 @@ class PostPrReviewTest(unittest.TestCase): + def test_github_api_response_retries_network_failures_with_timeout(self) -> None: + calls = [] + + class Response: + headers = {} + + def __enter__(self): + return self + + def __exit__(self, *args): + return False + + def read(self): + return b'{"id": 7}' + + def fake_urlopen(request, *, timeout): + calls.append((request.full_url, timeout)) + if len(calls) < 3: + raise urllib.error.URLError("temporary DNS failure") + return Response() + + with ( + mock.patch.object(post_pr_review.urllib.request, "urlopen", side_effect=fake_urlopen), + mock.patch.object(post_pr_review.time, "sleep") as sleep, + ): + response = post_pr_review.github_api_response("https://api.github.com/test", "token") + + self.assertEqual(response.data, {"id": 7}) + self.assertEqual(calls, [("https://api.github.com/test", 30)] * 3) + self.assertEqual([call.args[0] for call in sleep.call_args_list], [1, 2]) + + def test_github_api_response_stops_after_network_retry_limit(self) -> None: + with ( + mock.patch.object( + post_pr_review.urllib.request, + "urlopen", + side_effect=urllib.error.URLError("connection refused"), + ) as urlopen, + mock.patch.object(post_pr_review.time, "sleep") as sleep, + ): + with self.assertRaisesRegex(SystemExit, "after 3 attempts"): + post_pr_review.github_api_response("https://api.github.com/test", "token") + + self.assertEqual(urlopen.call_count, 3) + self.assertEqual([call.args[0] for call in sleep.call_args_list], [1, 2]) + def test_review_event_matrix_keeps_member_and_spec_reviews_as_comments(self) -> None: member_pr = {"author_association": "MEMBER", "user": {"login": "member", "type": "User"}} non_member_pr = {"author_association": "FIRST_TIMER", "user": {"login": "external", "type": "User"}} diff --git a/.github/scripts/post_pr_review.py b/.github/scripts/post_pr_review.py index 3360e71..9c63e5a 100644 --- a/.github/scripts/post_pr_review.py +++ b/.github/scripts/post_pr_review.py @@ -7,6 +7,7 @@ import json import os import re +import time import urllib.error import urllib.request from pathlib import Path @@ -19,6 +20,9 @@ ORG_MEMBER_ASSOCIATIONS = {"COLLABORATOR", "MEMBER", "OWNER"} NON_MEMBER_ASSOCIATIONS = {"CONTRIBUTOR", "FIRST_TIMER", "FIRST_TIME_CONTRIBUTOR", "NONE"} DEFAULT_REVIEW_BOT_LOGIN = "github-actions[bot]" +GITHUB_API_TIMEOUT_SECONDS = 30 +GITHUB_API_MAX_ATTEMPTS = 3 +GITHUB_API_BACKOFF_SECONDS = 1 class CodeownersRule(NamedTuple): @@ -67,13 +71,21 @@ def github_api_response( }, method=method, ) - try: - with urllib.request.urlopen(request) as response: - body = response.read().decode("utf-8") - return GitHubResponse(json.loads(body) if body else {}, response.headers) - except urllib.error.HTTPError as exc: - detail = exc.read().decode("utf-8", errors="replace") - raise SystemExit(f"GitHub API request failed: {exc.code} {detail}") from exc + for attempt in range(1, GITHUB_API_MAX_ATTEMPTS + 1): + try: + with urllib.request.urlopen(request, timeout=GITHUB_API_TIMEOUT_SECONDS) as response: + body = response.read().decode("utf-8") + return GitHubResponse(json.loads(body) if body else {}, response.headers) + except urllib.error.HTTPError as exc: + detail = exc.read().decode("utf-8", errors="replace") + raise SystemExit(f"GitHub API request failed: {exc.code} {detail}") from exc + except (urllib.error.URLError, TimeoutError) as exc: + if attempt == GITHUB_API_MAX_ATTEMPTS: + raise SystemExit( + "GitHub API request failed after " + f"{GITHUB_API_MAX_ATTEMPTS} attempts: {exc}" + ) from exc + time.sleep(GITHUB_API_BACKOFF_SECONDS * (2 ** (attempt - 1))) def request_json(url: str, token: str, payload: dict[str, Any]) -> dict[str, Any]: From 89d0d5707c262a49d1e9af85a2b08ecd37985a76 Mon Sep 17 00:00:00 2001 From: "Terry.Mao" Date: Sun, 4 Oct 2026 00:04:13 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix(github-api):=20=E9=81=BF=E5=85=8D?= =?UTF-8?q?=E9=9D=9E=E5=B9=82=E7=AD=89=E8=AF=B7=E6=B1=82=E9=87=8D=E5=A4=8D?= =?UTF-8?q?=E9=87=8D=E8=AF=95?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../aicodingflow-tests/test_post_pr_review.py | 20 +++++++++++++++++++ .github/scripts/post_pr_review.py | 9 ++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/.github/aicodingflow-tests/test_post_pr_review.py b/.github/aicodingflow-tests/test_post_pr_review.py index 052a35d..582653f 100644 --- a/.github/aicodingflow-tests/test_post_pr_review.py +++ b/.github/aicodingflow-tests/test_post_pr_review.py @@ -63,6 +63,26 @@ def test_github_api_response_stops_after_network_retry_limit(self) -> None: self.assertEqual(urlopen.call_count, 3) self.assertEqual([call.args[0] for call in sleep.call_args_list], [1, 2]) + def test_github_api_response_does_not_retry_post_requests(self) -> None: + with ( + mock.patch.object( + post_pr_review.urllib.request, + "urlopen", + side_effect=urllib.error.URLError("response read timeout"), + ) as urlopen, + mock.patch.object(post_pr_review.time, "sleep") as sleep, + ): + with self.assertRaisesRegex(SystemExit, "after 1 attempt"): + post_pr_review.github_api_response( + "https://api.github.com/repos/owner/repo/pulls/1/reviews", + "token", + method="POST", + payload={"body": "review"}, + ) + + urlopen.assert_called_once() + sleep.assert_not_called() + def test_review_event_matrix_keeps_member_and_spec_reviews_as_comments(self) -> None: member_pr = {"author_association": "MEMBER", "user": {"login": "member", "type": "User"}} non_member_pr = {"author_association": "FIRST_TIMER", "user": {"login": "external", "type": "User"}} diff --git a/.github/scripts/post_pr_review.py b/.github/scripts/post_pr_review.py index 9c63e5a..4f78c60 100644 --- a/.github/scripts/post_pr_review.py +++ b/.github/scripts/post_pr_review.py @@ -23,6 +23,7 @@ GITHUB_API_TIMEOUT_SECONDS = 30 GITHUB_API_MAX_ATTEMPTS = 3 GITHUB_API_BACKOFF_SECONDS = 1 +GITHUB_API_RETRYABLE_METHODS = {"DELETE", "GET", "HEAD", "OPTIONS", "PUT"} class CodeownersRule(NamedTuple): @@ -71,7 +72,8 @@ def github_api_response( }, method=method, ) - for attempt in range(1, GITHUB_API_MAX_ATTEMPTS + 1): + max_attempts = GITHUB_API_MAX_ATTEMPTS if method.upper() in GITHUB_API_RETRYABLE_METHODS else 1 + for attempt in range(1, max_attempts + 1): try: with urllib.request.urlopen(request, timeout=GITHUB_API_TIMEOUT_SECONDS) as response: body = response.read().decode("utf-8") @@ -80,10 +82,11 @@ def github_api_response( detail = exc.read().decode("utf-8", errors="replace") raise SystemExit(f"GitHub API request failed: {exc.code} {detail}") from exc except (urllib.error.URLError, TimeoutError) as exc: - if attempt == GITHUB_API_MAX_ATTEMPTS: + if attempt == max_attempts: + attempt_label = "attempt" if max_attempts == 1 else "attempts" raise SystemExit( "GitHub API request failed after " - f"{GITHUB_API_MAX_ATTEMPTS} attempts: {exc}" + f"{max_attempts} {attempt_label}: {exc}" ) from exc time.sleep(GITHUB_API_BACKOFF_SECONDS * (2 ** (attempt - 1)))