Skip to content

fix(backend): add retry logic and backoff for secondary rate limits in commit_files #100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ankittroy-21 merged 2 commits into from
Jul 21, 2026
Merged

fix(backend): add retry logic and backoff for secondary rate limits in commit_files #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ddd6aa8
feat: implement retry logic for GitHub secondary rate limits and add …
faizahmad-khan Jul 21, 2026
a3c6b72
fix(backend): run synchronous commit_files via run_in_threadpool in c…
faizahmad-khan Jul 21, 2026
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
47 changes: 43 additions & 4 deletions backend/github_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,45 @@
"""

import base64
import time

from diff_service import apply_diff
from github import Github, GithubException


class GitHubService:

def _execute_with_retry(self, func, *args, **kwargs):
"""
Executes a GitHub API call with retry logic for Secondary Rate Limits (Abuse limits).
Catches 403 Forbidden caused by rapid sequential requests and respects Retry-After headers.
"""
max_retries = 3
for attempt in range(max_retries):
try:
return func(*args, **kwargs)
except GithubException as e:
is_rate_limit = False
if e.status == 403:
msg = str(e.data).lower() if e.data else ""
if "secondary rate" in msg or "abuse" in msg or "rate limit" in msg:
is_rate_limit = True

if is_rate_limit and attempt < max_retries - 1:
retry_after = 60
if hasattr(e, "headers") and e.headers and "retry-after" in e.headers:
try:
retry_after = int(e.headers["retry-after"])
except ValueError:
pass
print(
f"[github_service] Secondary rate limit hit. "
f"Retrying after {retry_after}s (attempt {attempt + 1}/{max_retries})..."
)
time.sleep(retry_after)
continue
raise

def validate_token_and_repo(self, token: str, repo_name: str) -> dict:
"""
Validates a PAT/OAuth token and checks access to the specified repo.
Expand Down Expand Up @@ -145,7 +177,11 @@ def commit_files(
conflict_files: list[str] = []
committed_ids: list[str] = []

for staged in staged_files:
for idx, staged in enumerate(staged_files):
if idx > 0:
# Proactively sleep 1s between file commits per GitHub's integrator best practices
time.sleep(1)

Comment on lines +180 to +184

@coderabbitai coderabbitai Bot Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

Blocking call inside an async route.

The addition of time.sleep(1) (and the potential 60s sleep in _execute_with_retry) will block the thread. Since commit_files is called synchronously from the async def commit_direct route (in backend/routes/staged_files.py), this will block the FastAPI event loop, preventing the server from handling other concurrent requests on this worker.

Consider wrapping this synchronous service invocation in a thread pool using fastapi.concurrency.run_in_threadpool from the route, or changing the route definition to a synchronous def so FastAPI automatically runs it in a thread pool.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/github_service.py` around lines 180 - 184, The async commit_direct
route synchronously invokes commit_files, whose time.sleep and retry delays
block the FastAPI event loop. Update commit_direct to run the synchronous
service invocation through fastapi.concurrency.run_in_threadpool, preserving the
existing commit_files behavior and route response handling.

filepath = staged["filepath"]
stored_base_sha = staged["base_sha"]
is_binary = staged.get("is_binary", False)
Expand All @@ -168,7 +204,8 @@ def commit_files(
if file_id:
committed_ids.append(file_id)
continue
result = repo.delete_file(
result = self._execute_with_retry(
repo.delete_file,
path=filepath,
message=commit_message,
sha=current_sha,
Expand Down Expand Up @@ -201,14 +238,16 @@ def commit_files(

# --- Commit to GitHub ---------------------------------------------------------
if not exists_on_gh:
result = repo.create_file(
result = self._execute_with_retry(
repo.create_file,
path=filepath,
message=commit_message,
content=content_to_commit,
branch=branch,
)
else:
result = repo.update_file(
result = self._execute_with_retry(
repo.update_file,
path=filepath,
message=commit_message,
content=content_to_commit,
Expand Down
4 changes: 3 additions & 1 deletion backend/routes/staged_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import channel_logger
from auth import require_api_key
from fastapi import APIRouter, Depends, HTTPException, Request
from fastapi.concurrency import run_in_threadpool
from github_service import github_service
from pydantic import BaseModel
from supabase_service import (
Expand Down Expand Up @@ -156,7 +157,8 @@ async def commit_direct(payload: DirectCommitPayload, telegram_id: str = Depends
detail="No repo detected. Save a file so GitPhone can auto-detect your repo.",
)

result = github_service.commit_files(
result = await run_in_threadpool(
github_service.commit_files,
token=user["github_token"],
repo_name=repo,
branch=branch,
Expand Down