refactor(errors): explain failed requests through the SDK

The hints for a missing repository, a pull request number that names an
issue and an existing pull request read the API with raw GETs from the app
layer. Read them through SDK lookups in services, which reuse the failed
request's instance and credential, and let transport only hand out that
client.

Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
bircni committed 2026-10-12 09:57:12 +02:00
1 parent 03aa64233f
commit 5fd4be9803
4 files changed
+105 -58

No files matched your search

+9 -23
View File
@@ -10,7 +10,7 @@ import (
"fmt" "fmt"
"net" "net"
"net/http" "net/http"
"net/url" "strconv"
"strings" "strings"
"syscall" "syscall"
"time" "time"
@@ -116,18 +116,10 @@ func explainable(err error, apiErr *transport.APIError) bool {
// names an issue. It returns "" when the resource itself is missing. // names an issue. It returns "" when the resource itself is missing.
func explainRepositoryNotFound(ctx context.Context, apiErr *transport.APIError, resource services.Resource) string { func explainRepositoryNotFound(ctx context.Context, apiErr *transport.APIError, resource services.Resource) string {
repo := resource.Repository repo := resource.Repository
var state struct { var state services.RepositoryState
Empty bool `json:"empty"`
HasIssues bool `json:"has_issues"`
HasPullRequests bool `json:"has_pull_requests"`
HasWiki bool `json:"has_wiki"`
HasReleases bool `json:"has_releases"`
HasActions bool `json:"has_actions"`
HasProjects bool `json:"has_projects"`
}
status := http.StatusNotFound status := http.StatusNotFound
if resource.Kind != "repository" { if resource.Kind != "repository" {
status = apiErr.Lookup(ctx, "repos/"+repo, &state) state, status = services.LookupRepository(ctx, apiErr, repo)
} }
switch status { switch status {
case http.StatusNotFound: case http.StatusNotFound:
@@ -140,9 +132,9 @@ func explainRepositoryNotFound(ctx context.Context, apiErr *transport.APIError,
name string name string
enabled bool enabled bool
}{ }{
"issues": {"issues are", state.HasIssues}, "pulls": {"pull requests are", state.HasPullRequests}, "issues": {"issues are", state.Issues}, "pulls": {"pull requests are", state.PullRequests},
"wiki": {"the wiki is", state.HasWiki}, "releases": {"releases are", state.HasReleases}, "wiki": {"the wiki is", state.Wiki}, "releases": {"releases are", state.Releases},
"actions": {"Actions are", state.HasActions}, "projects": {"projects are", state.HasProjects}, "actions": {"Actions are", state.Actions}, "projects": {"projects are", state.Projects},
} }
if feature, ok := features[resource.Section]; ok && !feature.enabled { if feature, ok := features[resource.Section]; ok && !feature.enabled {
return feature.name + " disabled for " + repo return feature.name + " disabled for " + repo
@@ -151,10 +143,7 @@ func explainRepositoryNotFound(ctx context.Context, apiErr *transport.APIError,
return repo + " is empty; it has no pull requests" return repo + " is empty; it has no pull requests"
} }
if number, ok := strings.CutPrefix(resource.Key, "#"); ok && resource.Kind == "pull request" { if number, ok := strings.CutPrefix(resource.Key, "#"); ok && resource.Kind == "pull request" {
var issue struct { if index, err := strconv.ParseInt(number, 10, 64); err == nil && services.LookupIsIssue(ctx, apiErr, repo, index) {
PullRequest any `json:"pull_request"`
}
if apiErr.Lookup(ctx, "repos/"+repo+"/issues/"+number, &issue) == http.StatusOK && issue.PullRequest == nil {
return fmt.Sprintf("#%s in %s is an issue, not a pull request; view it with: tea issue view %s", number, repo, number) return fmt.Sprintf("#%s in %s is an issue, not a pull request; view it with: tea issue view %s", number, repo, number)
} }
} }
@@ -204,11 +193,8 @@ func existsHint(ctx context.Context, apiErr *transport.APIError, conflict *servi
if !ok || conflict.Scope == "" { if !ok || conflict.Scope == "" {
return "" return ""
} }
var pull struct { if pullURL := services.LookupPullURL(ctx, apiErr, conflict.Scope, base, head); pullURL != "" {
URL string `json:"html_url"` return "see " + pullURL
}
if apiErr.Lookup(ctx, "repos/"+conflict.Scope+"/pulls/"+url.PathEscape(base)+"/"+url.PathEscape(head), &pull) == http.StatusOK && pull.URL != "" {
return "see " + pull.URL
} }
case "wiki page": case "wiki page":
return "edit it with: tea wiki edit " + conflict.Key return "edit it with: tea wiki edit " + conflict.Key
+81
View File
@@ -0,0 +1,81 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package services
import (
"context"
"net/http"
gitea "gitea.dev/sdk"
"gitea.dev/tea/internal/transport"
)
// lookup repeats a read on the instance of the failed request apiErr, with its credential, to
// explain the failure. It returns the response status, or 0 when apiErr was not recorded from a
// request, the read got no response, or a successful response did not decode.
func lookup[T any](ctx context.Context, apiErr *transport.APIError, repository string, op repoOperation[T]) (T, int) {
var zero T
httpClient := apiErr.LookupClient()
if httpClient == nil {
return zero, 0
}
owner, repo, err := splitRepository(repository)
if err != nil {
return zero, 0
}
sdk, err := gitea.NewClient(apiErr.Instance, gitea.SetHTTPClient(httpClient), gitea.SetGiteaVersion(""))
if err != nil {
return zero, 0
}
result, resp, err := op(ctx, sdk, owner, repo)
switch {
case resp == nil || resp.Response == nil:
return zero, 0
case err == nil:
return result, resp.StatusCode
case resp.StatusCode >= 200 && resp.StatusCode < 300:
return zero, 0
}
return zero, resp.StatusCode
}
// RepositoryState holds the repository facts that explain a failed request on it.
type RepositoryState struct {
Empty, Issues, PullRequests, Wiki, Releases, Actions, Projects bool
}
// LookupRepository reads the state of repo for the failed request apiErr; see lookup for the status.
func LookupRepository(ctx context.Context, apiErr *transport.APIError, repo string) (RepositoryState, int) {
item, status := lookup(ctx, apiErr, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Repository, *gitea.Response, error) {
return c.Repositories.GetRepo(ctx, owner, name)
})
if item == nil {
return RepositoryState{}, status
}
return RepositoryState{
Empty: item.Empty, Issues: item.HasIssues, PullRequests: item.HasPullRequests, Wiki: item.HasWiki,
Releases: item.HasReleases, Actions: item.HasActions, Projects: item.HasProjects,
}, status
}
// LookupIsIssue reports whether number names an issue rather than a pull request in repo, for the
// failed request apiErr.
func LookupIsIssue(ctx context.Context, apiErr *transport.APIError, repo string, number int64) bool {
item, status := lookup(ctx, apiErr, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Issue, *gitea.Response, error) {
return c.Issues.GetIssue(ctx, owner, name, number)
})
return status == http.StatusOK && item != nil && item.PullRequest == nil
}
// LookupPullURL returns the web URL of the pull request from head into base in repo, for the failed
// request apiErr, or "".
func LookupPullURL(ctx context.Context, apiErr *transport.APIError, repo, base, head string) string {
item, status := lookup(ctx, apiErr, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.PullRequest, *gitea.Response, error) {
return c.PullRequests.GetPullRequestByBaseHead(ctx, owner, name, base, head)
})
if status != http.StatusOK || item == nil {
return ""
}
return item.HTMLURL
}
+8 -27
View File
@@ -35,8 +35,7 @@ type APIError struct {
// Instance is the installation URL the failed request was sent to, when it was recorded // Instance is the installation URL the failed request was sent to, when it was recorded
// from a request. // from a request.
Instance string Instance string
// apiBase and lookup repeat reads on the failed request's instance with its credential. // lookup repeats reads on the failed request's instance with its credential.
apiBase string
lookup http.RoundTripper lookup http.RoundTripper
} }
@@ -70,46 +69,28 @@ func (e *APIError) request() string {
} }
// bind records the instance of the request to u and the credentialed transport rt, so the // bind records the instance of the request to u and the credentialed transport rt, so the
// failure can later be explained with Lookup. // failure can later be explained with LookupClient.
func (e *APIError) bind(u *url.URL, rt http.RoundTripper) { func (e *APIError) bind(u *url.URL, rt http.RoundTripper) {
prefix, _, ok := strings.Cut(u.Path, "/api/v1/") prefix, _, ok := strings.Cut(u.Path, "/api/v1/")
if !ok { if !ok {
return return
} }
instance := url.URL{Scheme: u.Scheme, Host: u.Host, Path: prefix} instance := url.URL{Scheme: u.Scheme, Host: u.Host, Path: prefix}
e.Instance = instance.String() e.Instance, e.lookup = instance.String(), rt
instance.Path += "/api/v1/"
e.apiBase, e.lookup = instance.String(), rt
} }
// Lookup reads endpoint, a path below the API base such as "repos/alice/project", with the // LookupClient returns an HTTP client that repeats reads on the instance of the failed request,
// instance and credential of the request that failed, and decodes a successful JSON response // Instance, with its credential, or nil when the error was not recorded from a request.
// into out. It returns the response status, or 0 when the error was not recorded from a func (e *APIError) LookupClient() *http.Client {
// request or the lookup itself failed.
func (e *APIError) Lookup(ctx context.Context, endpoint string, out any) int {
if e.lookup == nil { if e.lookup == nil {
return 0 return nil
} }
req, err := http.NewRequestWithContext(ctx, http.MethodGet, e.apiBase+strings.TrimLeft(endpoint, "/"), nil) return &http.Client{Transport: e.lookup, CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }}
if err != nil {
return 0
}
req.Header.Set("Accept", "application/json")
resp, err := e.lookup.RoundTrip(req)
if err != nil {
return 0
}
defer resp.Body.Close()
if resp.StatusCode >= 200 && resp.StatusCode < 300 && out != nil && json.NewDecoder(io.LimitReader(resp.Body, maxLookupBody)).Decode(out) != nil {
return 0
}
return resp.StatusCode
} }
const ( const (
maxErrorBody = 4 << 10 maxErrorBody = 4 << 10
maxErrorMessage = 300 maxErrorMessage = 300
maxLookupBody = 1 << 20
) )
// newAPIError keeps only a bounded, printable JSON "message" field of a failed response body. // newAPIError keeps only a bounded, printable JSON "message" field of a failed response body.
+7 -8
View File
@@ -232,12 +232,11 @@ func TestAPIErrorLookupRepeatsReadsWithTheFailedCredential(t *testing.T) {
var apiErr *APIError var apiErr *APIError
require.ErrorAs(t, err, &apiErr) require.ErrorAs(t, err, &apiErr)
require.Equal(t, server.URL+"/base", apiErr.Instance) require.Equal(t, server.URL+"/base", apiErr.Instance)
var repo struct { lookup := apiErr.LookupClient()
FullName string `json:"full_name"` require.NotNil(t, lookup)
HasIssues bool `json:"has_issues"` resp, err := lookup.Get(server.URL + "/base/api/v1/repos/a/b")
} require.NoError(t, err)
require.Equal(t, http.StatusOK, apiErr.Lookup(context.Background(), "repos/a/b", &repo)) _ = resp.Body.Close()
require.Equal(t, "a/b", repo.FullName) require.Equal(t, http.StatusOK, resp.StatusCode)
require.Equal(t, http.StatusNotFound, apiErr.Lookup(context.Background(), "repos/a/missing", nil)) require.Nil(t, (&APIError{Status: http.StatusNotFound}).LookupClient(), "errors built without a request cannot look up")
require.Zero(t, (&APIError{Status: http.StatusNotFound}).Lookup(context.Background(), "repos/a/b", nil), "errors built without a request cannot look up")
} }