From bfe3355dc37b03617ba39ad8964878a3ed397f34 Mon Sep 17 00:00:00 2001
From: Mark Polyak <markpolyak@users.noreply.github.com>
Date: Tue, 24 Feb 2026 21:08:51 +0300
Subject: [PATCH] Add timeout and connection error handling to GitHubClient

Add DEFAULT_TIMEOUT=30s to all GitHub API requests to prevent
indefinite hangs on network issues. Wrap get_job_logs in
try/except RequestException so transient network errors return
None instead of propagating a 500 to the user.

Add tests for get_job_logs: success, HTTP 404, and ConnectionError cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 grading/github_client.py    | 16 +++++++++-----
 tests/test_github_client.py | 44 +++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/grading/github_client.py b/grading/github_client.py
index e668661..27d89f6 100644
--- a/grading/github_client.py
+++ b/grading/github_client.py
@@ -34,6 +34,7 @@ class GitHubClient:
     """Client for GitHub API operations."""
 
     BASE_URL = "https://api.github.com"
+    DEFAULT_TIMEOUT = 30
 
     def __init__(self, token: str):
         """
@@ -59,7 +60,7 @@ def user_exists(self, username: str) -> bool:
             True if user exists, False otherwise
         """
         url = f"{self.BASE_URL}/users/{username}"
-        resp = requests.get(url, headers=self.headers)
+        resp = requests.get(url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT)
         return resp.status_code == 200
 
     def file_exists(self, org: str, repo: str, path: str) -> bool:
@@ -75,7 +76,7 @@ def file_exists(self, org: str, repo: str, path: str) -> bool:
             True if file exists, False otherwise
         """
         url = f"{self.BASE_URL}/repos/{org}/{repo}/contents/{path}"
-        resp = requests.get(url, headers=self.headers)
+        resp = requests.get(url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT)
         return resp.status_code == 200
 
     def check_required_files(
@@ -127,7 +128,7 @@ def get_latest_commit(self, org: str, repo: str) -> CommitInfo | None:
         """
         # Get commits list
         commits_url = f"{self.BASE_URL}/repos/{org}/{repo}/commits"
-        commits_resp = requests.get(commits_url, headers=self.headers)
+        commits_resp = requests.get(commits_url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT)
 
         if commits_resp.status_code != 200:
             return None
@@ -140,7 +141,7 @@ def get_latest_commit(self, org: str, repo: str) -> CommitInfo | None:
 
         # Get commit details with files
         commit_url = f"{self.BASE_URL}/repos/{org}/{repo}/commits/{latest_sha}"
-        commit_resp = requests.get(commit_url, headers=self.headers)
+        commit_resp = requests.get(commit_url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT)
 
         if commit_resp.status_code != 200:
             return CommitInfo(sha=latest_sha, files=[])
@@ -169,7 +170,7 @@ def get_check_runs(
             List of check run dicts from GitHub API, or None on error
         """
         url = f"{self.BASE_URL}/repos/{org}/{repo}/commits/{commit_sha}/check-runs"
-        resp = requests.get(url, headers=self.headers)
+        resp = requests.get(url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT)
 
         if resp.status_code != 200:
             return None
@@ -189,7 +190,10 @@ def get_job_logs(self, org: str, repo: str, job_id: int) -> str | None:
             Log text or None if not available
         """
         url = f"{self.BASE_URL}/repos/{org}/{repo}/actions/jobs/{job_id}/logs"
-        resp = requests.get(url, headers=self.headers)
+        try:
+            resp = requests.get(url, headers=self.headers, timeout=self.DEFAULT_TIMEOUT)
+        except requests.exceptions.RequestException:
+            return None
 
         if resp.status_code != 200:
             return None
diff --git a/tests/test_github_client.py b/tests/test_github_client.py
index 3a184af..fcbc867 100644
--- a/tests/test_github_client.py
+++ b/tests/test_github_client.py
@@ -4,6 +4,7 @@
 Tests GitHub API client with mocked HTTP responses.
 """
 import pytest
+import requests
 import responses
 import sys
 import os
@@ -260,6 +261,49 @@ def test_empty_check_runs(self):
         assert runs == []
 
 
+class TestGitHubClientGetJobLogs:
+    """Tests for get_job_logs method."""
+
+    @responses.activate
+    def test_get_job_logs_success(self):
+        """Successfully fetched logs are returned as text."""
+        responses.add(
+            responses.GET,
+            "https://api.github.com/repos/org/repo/actions/jobs/12345/logs",
+            body="2024-01-01T00:00:00Z TASKID=5\nDone.",
+            status=200
+        )
+        client = GitHubClient("test_token")
+        logs = client.get_job_logs("org", "repo", 12345)
+        assert logs is not None
+        assert "TASKID=5" in logs
+
+    @responses.activate
+    def test_get_job_logs_api_error(self):
+        """HTTP error returns None."""
+        responses.add(
+            responses.GET,
+            "https://api.github.com/repos/org/repo/actions/jobs/12345/logs",
+            json={"message": "Not Found"},
+            status=404
+        )
+        client = GitHubClient("test_token")
+        logs = client.get_job_logs("org", "repo", 12345)
+        assert logs is None
+
+    @responses.activate
+    def test_get_job_logs_connection_error(self):
+        """Network error returns None instead of raising."""
+        responses.add(
+            responses.GET,
+            "https://api.github.com/repos/org/repo/actions/jobs/12345/logs",
+            body=requests.exceptions.ConnectionError("Remote end closed connection without response")
+        )
+        client = GitHubClient("test_token")
+        logs = client.get_job_logs("org", "repo", 12345)
+        assert logs is None
+
+
 class TestCheckForbiddenModifications:
     """Tests for check_forbidden_modifications function."""