Merge pull request 'fix: distinguish 404 in GetAllFilesInPath, make uploads idempotent' (#33) from fix/8-10-error-handling-idempotent-upload into main

.gitea/workflows/release.yml

@@ -69,14 +69,28 @@ jobs:
 
           echo "Release ID: ${RELEASE_ID}"
 
-          # Upload each asset
+          # 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=${filename}" \
+              "${GITEA_URL}/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=$(printf '%s' "${filename}" | jq -sRr @uri)" \
               --data-binary "@${file}"
           done
 

gitea/client.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -16,6 +17,28 @@ import (
 	"time"
 )
 
+// APIError represents an HTTP error response from the Gitea API.
+// It carries the status code so callers can distinguish between
+// different failure modes (e.g. 404 vs 500).
+type APIError struct {
+	StatusCode int
+	Body       string
+}
+
+func (e *APIError) Error() string {
+	body := e.Body
+	if len(body) > 200 {
+		body = body[:200] + "...(truncated)"
+	}
+	return fmt.Sprintf("HTTP %d: %s", e.StatusCode, body)
+}
+
+// IsNotFound reports whether an error is an API 404 response.
+func IsNotFound(err error) bool {
+	var apiErr *APIError
+	return errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound
+}
+
 // Client interacts with the Gitea API.
 // A Client is safe for concurrent use by multiple goroutines.
 type Client struct {
@@ -201,7 +224,7 @@ func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) {
 
 	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
 		body, _ := io.ReadAll(resp.Body)
-		return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, string(body))
+		return nil, &APIError{StatusCode: resp.StatusCode, Body: string(body)}
 	}
 	return io.ReadAll(resp.Body)
 }
@@ -254,10 +277,15 @@ func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string
 	// Try listing as directory first
 	entries, err := c.ListContents(ctx, owner, repo, path)
 	if err != nil {
-		// Might be a file, try fetching directly
+		// Only fall back to single-file fetch on 404 (path is a file, not a dir).
+		// Propagate all other errors (auth failures, server errors, rate limits).
+		if !IsNotFound(err) {
+			return nil, fmt.Errorf("list contents %q: %w", path, err)
+		}
+		// 404 means the path might be a file — try fetching directly
 		content, fileErr := c.GetFileContent(ctx, owner, repo, path)
 		if fileErr != nil {
-			return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, err)
+			return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, fileErr)
 		}
 		results[path] = content
 		return results, nil

gitea/client_test.go

@@ -3,6 +3,7 @@ package gitea
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -505,3 +506,99 @@ func TestGetTimelineReviewCommentID_NotFound(t *testing.T) {
 		t.Fatal("expected error when sentinel not found")
 	}
 }
+
+func TestGetAllFilesInPath_404FallsBackToFile(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/api/v1/repos/owner/repo/contents/README.md":
+			// Contents API returns 404 for files (not a directory)
+			w.WriteHeader(http.StatusNotFound)
+			w.Write([]byte(`{"message":"not found"}`))
+		case "/api/v1/repos/owner/repo/raw/README.md":
+			w.Write([]byte("# Hello\n"))
+		default:
+			w.WriteHeader(http.StatusNotFound)
+			w.Write([]byte(`{"message":"not found"}`))
+		}
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("expected fallback to file on 404, got error: %v", err)
+	}
+	if len(files) != 1 {
+		t.Fatalf("expected 1 file, got %d", len(files))
+	}
+	if files["README.md"] != "# Hello\n" {
+		t.Errorf("unexpected content: %q", files["README.md"])
+	}
+}
+
+func TestGetAllFilesInPath_500Propagates(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Simulate a server error from ListContents
+		w.WriteHeader(http.StatusInternalServerError)
+		w.Write([]byte(`{"message":"internal server error"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "somepath")
+	if err == nil {
+		t.Fatal("expected error to propagate for 500, got nil")
+	}
+	// Should NOT fall back to file fetch — error should propagate
+	var apiErr *APIError
+	if !errors.As(err, &apiErr) {
+		t.Fatalf("expected APIError in chain, got: %v", err)
+	}
+	if apiErr.StatusCode != http.StatusInternalServerError {
+		t.Errorf("expected status 500, got %d", apiErr.StatusCode)
+	}
+}
+
+func TestGetAllFilesInPath_403Propagates(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusForbidden)
+		w.Write([]byte(`{"message":"token has insufficient scope"}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "test-token")
+	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "private/stuff")
+	if err == nil {
+		t.Fatal("expected error to propagate for 403, got nil")
+	}
+	var apiErr *APIError
+	if !errors.As(err, &apiErr) {
+		t.Fatalf("expected APIError in chain, got: %v", err)
+	}
+	if apiErr.StatusCode != http.StatusForbidden {
+		t.Errorf("expected status 403, got %d", apiErr.StatusCode)
+	}
+}
+
+func TestIsNotFound(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"}, true},
+		{"500 APIError", &APIError{StatusCode: 500, Body: "server error"}, false},
+		{"wrapped 404", fmt.Errorf("list contents: %w", &APIError{StatusCode: 404, Body: "not found"}), true},
+		{"wrapped 500", fmt.Errorf("list contents: %w", &APIError{StatusCode: 500, Body: "err"}), false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := IsNotFound(tt.err)
+			if got != tt.want {
+				t.Errorf("IsNotFound(%v) = %v, want %v", tt.err, got, tt.want)
+			}
+		})
+	}
+}