Report HTTP status in Gitea/Forgejo API errors

gitea/branches.go

@@ -3,7 +3,6 @@ package gitea
 import (
 	"context"
 	forge "github.com/git-pkgs/forge"
-	"net/http"
 
 	"code.gitea.io/sdk/gitea"
 )
@@ -29,10 +28,7 @@ func (s *giteaBranchService) List(ctx context.Context, owner, repo string, opts
 			ListOptions: gitea.ListOptions{Page: page, PageSize: perPage},
 		})
 		if err != nil {
-			if resp != nil && resp.StatusCode == http.StatusNotFound {
-				return nil, forge.ErrNotFound
-			}
-			return nil, err
+			return nil, wrapErr("list branches", resp, err)
 		}
 		for _, b := range branches {
 			branch := forge.Branch{
@@ -63,10 +59,7 @@ func (s *giteaBranchService) Create(ctx context.Context, owner, repo, name, from
 		OldBranchName: from,
 	})
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("create branch", resp, err)
 	}
 
 	result := forge.Branch{
@@ -81,10 +74,7 @@ func (s *giteaBranchService) Create(ctx context.Context, owner, repo, name, from
 func (s *giteaBranchService) Delete(ctx context.Context, owner, repo, name string) error {
 	_, resp, err := s.client.DeleteRepoBranch(owner, repo, name)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return forge.ErrNotFound
-		}
-		return err
+		return wrapErr("delete branch", resp, err)
 	}
 	return nil
 }

gitea/ci.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"context"
 	"io"
-	"net/http"
 
 	forge "github.com/git-pkgs/forge"
 
@@ -93,10 +92,7 @@ func (s *giteaCIService) ListRuns(_ context.Context, owner, repo string, opts fo
 	for {
 		resp, httpResp, err := s.client.ListRepoActionRuns(owner, repo, gOpts)
 		if err != nil {
-			if httpResp != nil && httpResp.StatusCode == http.StatusNotFound {
-				return nil, forge.ErrNotFound
-			}
-			return nil, err
+			return nil, wrapErr("list workflow runs", httpResp, err)
 		}
 		for _, r := range resp.WorkflowRuns {
 			all = append(all, convertGiteaWorkflowRun(r))
@@ -117,10 +113,7 @@ func (s *giteaCIService) ListRuns(_ context.Context, owner, repo string, opts fo
 func (s *giteaCIService) GetRun(_ context.Context, owner, repo string, runID int64) (*forge.CIRun, error) {
 	r, resp, err := s.client.GetRepoActionRun(owner, repo, runID)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("get workflow run", resp, err)
 	}
 	result := convertGiteaWorkflowRun(r)
 
@@ -149,10 +142,7 @@ func (s *giteaCIService) RetryRun(_ context.Context, _, _ string, _ int64) error
 func (s *giteaCIService) GetJobLog(_ context.Context, owner, repo string, jobID int64) (io.ReadCloser, error) {
 	data, resp, err := s.client.GetRepoActionJobLogs(owner, repo, jobID)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("get job log", resp, err)
 	}
 	return io.NopCloser(bytes.NewReader(data)), nil
 }

gitea/collaborators.go

@@ -3,7 +3,6 @@ package gitea
 import (
 	"context"
 	forge "github.com/git-pkgs/forge"
-	"net/http"
 
 	"code.gitea.io/sdk/gitea"
 )
@@ -29,10 +28,7 @@ func (s *giteaCollaboratorService) List(ctx context.Context, owner, repo string,
 			ListOptions: gitea.ListOptions{Page: page, PageSize: perPage},
 		})
 		if err != nil {
-			if resp != nil && resp.StatusCode == http.StatusNotFound {
-				return nil, forge.ErrNotFound
-			}
-			return nil, err
+			return nil, wrapErr("list collaborators", resp, err)
 		}
 		for _, u := range users {
 			perm, err := s.getPermission(owner, repo, u.UserName)
@@ -60,10 +56,7 @@ func (s *giteaCollaboratorService) List(ctx context.Context, owner, repo string,
 func (s *giteaCollaboratorService) getPermission(owner, repo, username string) (string, error) {
 	result, resp, err := s.client.CollaboratorPermission(owner, repo, username)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return "", forge.ErrNotFound
-		}
-		return "", err
+		return "", wrapErr("get collaborator permission", resp, err)
 	}
 	switch result.Permission {
 	case "admin", "owner":
@@ -86,10 +79,7 @@ func (s *giteaCollaboratorService) Add(ctx context.Context, owner, repo, usernam
 		Permission: perm,
 	})
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return forge.ErrNotFound
-		}
-		return err
+		return wrapErr("add collaborator", resp, err)
 	}
 	return nil
 }
@@ -110,10 +100,7 @@ func giteaPermission(permission forge.AccessLevel) string {
 func (s *giteaCollaboratorService) Remove(ctx context.Context, owner, repo, username string) error {
 	resp, err := s.client.DeleteCollaborator(owner, repo, username)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return forge.ErrNotFound
-		}
-		return err
+		return wrapErr("remove collaborator", resp, err)
 	}
 	return nil
 }

gitea/commit_statuses.go

@@ -3,7 +3,6 @@ package gitea
 import (
 	"context"
 	forge "github.com/git-pkgs/forge"
-	"net/http"
 
 	"code.gitea.io/sdk/gitea"
 )
@@ -50,10 +49,7 @@ func (s *giteaCommitStatusService) List(ctx context.Context, owner, repo, sha st
 			ListOptions: gitea.ListOptions{Page: page, PageSize: defaultPageSize},
 		})
 		if err != nil {
-			if resp != nil && resp.StatusCode == http.StatusNotFound {
-				return nil, forge.ErrNotFound
-			}
-			return nil, err
+			return nil, wrapErr("list commit statuses", resp, err)
 		}
 		for _, st := range statuses {
 			cs := forge.CommitStatus{
@@ -84,10 +80,7 @@ func (s *giteaCommitStatusService) Set(ctx context.Context, owner, repo, sha str
 		Context:     opts.Context,
 	})
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("set commit status", resp, err)
 	}
 
 	cs := &forge.CommitStatus{

gitea/commits.go

@@ -38,7 +38,7 @@ func (s *giteaCommitService) ResolveCommit(_ context.Context, owner, repo, ref s
 		if resp != nil && resp.StatusCode == http.StatusNotFound {
 			return "", forge.CommitRefError(owner, repo, ref, forge.ErrNotFound)
 		}
-		return "", forge.CommitRefError(owner, repo, ref, err)
+		return "", forge.CommitRefError(owner, repo, ref, wrapErr("get commit", resp, err))
 	}
 	if commit == nil || commit.CommitMeta == nil {
 		return "", forge.CommitRefError(owner, repo, ref, errors.New("empty response"))

gitea/deploy_keys.go

@@ -3,7 +3,6 @@ package gitea
 import (
 	"context"
 	forge "github.com/git-pkgs/forge"
-	"net/http"
 
 	"code.gitea.io/sdk/gitea"
 )
@@ -29,10 +28,7 @@ func (s *giteaDeployKeyService) List(ctx context.Context, owner, repo string, op
 			ListOptions: gitea.ListOptions{Page: page, PageSize: perPage},
 		})
 		if err != nil {
-			if resp != nil && resp.StatusCode == http.StatusNotFound {
-				return nil, forge.ErrNotFound
-			}
-			return nil, err
+			return nil, wrapErr("list deploy keys", resp, err)
 		}
 		for _, k := range keys {
 			all = append(all, forge.DeployKey{
@@ -59,10 +55,7 @@ func (s *giteaDeployKeyService) List(ctx context.Context, owner, repo string, op
 func (s *giteaDeployKeyService) Get(ctx context.Context, owner, repo string, id int64) (*forge.DeployKey, error) {
 	k, resp, err := s.client.GetDeployKey(owner, repo, id)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("get deploy key", resp, err)
 	}
 
 	return &forge.DeployKey{
@@ -81,10 +74,7 @@ func (s *giteaDeployKeyService) Create(ctx context.Context, owner, repo string,
 		ReadOnly: opts.ReadOnly,
 	})
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("create deploy key", resp, err)
 	}
 
 	return &forge.DeployKey{
@@ -99,10 +89,7 @@ func (s *giteaDeployKeyService) Create(ctx context.Context, owner, repo string,
 func (s *giteaDeployKeyService) Delete(ctx context.Context, owner, repo string, id int64) error {
 	resp, err := s.client.DeleteDeployKey(owner, repo, id)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return forge.ErrNotFound
-		}
-		return err
+		return wrapErr("delete deploy key", resp, err)
 	}
 	return nil
 }

gitea/errors.go

@@ -0,0 +1,32 @@
+package gitea
+
+import (
+	"fmt"
+	forge "github.com/git-pkgs/forge"
+	"net/http"
+	"strings"
+
+	"code.gitea.io/sdk/gitea"
+)
+
+// wrapErr adds the operation name and HTTP status to an SDK error. The SDK
+// returns errors containing only the server's "message" field, which Forgejo
+// blanks on 500s in production mode and which can otherwise be cryptic
+// (e.g. "Release has no Tag" for a 409). Without the status code the caller
+// has nothing to go on.
+func wrapErr(op string, resp *gitea.Response, err error) error {
+	if resp != nil && resp.StatusCode == http.StatusNotFound {
+		return forge.ErrNotFound
+	}
+	msg := strings.TrimSpace(err.Error())
+	if resp == nil {
+		if msg == "" {
+			return fmt.Errorf("%s: %w", op, err)
+		}
+		return fmt.Errorf("%s: %s", op, msg)
+	}
+	if msg == "" {
+		return fmt.Errorf("%s: %s", op, resp.Status)
+	}
+	return fmt.Errorf("%s: %s: %s", op, resp.Status, msg)
+}

gitea/errors_test.go

@@ -0,0 +1,55 @@
+package gitea
+
+import (
+	"errors"
+	forge "github.com/git-pkgs/forge"
+	"net/http"
+	"testing"
+
+	"code.gitea.io/sdk/gitea"
+)
+
+func resp(code int, status string) *gitea.Response {
+	return &gitea.Response{Response: &http.Response{StatusCode: code, Status: status}}
+}
+
+func TestWrapErr(t *testing.T) {
+	tests := []struct {
+		name string
+		op   string
+		resp *gitea.Response
+		err  error
+		want string
+	}{
+		{"blank message", "create issue", resp(500, "500 Internal Server Error"), errors.New(""), "create issue: 500 Internal Server Error"},
+		{"whitespace message", "create issue", resp(500, "500 Internal Server Error"), errors.New("  \n"), "create issue: 500 Internal Server Error"},
+		{"with message", "create issue", resp(422, "422 Unprocessable Entity"), errors.New("bad input"), "create issue: 422 Unprocessable Entity: bad input"},
+		{"nil resp with message", "list labels", nil, errors.New("dial tcp: connection refused"), "list labels: dial tcp: connection refused"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := wrapErr(tc.op, tc.resp, tc.err)
+			if got.Error() != tc.want {
+				t.Errorf("want %q, got %q", tc.want, got.Error())
+			}
+		})
+	}
+}
+
+func TestWrapErrNotFound(t *testing.T) {
+	got := wrapErr("get issue", resp(404, "404 Not Found"), errors.New("The target couldn't be found."))
+	if !errors.Is(got, forge.ErrNotFound) {
+		t.Errorf("404 should return ErrNotFound sentinel, got %v", got)
+	}
+}
+
+func TestWrapErrNilRespBlankMessage(t *testing.T) {
+	inner := errors.New("")
+	got := wrapErr("get issue", nil, inner)
+	if got.Error() != "get issue: " {
+		t.Errorf("want %q, got %q", "get issue: ", got.Error())
+	}
+	if !errors.Is(got, inner) {
+		t.Error("nil-resp blank error should wrap the original for errors.Is")
+	}
+}

gitea/files.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/base64"
 	forge "github.com/git-pkgs/forge"
-	"net/http"
 
 	"code.gitea.io/sdk/gitea"
 )
@@ -20,10 +19,7 @@ func (f *giteaForge) Files() forge.FileService {
 func (s *giteaFileService) Get(ctx context.Context, owner, repo, path, ref string) (*forge.FileContent, error) {
 	cr, resp, err := s.client.GetContents(owner, repo, ref, path)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("get file", resp, err)
 	}
 
 	var content []byte
@@ -49,10 +45,7 @@ func (s *giteaFileService) Get(ctx context.Context, owner, repo, path, ref strin
 func (s *giteaFileService) List(ctx context.Context, owner, repo, path, ref string) ([]forge.FileEntry, error) {
 	items, resp, err := s.client.ListContents(owner, repo, ref, path)
 	if err != nil {
-		if resp != nil && resp.StatusCode == http.StatusNotFound {
-			return nil, forge.ErrNotFound
-		}
-		return nil, err
+		return nil, wrapErr("list files", resp, err)
 	}
 
 	entries := make([]forge.FileEntry, len(items))