Compare commits
18 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| dd003c66d5 | |||
| 019b815280 | |||
| c27dfd0f08 | |||
| 1b6c37605f | |||
| 036e96d9b7 | |||
| ea74f7e088 | |||
| e6b1840ffc | |||
| 1ca9250e4a | |||
| 1b38e6ad00 | |||
| 5498dccd60 | |||
| ecfbfddc7c | |||
| ac53ecfa5d | |||
| 090ae3848c | |||
| 23da7eedf5 | |||
| 7279cdd216 | |||
| 4c327b61d4 | |||
| 877dbf9999 | |||
| 4a1cb6b47c |
@@ -38,6 +38,8 @@ jobs:
|
||||
- name: security
|
||||
token_secret: SECURITY_REVIEW_TOKEN
|
||||
model: gpt-5
|
||||
patterns_repo: rodin/security-patterns
|
||||
patterns_files: "."
|
||||
system_prompt_file: SECURITY_REVIEW.md
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
@@ -60,8 +62,8 @@ jobs:
|
||||
AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
|
||||
AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
|
||||
CONVENTIONS_FILE: "CONVENTIONS.md"
|
||||
PATTERNS_REPO: "rodin/go-patterns"
|
||||
PATTERNS_FILES: "README.md,patterns/"
|
||||
PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
|
||||
PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
|
||||
LLM_TIMEOUT: "600"
|
||||
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
|
||||
run: ./review-bot
|
||||
|
||||
@@ -0,0 +1,200 @@
|
||||
# This composite action is designed for Gitea Actions runners.
|
||||
# Gitea Actions supports GitHub Actions syntax including $GITHUB_OUTPUT,
|
||||
# actions/cache, and actions/checkout.
|
||||
# Requirements: python3, sha256sum, curl (all present on ubuntu-* runners).
|
||||
name: 'AI Code Review'
|
||||
description: 'Run AI-powered code review on a pull request using review-bot'
|
||||
|
||||
inputs:
|
||||
gitea-url:
|
||||
description: 'Gitea instance URL (defaults to server_url)'
|
||||
required: false
|
||||
default: ''
|
||||
repo:
|
||||
description: 'Repository (owner/name, defaults to current)'
|
||||
required: false
|
||||
default: ''
|
||||
pr-number:
|
||||
description: 'Pull request number (defaults to current PR)'
|
||||
required: false
|
||||
default: ''
|
||||
reviewer-token:
|
||||
description: 'Gitea token for posting the review'
|
||||
required: true
|
||||
reviewer-name:
|
||||
description: 'Display name for the reviewer'
|
||||
required: false
|
||||
default: ''
|
||||
llm-base-url:
|
||||
description: 'OpenAI-compatible LLM API base URL (not required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
llm-api-key:
|
||||
description: 'LLM API key (not required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
llm-model:
|
||||
description: 'LLM model name'
|
||||
required: true
|
||||
llm-provider:
|
||||
description: 'LLM API provider: openai, anthropic, or aicore (default openai)'
|
||||
required: false
|
||||
default: 'openai'
|
||||
aicore-client-id:
|
||||
description: 'SAP AI Core client ID (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-client-secret:
|
||||
description: 'SAP AI Core client secret (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-auth-url:
|
||||
description: 'SAP AI Core authentication URL (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-api-url:
|
||||
description: 'SAP AI Core API URL (required for aicore provider)'
|
||||
required: false
|
||||
default: ''
|
||||
aicore-resource-group:
|
||||
description: 'SAP AI Core resource group (default: default)'
|
||||
required: false
|
||||
default: 'default'
|
||||
conventions-file:
|
||||
description: 'Path to conventions file in the repo (e.g. CLAUDE.md)'
|
||||
required: false
|
||||
default: ''
|
||||
patterns-repo:
|
||||
description: 'Comma-separated repos with language patterns (e.g. rodin/elixir-patterns,rodin/phoenix-conventions)'
|
||||
required: false
|
||||
default: ''
|
||||
patterns-files:
|
||||
description: 'Comma-separated file paths or directories to fetch from patterns repos'
|
||||
required: false
|
||||
default: 'README.md'
|
||||
temperature:
|
||||
description: 'LLM temperature (0 = server default)'
|
||||
required: false
|
||||
default: '0'
|
||||
timeout:
|
||||
description: 'LLM request timeout in seconds (default 300)'
|
||||
required: false
|
||||
default: '300'
|
||||
version:
|
||||
description: 'review-bot version to install (e.g. v0.1.0, defaults to latest)'
|
||||
required: false
|
||||
default: 'latest'
|
||||
dry-run:
|
||||
description: 'Print review to stdout instead of posting'
|
||||
required: false
|
||||
default: 'false'
|
||||
update-existing:
|
||||
description: 'Delete previous review from same bot after posting new one. Accepts: true/1/yes or false/0/no (default true)'
|
||||
required: false
|
||||
default: 'true'
|
||||
system-prompt-file:
|
||||
description: 'Local file with additional system prompt instructions (e.g. security review focus)'
|
||||
required: false
|
||||
default: ''
|
||||
persona:
|
||||
description: 'Built-in persona name (security, architect, docs)'
|
||||
required: false
|
||||
default: ''
|
||||
persona-file:
|
||||
description: 'Path to custom persona JSON file'
|
||||
required: false
|
||||
default: ''
|
||||
|
||||
runs:
|
||||
using: 'composite'
|
||||
steps:
|
||||
- name: Determine version
|
||||
id: version
|
||||
shell: bash
|
||||
run: |
|
||||
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
|
||||
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
|
||||
if [ "${{ inputs.version }}" = "latest" ]; then
|
||||
VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
|
||||
| python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
|
||||
if [ -z "$VERSION" ]; then
|
||||
echo "Failed to determine latest version" >&2
|
||||
exit 1
|
||||
fi
|
||||
else
|
||||
VERSION="${{ inputs.version }}"
|
||||
fi
|
||||
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
|
||||
|
||||
- name: Cache review-bot binary
|
||||
id: cache
|
||||
uses: actions/cache@v4
|
||||
with:
|
||||
path: ${{ runner.temp }}/review-bot
|
||||
key: review-bot-linux-amd64-${{ steps.version.outputs.version }}
|
||||
|
||||
- name: Install review-bot
|
||||
if: steps.cache.outputs.cache-hit != 'true'
|
||||
shell: bash
|
||||
run: |
|
||||
GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
|
||||
REPO="${{ inputs.repo || 'rodin/review-bot' }}"
|
||||
VERSION="${{ steps.version.outputs.version }}"
|
||||
BINARY="review-bot-linux-amd64"
|
||||
|
||||
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
|
||||
-o "${{ runner.temp }}/review-bot"
|
||||
curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/checksums.txt" \
|
||||
-o "${{ runner.temp }}/checksums.txt"
|
||||
|
||||
# Verify SHA-256 checksum
|
||||
cd "${{ runner.temp }}"
|
||||
EXPECTED=$(grep "${BINARY}" checksums.txt | awk '{print $1}')
|
||||
ACTUAL=$(sha256sum review-bot | awk '{print $1}')
|
||||
|
||||
if [ -z "$EXPECTED" ]; then
|
||||
echo "Error: no checksum found for ${BINARY}" >&2
|
||||
exit 1
|
||||
fi
|
||||
if [ "$EXPECTED" != "$ACTUAL" ]; then
|
||||
echo "Error: checksum mismatch!" >&2
|
||||
echo " Expected: $EXPECTED" >&2
|
||||
echo " Actual: $ACTUAL" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
chmod +x "${{ runner.temp }}/review-bot"
|
||||
echo "Installed review-bot ${VERSION} (checksum verified)"
|
||||
|
||||
- name: Run review
|
||||
shell: bash
|
||||
env:
|
||||
GITHUB_SERVER_URL: ${{ inputs.gitea-url || github.server_url }}
|
||||
GITHUB_REPOSITORY: ${{ inputs.repo || github.repository }}
|
||||
PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
|
||||
REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
|
||||
REVIEWER_NAME: ${{ inputs.reviewer-name }}
|
||||
LLM_BASE_URL: ${{ inputs.llm-base-url }}
|
||||
LLM_API_KEY: ${{ inputs.llm-api-key }}
|
||||
LLM_MODEL: ${{ inputs.llm-model }}
|
||||
CONVENTIONS_FILE: ${{ inputs.conventions-file }}
|
||||
PATTERNS_REPO: ${{ inputs.patterns-repo }}
|
||||
PATTERNS_FILES: ${{ inputs.patterns-files }}
|
||||
LLM_TEMPERATURE: ${{ inputs.temperature }}
|
||||
LLM_TIMEOUT: ${{ inputs.timeout }}
|
||||
LLM_PROVIDER: ${{ inputs.llm-provider }}
|
||||
UPDATE_EXISTING: ${{ inputs.update-existing }}
|
||||
SYSTEM_PROMPT_FILE: ${{ inputs.system-prompt-file }}
|
||||
PERSONA: ${{ inputs.persona }}
|
||||
PERSONA_FILE: ${{ inputs.persona-file }}
|
||||
AICORE_CLIENT_ID: ${{ inputs.aicore-client-id }}
|
||||
AICORE_CLIENT_SECRET: ${{ inputs.aicore-client-secret }}
|
||||
AICORE_AUTH_URL: ${{ inputs.aicore-auth-url }}
|
||||
AICORE_API_URL: ${{ inputs.aicore-api-url }}
|
||||
AICORE_RESOURCE_GROUP: ${{ inputs.aicore-resource-group }}
|
||||
run: |
|
||||
ARGS=""
|
||||
if [ "${{ inputs.dry-run }}" = "true" ]; then
|
||||
ARGS="--dry-run"
|
||||
fi
|
||||
${{ runner.temp }}/review-bot $ARGS
|
||||
@@ -0,0 +1,69 @@
|
||||
name: CI
|
||||
|
||||
on:
|
||||
push:
|
||||
branches: [main]
|
||||
pull_request:
|
||||
types: [opened, synchronize]
|
||||
|
||||
jobs:
|
||||
test:
|
||||
runs-on: ubuntu-24.04
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-go@v5
|
||||
with:
|
||||
go-version: '1.26'
|
||||
- run: go test ./...
|
||||
- run: go vet ./...
|
||||
- run: go build -o review-bot ./cmd/review-bot
|
||||
|
||||
# Self-review using native SAP AI Core provider
|
||||
# Models must match SAP AI Core deployments
|
||||
# Available models: gpt-5, anthropic--claude-4.6-sonnet, anthropic--claude-4.6-opus
|
||||
# Removed gpt-4.1, gpt-5-mini, gpt-4.1-mini - not deployed on AI Core
|
||||
review:
|
||||
runs-on: ubuntu-24.04
|
||||
if: github.event_name == 'pull_request'
|
||||
needs: test
|
||||
strategy:
|
||||
matrix:
|
||||
include:
|
||||
- name: sonnet
|
||||
token_secret: SONNET_REVIEW_TOKEN
|
||||
model: anthropic--claude-4.6-sonnet
|
||||
- name: gpt
|
||||
token_secret: GPT_REVIEW_TOKEN
|
||||
model: gpt-5
|
||||
- name: security
|
||||
token_secret: SECURITY_REVIEW_TOKEN
|
||||
model: gpt-5
|
||||
patterns_repo: rodin/security-patterns
|
||||
patterns_files: "."
|
||||
system_prompt_file: SECURITY_REVIEW.md
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-go@v5
|
||||
with:
|
||||
go-version: '1.26'
|
||||
- run: go build -o review-bot ./cmd/review-bot
|
||||
- name: Run ${{ matrix.name }} review
|
||||
env:
|
||||
GITHUB_SERVER_URL: ${{ github.server_url }}
|
||||
GITHUB_REPOSITORY: ${{ github.repository }}
|
||||
PR_NUMBER: ${{ github.event.pull_request.number }}
|
||||
REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
|
||||
REVIEWER_NAME: ${{ matrix.name }}
|
||||
LLM_PROVIDER: aicore
|
||||
LLM_MODEL: ${{ matrix.model }}
|
||||
AICORE_CLIENT_ID: ${{ secrets.AICORE_CLIENT_ID }}
|
||||
AICORE_CLIENT_SECRET: ${{ secrets.AICORE_CLIENT_SECRET }}
|
||||
AICORE_AUTH_URL: ${{ secrets.AICORE_AUTH_URL }}
|
||||
AICORE_API_URL: ${{ secrets.AICORE_API_URL }}
|
||||
AICORE_RESOURCE_GROUP: ${{ secrets.AICORE_RESOURCE_GROUP }}
|
||||
CONVENTIONS_FILE: "CONVENTIONS.md"
|
||||
PATTERNS_REPO: ${{ matrix.patterns_repo || 'rodin/go-patterns' }}
|
||||
PATTERNS_FILES: ${{ matrix.patterns_files || 'README.md,patterns/' }}
|
||||
LLM_TIMEOUT: "600"
|
||||
SYSTEM_PROMPT_FILE: ${{ matrix.system_prompt_file }}
|
||||
run: ./review-bot
|
||||
@@ -0,0 +1,38 @@
|
||||
name: PR Ready Gate
|
||||
|
||||
on:
|
||||
pull_request:
|
||||
types: [synchronize]
|
||||
|
||||
jobs:
|
||||
clear-labels:
|
||||
runs-on: ubuntu-24.04
|
||||
# Always run - curl commands are safe if labels don't exist
|
||||
steps:
|
||||
- name: Remove ready and self-reviewed labels, reassign to author
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.RODIN_TOKEN }}
|
||||
run: |
|
||||
PR_NUMBER=${{ github.event.pull_request.number }}
|
||||
AUTHOR=${{ github.event.pull_request.user.login }}
|
||||
READY_LABEL_ID=38
|
||||
SELF_REVIEWED_LABEL_ID=37
|
||||
|
||||
# Remove ready label if present
|
||||
curl -sS -X DELETE \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER}/labels/${READY_LABEL_ID}" || true
|
||||
|
||||
# Remove self-reviewed label if present
|
||||
curl -sS -X DELETE \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/issues/${PR_NUMBER}/labels/${SELF_REVIEWED_LABEL_ID}" || true
|
||||
|
||||
# Reassign to author
|
||||
curl -sS -X PATCH \
|
||||
-H "Authorization: token $GITEA_TOKEN" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "{\"assignees\": [\"${AUTHOR}\"]}" \
|
||||
"https://gitea.weiker.me/api/v1/repos/${{ github.repository }}/pulls/${PR_NUMBER}"
|
||||
|
||||
echo "Cleared ready/self-reviewed labels and reassigned PR #${PR_NUMBER} to ${AUTHOR}"
|
||||
@@ -0,0 +1,97 @@
|
||||
name: Release
|
||||
|
||||
on:
|
||||
push:
|
||||
tags:
|
||||
- 'v*'
|
||||
|
||||
jobs:
|
||||
release:
|
||||
runs-on: ubuntu-24.04
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
||||
- uses: actions/setup-go@v5
|
||||
with:
|
||||
go-version: '1.26'
|
||||
|
||||
- name: Run tests
|
||||
run: |
|
||||
go vet ./...
|
||||
go test ./...
|
||||
|
||||
- name: Build binaries
|
||||
run: |
|
||||
VERSION=${GITHUB_REF_NAME}
|
||||
mkdir -p dist
|
||||
|
||||
GOOS=linux GOARCH=amd64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-linux-amd64 ./cmd/review-bot
|
||||
GOOS=linux GOARCH=arm64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-linux-arm64 ./cmd/review-bot
|
||||
GOOS=darwin GOARCH=amd64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-darwin-amd64 ./cmd/review-bot
|
||||
GOOS=darwin GOARCH=arm64 go build -ldflags "-s -w -X main.version=${VERSION}" -o dist/review-bot-darwin-arm64 ./cmd/review-bot
|
||||
|
||||
cd dist && sha256sum * > checksums.txt
|
||||
|
||||
- name: Create release and upload assets
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.RELEASE_TOKEN }}
|
||||
run: |
|
||||
VERSION=${GITHUB_REF_NAME}
|
||||
GITEA_URL="${{ github.server_url }}"
|
||||
REPO="${{ github.repository }}"
|
||||
|
||||
# Create release (or find existing one for this tag)
|
||||
HTTP_CODE=$(curl -s -o /tmp/release_response.json -w "%{http_code}" -X POST \
|
||||
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||
-H "Content-Type: application/json" \
|
||||
"${GITEA_URL}/api/v1/repos/${REPO}/releases" \
|
||||
-d "{\"tag_name\": \"${VERSION}\", \"name\": \"${VERSION}\", \"body\": \"Release ${VERSION}\", \"draft\": false, \"prerelease\": false}")
|
||||
|
||||
if [ "$HTTP_CODE" = "409" ]; then
|
||||
echo "Release for ${VERSION} already exists, fetching existing..."
|
||||
curl -sSf -o /tmp/release_response.json \
|
||||
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||
"${GITEA_URL}/api/v1/repos/${REPO}/releases/tags/${VERSION}"
|
||||
elif [ "$HTTP_CODE" != "201" ]; then
|
||||
echo "Failed to create release (HTTP ${HTTP_CODE})" >&2
|
||||
cat /tmp/release_response.json >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# Parse release ID (python3 available on ubuntu-24.04 runners)
|
||||
RELEASE_ID=$(python3 -c "import json; print(json.load(open('/tmp/release_response.json'))['id'])")
|
||||
|
||||
if [ -z "$RELEASE_ID" ]; then
|
||||
echo "Failed to parse release ID" >&2
|
||||
cat /tmp/release_response.json >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "Release ID: ${RELEASE_ID}"
|
||||
|
||||
# Upload each asset (idempotent: delete existing asset with same name first)
|
||||
for file in dist/*; do
|
||||
filename=$(basename "$file")
|
||||
echo "Uploading ${filename}..."
|
||||
|
||||
# Check if asset already exists and delete it
|
||||
EXISTING_ID=$(export ASSET_NAME="${filename}"; curl -sS \
|
||||
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||
"${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets" \
|
||||
| python3 -c "import json,sys,os; name=os.environ['ASSET_NAME']; assets=json.load(sys.stdin); print(next((str(a['id']) for a in assets if a['name']==name),''))" 2>/dev/null)
|
||||
|
||||
if [ -n "$EXISTING_ID" ]; then
|
||||
echo " Asset ${filename} already exists (id=${EXISTING_ID}), deleting..."
|
||||
curl -sSf -X DELETE \
|
||||
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||
"${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets/${EXISTING_ID}"
|
||||
fi
|
||||
|
||||
curl -sSf -X POST \
|
||||
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||
-H "Content-Type: application/octet-stream" \
|
||||
"${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=$(printf '%s' "${filename}" | jq -sRr @uri)" \
|
||||
--data-binary "@${file}"
|
||||
done
|
||||
|
||||
echo "Release ${VERSION} created with assets"
|
||||
@@ -329,11 +329,12 @@ All flags have environment variable equivalents:
|
||||
### Token Scopes Required
|
||||
|
||||
| Scope | Purpose |
|
||||
|-------|---------|
|
||||
|-------|--------|
|
||||
| `write:issue` | Post and delete reviews |
|
||||
| `write:repository` | Read PR diffs, file content, commit statuses |
|
||||
| `read:user` | Self-request as reviewer (optional but recommended) |
|
||||
|
||||
No `read:user` scope needed — the bot identifies itself from the review response.
|
||||
Without `read:user`, the bot still works but cannot add itself to the PR's reviewer list.
|
||||
|
||||
## Development
|
||||
|
||||
|
||||
@@ -54,8 +54,8 @@ func main() {
|
||||
logFormat := flag.String("log-format", envOrDefault("LOG_FORMAT", "text"), "Log output format: text or json")
|
||||
verbosity := flag.String("verbosity", envOrDefault("LOG_VERBOSITY", "info"), "Log verbosity: debug, info, warn, error")
|
||||
// CLI flags
|
||||
giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
|
||||
repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)")
|
||||
giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", envOrDefault("GITHUB_SERVER_URL", "")), "Gitea instance URL")
|
||||
repo := flag.String("repo", envOrDefault("GITEA_REPO", envOrDefault("GITHUB_REPOSITORY", "")), "Repository (owner/name)")
|
||||
prNum := flag.String("pr", envOrDefault("PR_NUMBER", ""), "Pull request number")
|
||||
reviewerName := flag.String("reviewer-name", envOrDefault("REVIEWER_NAME", ""), "Reviewer display name")
|
||||
reviewerToken := flag.String("reviewer-token", envOrDefault("REVIEWER_TOKEN", ""), "Gitea token for posting review")
|
||||
|
||||
+215
-17
@@ -11,9 +11,11 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"syscall"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -39,12 +41,26 @@ func IsNotFound(err error) bool {
|
||||
return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
|
||||
}
|
||||
|
||||
// IsServerError reports whether an error is an API 5xx response.
|
||||
func IsServerError(err error) bool {
|
||||
var apiErr *APIError
|
||||
return errors.As(err, &apiErr) && apiErr.StatusCode >= 500 && apiErr.StatusCode < 600
|
||||
}
|
||||
|
||||
// Client interacts with the Gitea API.
|
||||
// A Client is safe for concurrent use by multiple goroutines.
|
||||
type Client struct {
|
||||
baseURL string
|
||||
token string
|
||||
http *http.Client
|
||||
|
||||
// RetryBackoff defines the delays between retry attempts.
|
||||
// RetryBackoff[i] is the delay before attempt i+1 (after attempt i fails).
|
||||
// If nil, defaults to {1s, 2s}. Set to shorter durations in tests.
|
||||
//
|
||||
// This field must be configured before the first request is made.
|
||||
// Modifying it while requests are in flight is not safe.
|
||||
RetryBackoff []time.Duration
|
||||
}
|
||||
|
||||
// NewClient creates a new Gitea API client.
|
||||
@@ -56,6 +72,12 @@ func NewClient(baseURL, token string) *Client {
|
||||
}
|
||||
}
|
||||
|
||||
// SetHTTPClient sets the underlying HTTP client used for requests.
|
||||
// This is intended for testing to inject mock transports.
|
||||
func (c *Client) SetHTTPClient(hc *http.Client) {
|
||||
c.http = hc
|
||||
}
|
||||
|
||||
// PullRequest holds relevant PR metadata.
|
||||
type PullRequest struct {
|
||||
Title string `json:"title"`
|
||||
@@ -210,24 +232,185 @@ func (c *Client) PostReview(ctx context.Context, owner, repo string, number int,
|
||||
return &review, nil
|
||||
}
|
||||
|
||||
// isTemporaryNetError reports whether err is a temporary network error worth retrying.
|
||||
// This includes connection refused, network unreachable, connection reset, and DNS
|
||||
// timeouts. It explicitly excludes permanent errors like permission denied or
|
||||
// "no such host" DNS failures.
|
||||
func isTemporaryNetError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check for OpError and inspect the underlying syscall error.
|
||||
// Not all OpErrors are transient — permission denied, for example, is permanent.
|
||||
var opErr *net.OpError
|
||||
if errors.As(err, &opErr) {
|
||||
return isRetriableSyscallError(opErr.Err)
|
||||
}
|
||||
|
||||
// DNS errors: only retry on timeout, not on "no such host" which is permanent.
|
||||
var dnsErr *net.DNSError
|
||||
if errors.As(err, &dnsErr) {
|
||||
return dnsErr.IsTimeout
|
||||
}
|
||||
|
||||
// Check for net.Error with Timeout() (Temporary is deprecated)
|
||||
var netErr net.Error
|
||||
if errors.As(err, &netErr) {
|
||||
return netErr.Timeout()
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// isRetriableSyscallError reports whether the underlying error from a net.OpError
|
||||
// is a transient syscall error worth retrying.
|
||||
func isRetriableSyscallError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
// Check for syscall.Errno directly or wrapped
|
||||
var errno syscall.Errno
|
||||
if errors.As(err, &errno) {
|
||||
switch errno {
|
||||
case syscall.ECONNREFUSED, // connection refused — server not listening
|
||||
syscall.ECONNRESET, // connection reset by peer
|
||||
syscall.ENETUNREACH, // network unreachable
|
||||
syscall.EHOSTUNREACH, // host unreachable
|
||||
syscall.ETIMEDOUT: // connection timed out
|
||||
return true
|
||||
default:
|
||||
// EACCES, EPERM, etc. are permanent — don't retry
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// If we can't identify the specific syscall error, be conservative and retry.
|
||||
// This handles wrapped errors or platform-specific error types.
|
||||
// The retry count is limited, so erring on the side of retrying is safe.
|
||||
return true
|
||||
}
|
||||
|
||||
// redactURL strips query parameters from a URL for safe logging.
|
||||
// This prevents accidental exposure of sensitive data that future callers
|
||||
// might pass via query strings.
|
||||
func redactURL(rawURL string) string {
|
||||
parsed, err := url.Parse(rawURL)
|
||||
if err != nil {
|
||||
// If we cannot parse it, return a safe placeholder rather than
|
||||
// potentially logging something sensitive.
|
||||
return "[invalid URL]"
|
||||
}
|
||||
if parsed.RawQuery != "" {
|
||||
parsed.RawQuery = "[redacted]"
|
||||
}
|
||||
return parsed.String()
|
||||
}
|
||||
|
||||
// sanitizeErrorForLog returns a loggable version of an error that omits
|
||||
// potentially sensitive content like response bodies. For APIError, only
|
||||
// the status code is included; for other errors, the type is preserved.
|
||||
func sanitizeErrorForLog(err error) string {
|
||||
if err == nil {
|
||||
return "<nil>"
|
||||
}
|
||||
var apiErr *APIError
|
||||
if errors.As(err, &apiErr) {
|
||||
return fmt.Sprintf("HTTP %d", apiErr.StatusCode)
|
||||
}
|
||||
return err.Error()
|
||||
}
|
||||
|
||||
// doGet performs an HTTP GET request with retry on 5xx errors and temporary
|
||||
// network errors. Retries up to 3 times with exponential backoff (1s, 2s delays
|
||||
// by default; configurable via Client.RetryBackoff for testing).
|
||||
func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
const maxAttempts = 3
|
||||
// backoff[i] is the delay before attempt i+1 (i.e., after attempt i fails).
|
||||
// First attempt (i=0) has no delay; retries wait 1s then 2s by default.
|
||||
backoff := c.RetryBackoff
|
||||
if backoff == nil {
|
||||
backoff = []time.Duration{1 * time.Second, 2 * time.Second}
|
||||
}
|
||||
req.Header.Set("Authorization", "token "+c.token)
|
||||
|
||||
resp, err := c.http.Do(req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
// maxErrorBodyBytes limits how much of an error response body we read
|
||||
// to protect against malicious servers sending unbounded data.
|
||||
const maxErrorBodyBytes = 64 * 1024 // 64 KB
|
||||
|
||||
if resp.StatusCode < 200 || resp.StatusCode >= 300 {
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
return nil, &APIError{StatusCode: resp.StatusCode, Body: string(body)}
|
||||
var lastErr error
|
||||
for attempt := 0; attempt < maxAttempts; attempt++ {
|
||||
if attempt > 0 {
|
||||
// Determine delay: use backoff slice if available, otherwise retry immediately.
|
||||
// An empty RetryBackoff slice means "retry without delay" — this is intentional
|
||||
// as the caller explicitly configured no delays.
|
||||
var delay time.Duration
|
||||
if attempt-1 < len(backoff) {
|
||||
delay = backoff[attempt-1]
|
||||
}
|
||||
|
||||
if delay > 0 {
|
||||
slog.Warn("retrying request after error",
|
||||
"attempt", attempt+1,
|
||||
"url", redactURL(reqURL),
|
||||
"delay", delay.String(),
|
||||
"lastError", sanitizeErrorForLog(lastErr))
|
||||
|
||||
timer := time.NewTimer(delay)
|
||||
select {
|
||||
case <-timer.C:
|
||||
case <-ctx.Done():
|
||||
timer.Stop()
|
||||
return nil, ctx.Err()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
req.Header.Set("Authorization", "token "+c.token)
|
||||
|
||||
resp, err := c.http.Do(req)
|
||||
if err != nil {
|
||||
// Always capture the error for consistent return at loop end.
|
||||
// This ensures both network errors and HTTP 5xx return lastErr.
|
||||
lastErr = err
|
||||
|
||||
// Only retry temporary network errors when attempts remain.
|
||||
if attempt < maxAttempts-1 && isTemporaryNetError(err) {
|
||||
slog.Warn("temporary network error, will retry",
|
||||
"attempt", attempt+1,
|
||||
"url", redactURL(reqURL),
|
||||
"error", err)
|
||||
continue
|
||||
}
|
||||
// Non-retryable network error or final attempt exhausted.
|
||||
return nil, lastErr
|
||||
}
|
||||
if resp.StatusCode >= 200 && resp.StatusCode < 300 {
|
||||
body, err := io.ReadAll(resp.Body)
|
||||
resp.Body.Close()
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return body, nil
|
||||
}
|
||||
|
||||
// Error path: limit how much we read from potentially malicious server
|
||||
errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
|
||||
resp.Body.Close()
|
||||
|
||||
lastErr = &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
|
||||
|
||||
// Only retry on 5xx server errors
|
||||
if resp.StatusCode < 500 || resp.StatusCode >= 600 {
|
||||
return nil, lastErr
|
||||
}
|
||||
}
|
||||
return io.ReadAll(resp.Body)
|
||||
|
||||
return nil, lastErr
|
||||
}
|
||||
|
||||
// escapePath escapes each segment of a relative file path for use in URLs.
|
||||
@@ -251,7 +434,13 @@ type ContentEntry struct {
|
||||
|
||||
// ListContents lists files and directories at a given path in a repo.
|
||||
// Pass an empty path to list the repository root.
|
||||
// If the path points to a file (not a directory), Gitea returns a single
|
||||
// object instead of an array; this method normalizes both cases to a slice.
|
||||
func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
|
||||
// Normalize "." to empty string — Gitea API rejects "." with 500
|
||||
if path == "." {
|
||||
path = ""
|
||||
}
|
||||
var reqURL string
|
||||
if path == "" {
|
||||
reqURL = fmt.Sprintf("%s/api/v1/repos/%s/%s/contents", c.baseURL, url.PathEscape(owner), url.PathEscape(repo))
|
||||
@@ -264,7 +453,16 @@ func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]
|
||||
}
|
||||
var entries []ContentEntry
|
||||
if err := json.Unmarshal(body, &entries); err != nil {
|
||||
return nil, fmt.Errorf("parse contents JSON: %w", err)
|
||||
// Gitea returns a single object (not an array) when path is a file
|
||||
var single ContentEntry
|
||||
if err2 := json.Unmarshal(body, &single); err2 != nil {
|
||||
return nil, fmt.Errorf("parse contents JSON: %w", err)
|
||||
}
|
||||
// Guard against empty/malformed responses
|
||||
if single.Name == "" && single.Path == "" {
|
||||
return nil, fmt.Errorf("parse contents JSON: empty response for path %q", path)
|
||||
}
|
||||
entries = []ContentEntry{single}
|
||||
}
|
||||
return entries, nil
|
||||
}
|
||||
@@ -317,9 +515,9 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
|
||||
|
||||
// Review represents a pull request review from the Gitea API.
|
||||
type Review struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
User struct {
|
||||
ID int64 `json:"id"`
|
||||
Body string `json:"body"`
|
||||
User struct {
|
||||
Login string `json:"login"`
|
||||
} `json:"user"`
|
||||
State string `json:"state"`
|
||||
|
||||
+406
-5
@@ -6,10 +6,14 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestGetPullRequest(t *testing.T) {
|
||||
@@ -276,11 +280,64 @@ func TestListContents(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestListContents_DotPath(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
// "." should be normalized to empty path, which hits the root contents endpoint
|
||||
if r.URL.Path != "/api/v1/repos/owner/repo/contents" {
|
||||
t.Errorf("expected root contents path, got: %s", r.URL.Path)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprintf(w, `[{"name":"README.md","path":"README.md","type":"file"}]`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
entries, err := client.ListContents(context.Background(), "owner", "repo", ".")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(entries) != 1 {
|
||||
t.Fatalf("expected 1 entry, got %d", len(entries))
|
||||
}
|
||||
if entries[0].Name != "README.md" {
|
||||
t.Errorf("expected README.md, got %s", entries[0].Name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListContents_FilePath(t *testing.T) {
|
||||
// Gitea returns a single object (not an array) when path is a file
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path != "/api/v1/repos/owner/repo/contents/README.md" {
|
||||
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||
}
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
// Single object, not an array
|
||||
fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
entries, err := client.ListContents(context.Background(), "owner", "repo", "README.md")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if len(entries) != 1 {
|
||||
t.Fatalf("expected 1 entry, got %d", len(entries))
|
||||
}
|
||||
if entries[0].Name != "README.md" {
|
||||
t.Errorf("expected README.md, got %s", entries[0].Name)
|
||||
}
|
||||
if entries[0].Type != "file" {
|
||||
t.Errorf("expected type file, got %s", entries[0].Type)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetAllFilesInPath_File(t *testing.T) {
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/api/v1/repos/owner/repo/contents/README.md" {
|
||||
// Gitea returns 404 for contents API on files (it's not a dir)
|
||||
http.NotFound(w, r)
|
||||
// Gitea returns a single object (not array) when path is a file
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
fmt.Fprintf(w, `{"name":"README.md","path":"README.md","type":"file"}`)
|
||||
return
|
||||
}
|
||||
if r.URL.Path == "/api/v1/repos/owner/repo/raw/README.md" {
|
||||
@@ -584,9 +641,9 @@ func TestGetAllFilesInPath_403Propagates(t *testing.T) {
|
||||
|
||||
func TestIsNotFound(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil error", nil, false},
|
||||
{"non-API error", fmt.Errorf("network timeout"), false},
|
||||
@@ -743,3 +800,347 @@ func TestResolveComment_Error(t *testing.T) {
|
||||
t.Fatal("expected error for 404 response")
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsServerError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil error", nil, false},
|
||||
{"non-API error", fmt.Errorf("network timeout"), false},
|
||||
{"404 APIError", &APIError{StatusCode: 404, Body: "not found"}, false},
|
||||
{"500 APIError", &APIError{StatusCode: 500, Body: "server error"}, true},
|
||||
{"502 APIError", &APIError{StatusCode: 502, Body: "bad gateway"}, true},
|
||||
{"503 APIError", &APIError{StatusCode: 503, Body: "unavailable"}, true},
|
||||
{"599 APIError", &APIError{StatusCode: 599, Body: "edge case"}, true},
|
||||
{"600 not server error", &APIError{StatusCode: 600, Body: "edge"}, false},
|
||||
{"400 not server error", &APIError{StatusCode: 400, Body: "bad request"}, false},
|
||||
{"wrapped 500", fmt.Errorf("fetch: %w", &APIError{StatusCode: 500, Body: "err"}), true},
|
||||
{"wrapped 404", fmt.Errorf("fetch: %w", &APIError{StatusCode: 404, Body: "err"}), false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := IsServerError(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("IsServerError(%v) = %v, want %v", tt.err, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_RetriesOn500(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
if attempts < 3 {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
w.Write([]byte(`{"message":"transient error"}`))
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte(`{"data":"success"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
// Use short backoff for fast tests
|
||||
client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
|
||||
|
||||
body, err := client.doGet(context.Background(), server.URL+"/test")
|
||||
if err != nil {
|
||||
t.Fatalf("expected success after retry, got error: %v", err)
|
||||
}
|
||||
if string(body) != `{"data":"success"}` {
|
||||
t.Errorf("body = %q, want %q", string(body), `{"data":"success"}`)
|
||||
}
|
||||
if attempts != 3 {
|
||||
t.Errorf("attempts = %d, want 3", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_FailsAfterMaxRetries(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
w.Write([]byte(`{"message":"persistent error"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
// Use short backoff for fast tests
|
||||
client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
|
||||
|
||||
_, err := client.doGet(context.Background(), server.URL+"/test")
|
||||
if err == nil {
|
||||
t.Fatal("expected error after max retries")
|
||||
}
|
||||
var apiErr *APIError
|
||||
if !errors.As(err, &apiErr) {
|
||||
t.Fatalf("expected APIError, got: %v", err)
|
||||
}
|
||||
if apiErr.StatusCode != http.StatusInternalServerError {
|
||||
t.Errorf("status = %d, want 500", apiErr.StatusCode)
|
||||
}
|
||||
if attempts != 3 {
|
||||
t.Errorf("attempts = %d, want 3 (max retries)", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_NoRetryOn4xx(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
w.WriteHeader(http.StatusForbidden)
|
||||
w.Write([]byte(`{"message":"forbidden"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
_, err := client.doGet(context.Background(), server.URL+"/test")
|
||||
if err == nil {
|
||||
t.Fatal("expected error for 403")
|
||||
}
|
||||
var apiErr *APIError
|
||||
if !errors.As(err, &apiErr) {
|
||||
t.Fatalf("expected APIError, got: %v", err)
|
||||
}
|
||||
if apiErr.StatusCode != http.StatusForbidden {
|
||||
t.Errorf("status = %d, want 403", apiErr.StatusCode)
|
||||
}
|
||||
if attempts != 1 {
|
||||
t.Errorf("attempts = %d, want 1 (no retry on 4xx)", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoGet_RespectsContextCancellation(t *testing.T) {
|
||||
attempts := 0
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
attempts++
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
w.Write([]byte(`{"message":"error"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
|
||||
client := NewClient(server.URL, "test-token")
|
||||
// Use longer backoff to give us time to cancel during the wait
|
||||
client.RetryBackoff = []time.Duration{100 * time.Millisecond, 100 * time.Millisecond}
|
||||
|
||||
// Cancel after first attempt returns and retry begins
|
||||
go func() {
|
||||
time.Sleep(20 * time.Millisecond)
|
||||
cancel()
|
||||
}()
|
||||
|
||||
_, err := client.doGet(ctx, server.URL+"/test")
|
||||
if err == nil {
|
||||
t.Fatal("expected error on context cancellation")
|
||||
}
|
||||
// Should have made 1 attempt, then context cancelled during backoff
|
||||
if attempts != 1 {
|
||||
t.Errorf("attempts = %d, expected 1 before context cancel during backoff", attempts)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// mockTransport is a test helper that returns errors for the first N calls,
|
||||
// then delegates to a real server.
|
||||
type mockTransport struct {
|
||||
failCount int32 // number of failures remaining (atomic)
|
||||
failErr error // error to return on failure
|
||||
realServer *httptest.Server
|
||||
attemptsMade atomic.Int32 // tracks total attempts
|
||||
}
|
||||
|
||||
func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
|
||||
m.attemptsMade.Add(1)
|
||||
remaining := atomic.AddInt32(&m.failCount, -1)
|
||||
if remaining >= 0 {
|
||||
// Still have failures to return
|
||||
return nil, m.failErr
|
||||
}
|
||||
// Redirect to real server
|
||||
req.URL.Host = m.realServer.Listener.Addr().String()
|
||||
req.URL.Scheme = "http"
|
||||
return http.DefaultTransport.RoundTrip(req)
|
||||
}
|
||||
|
||||
func TestDoGet_RetriesOnTemporaryNetError(t *testing.T) {
|
||||
// Real server that will handle successful requests
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte(`{"status":"ok"}`))
|
||||
}))
|
||||
defer server.Close()
|
||||
|
||||
// Mock transport: fail twice with ECONNREFUSED, then succeed
|
||||
mt := &mockTransport{
|
||||
failCount: 2,
|
||||
failErr: &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED},
|
||||
realServer: server,
|
||||
}
|
||||
|
||||
client := NewClient("http://fake-host/", "test-token")
|
||||
client.SetHTTPClient(&http.Client{Transport: mt})
|
||||
client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}
|
||||
|
||||
body, err := client.doGet(context.Background(), "http://fake-host/test")
|
||||
if err != nil {
|
||||
t.Fatalf("expected success after retries, got error: %v", err)
|
||||
}
|
||||
if string(body) != `{"status":"ok"}` {
|
||||
t.Errorf("body = %q, want %q", string(body), `{"status":"ok"}`)
|
||||
}
|
||||
|
||||
// Should have made exactly 3 attempts: 2 failures + 1 success
|
||||
if got := mt.attemptsMade.Load(); got != 3 {
|
||||
t.Errorf("attempts = %d, want 3 (2 failures + 1 success)", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsTemporaryNetError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil error", nil, false},
|
||||
{"plain error", fmt.Errorf("some error"), false},
|
||||
// OpError with retriable syscall errors
|
||||
{"OpError ECONNREFUSED", &net.OpError{Op: "dial", Err: syscall.ECONNREFUSED}, true},
|
||||
{"OpError ECONNRESET", &net.OpError{Op: "read", Err: syscall.ECONNRESET}, true},
|
||||
{"OpError ENETUNREACH", &net.OpError{Op: "dial", Err: syscall.ENETUNREACH}, true},
|
||||
{"OpError EHOSTUNREACH", &net.OpError{Op: "dial", Err: syscall.EHOSTUNREACH}, true},
|
||||
{"OpError ETIMEDOUT", &net.OpError{Op: "dial", Err: syscall.ETIMEDOUT}, true},
|
||||
// OpError with permanent syscall errors — should NOT retry
|
||||
{"OpError EACCES", &net.OpError{Op: "dial", Err: syscall.EACCES}, false},
|
||||
{"OpError EPERM", &net.OpError{Op: "dial", Err: syscall.EPERM}, false},
|
||||
// OpError with unknown inner error — conservative retry
|
||||
{"OpError unknown inner", &net.OpError{Op: "dial", Err: fmt.Errorf("unknown")}, true},
|
||||
// DNS errors
|
||||
{"DNS timeout", &net.DNSError{IsTimeout: true}, true},
|
||||
{"DNS no such host", &net.DNSError{IsTimeout: false, Name: "bad.host"}, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := isTemporaryNetError(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("isTemporaryNetError(%v) = %v, want %v", tt.err, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsRetriableSyscallError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{"nil", nil, false},
|
||||
{"ECONNREFUSED", syscall.ECONNREFUSED, true},
|
||||
{"ECONNRESET", syscall.ECONNRESET, true},
|
||||
{"ENETUNREACH", syscall.ENETUNREACH, true},
|
||||
{"EHOSTUNREACH", syscall.EHOSTUNREACH, true},
|
||||
{"ETIMEDOUT", syscall.ETIMEDOUT, true},
|
||||
{"EACCES (permanent)", syscall.EACCES, false},
|
||||
{"EPERM (permanent)", syscall.EPERM, false},
|
||||
{"ENOENT (permanent)", syscall.ENOENT, false},
|
||||
{"unknown error", fmt.Errorf("something"), true}, // conservative retry
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := isRetriableSyscallError(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("isRetriableSyscallError(%v) = %v, want %v", tt.err, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedactURL(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "no query params",
|
||||
input: "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
|
||||
want: "https://gitea.example.com/api/v1/repos/owner/repo/pulls/1",
|
||||
},
|
||||
{
|
||||
name: "with query params - redacts",
|
||||
input: "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?ref=main",
|
||||
want: "https://gitea.example.com/api/v1/repos/owner/repo/raw/file?[redacted]",
|
||||
},
|
||||
{
|
||||
name: "multiple query params",
|
||||
input: "https://example.com/path?token=secret&page=1",
|
||||
want: "https://example.com/path?[redacted]",
|
||||
},
|
||||
{
|
||||
name: "invalid URL",
|
||||
input: "://invalid",
|
||||
want: "[invalid URL]",
|
||||
},
|
||||
{
|
||||
name: "empty string",
|
||||
input: "",
|
||||
want: "",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := redactURL(tt.input)
|
||||
if got != tt.want {
|
||||
t.Errorf("redactURL(%q) = %q, want %q", tt.input, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSanitizeErrorForLog(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "nil error",
|
||||
err: nil,
|
||||
want: "<nil>",
|
||||
},
|
||||
{
|
||||
name: "APIError omits body",
|
||||
err: &APIError{StatusCode: 500, Body: "internal error: database connection failed"},
|
||||
want: "HTTP 500",
|
||||
},
|
||||
{
|
||||
name: "APIError with large body still only shows status",
|
||||
err: &APIError{StatusCode: 502, Body: strings.Repeat("x", 1000)},
|
||||
want: "HTTP 502",
|
||||
},
|
||||
{
|
||||
name: "non-API error preserved",
|
||||
err: fmt.Errorf("connection refused"),
|
||||
want: "connection refused",
|
||||
},
|
||||
{
|
||||
name: "wrapped APIError",
|
||||
err: fmt.Errorf("request failed: %w", &APIError{StatusCode: 503, Body: "service unavailable"}),
|
||||
want: "HTTP 503",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := sanitizeErrorForLog(tt.err)
|
||||
if got != tt.want {
|
||||
t.Errorf("sanitizeErrorForLog() = %q, want %q", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user