From ad77ab6940fdc81057c834246ee45532c56c59f7 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Wed, 1 Apr 2026 15:51:58 +0200 Subject: [PATCH 1/4] update GraphQLErrors to have constructor + handle paths --- internal/api/errors.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/internal/api/errors.go b/internal/api/errors.go index e5ff8193ce..fe7fffb973 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -9,6 +9,19 @@ import ( // GraphQlErrors contains one or more GraphQlError instances. type GraphQlErrors []*GraphQlError +// NewGraphQlErrors wraps raw GraphQL error payloads in GraphQlError values. +func NewGraphQlErrors(graphQLErrorPayloads []json.RawMessage) GraphQlErrors { + errs := make(GraphQlErrors, 0, len(graphQLErrorPayloads)) + for _, graphQLErrorPayload := range graphQLErrorPayloads { + var value any + if err := json.Unmarshal(graphQLErrorPayload, &value); err != nil { + value = map[string]any{"message": string(graphQLErrorPayload)} + } + errs = append(errs, &GraphQlError{v: value}) + } + return errs +} + func (gg GraphQlErrors) Error() string { // This slightly convoluted implementation is used to ensure that output // remains stable with earlier versions of src-cli, which returned a wrapped @@ -48,6 +61,23 @@ func (g *GraphQlError) Code() (string, error) { return "", nil } +// Path returns the GraphQL error path, if one was set on the error. +func (g *GraphQlError) Path() ([]any, error) { + e, ok := g.v.(map[string]any) + if !ok { + return nil, errors.Errorf("unexpected GraphQL error of type %T", g.v) + } + + if e["path"] == nil { + return nil, nil + } + path, ok := e["path"].([]any) + if !ok { + return nil, errors.Errorf("unexpected path of type %T", e["path"]) + } + return path, nil +} + func (g *GraphQlError) Error() string { j, _ := json.MarshalIndent(g.v, "", " ") return string(j) From 9fcfd4c2d536eef995aefaa11be87486414509b3 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Wed, 1 Apr 2026 15:52:24 +0200 Subject: [PATCH 2/4] add test for repo list with some repos failing but not all --- cmd/src/repos_list_test.go | 101 +++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 cmd/src/repos_list_test.go diff --git a/cmd/src/repos_list_test.go b/cmd/src/repos_list_test.go new file mode 100644 index 0000000000..2866c95f4c --- /dev/null +++ b/cmd/src/repos_list_test.go @@ -0,0 +1,101 @@ +package main + +import ( + "context" + "strings" + "testing" + + mockapi "github.com/sourcegraph/src-cli/internal/api/mock" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestListRepositoriesSkipsRepositoryWhenDefaultBranchCannotBeResolved(t *testing.T) { + client := new(mockapi.Client) + request := &mockapi.Request{Response: `{ + "data": { + "repositories": { + "nodes": [ + { + "id": "UmVwb3NpdG9yeTox", + "name": "github.com/sourcegraph/ok", + "url": "/github.com/sourcegraph/ok", + "description": "", + "language": "Go", + "createdAt": "2026-03-31T00:00:00Z", + "updatedAt": null, + "externalRepository": { + "id": "RXh0ZXJuYWxSZXBvc2l0b3J5OjE=", + "serviceType": "github", + "serviceID": "https://github.com/" + }, + "defaultBranch": { + "name": "refs/heads/main", + "displayName": "main" + }, + "viewerCanAdminister": false, + "keyValuePairs": [] + }, + { + "id": "UmVwb3NpdG9yeToy", + "name": "github.com/sourcegraph/broken", + "url": "/github.com/sourcegraph/broken", + "description": "", + "language": "Go", + "createdAt": "2026-03-31T00:00:00Z", + "updatedAt": null, + "externalRepository": { + "id": "RXh0ZXJuYWxSZXBvc2l0b3J5OjI=", + "serviceType": "github", + "serviceID": "https://github.com/" + }, + "defaultBranch": null, + "viewerCanAdminister": false, + "keyValuePairs": [] + } + ] + } + }, + "errors": [ + { + "message": "failed to resolve HEAD for github.com/sourcegraph/broken", + "path": ["repositories", "nodes", 1, "defaultBranch"] + } + ] + }`} + + client.On( + "NewRequest", + mock.MatchedBy(func(query string) bool { + return strings.Contains(query, "defaultBranch") + }), + mock.MatchedBy(func(vars map[string]any) bool { + indexed, ok := vars["indexed"].(bool) + if !ok || !indexed { + return false + } + notIndexed, ok := vars["notIndexed"].(bool) + return ok && !notIndexed + }), + ).Return(request).Once() + + request.On("DoRaw", context.Background(), mock.Anything). + Return(true, nil). + Once() + + repos, skipped, err := listRepositories(context.Background(), client, reposListOptions{ + first: 1000, + cloned: true, + notCloned: true, + indexed: true, + notIndexed: false, + orderBy: "REPOSITORY_NAME", + }) + require.NoError(t, err) + require.Len(t, repos, 1) + require.Len(t, skipped, 1) + require.Equal(t, "github.com/sourcegraph/ok", repos[0].Name) + require.Equal(t, "github.com/sourcegraph/broken", skipped[0].Name) + client.AssertExpectations(t) + request.AssertExpectations(t) +} From 05ed4b19149b19428cd87c6a856577e6cd0c5335 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Wed, 1 Apr 2026 15:52:47 +0200 Subject: [PATCH 3/4] update src repo list to handle graphql errors - errors are treated as warnings when getting partial data - if we have no data and just errors that is a fatal error --- cmd/src/repos_list.go | 202 +++++++++++++++++++++++++++++-------- cmd/src/repos_list_test.go | 137 ++++++++++++++++++++++++- 2 files changed, 293 insertions(+), 46 deletions(-) diff --git a/cmd/src/repos_list.go b/cmd/src/repos_list.go index 575c48dda9..f887faa2e8 100644 --- a/cmd/src/repos_list.go +++ b/cmd/src/repos_list.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "flag" "fmt" "strings" @@ -9,6 +10,141 @@ import ( "github.com/sourcegraph/src-cli/internal/api" ) +type reposListOptions struct { + first int + query string + cloned bool + notCloned bool + indexed bool + notIndexed bool + orderBy string + descending bool +} + +type repositoriesListResult struct { + Data struct { + Repositories struct { + Nodes []Repository `json:"nodes"` + } `json:"repositories"` + } `json:"data"` + Errors []json.RawMessage `json:"errors,omitempty"` +} + +func listRepositories(ctx context.Context, client api.Client, params reposListOptions) ([]Repository, api.GraphQlErrors, error) { + query := `query Repositories( + $first: Int, + $query: String, + $cloned: Boolean, + $notCloned: Boolean, + $indexed: Boolean, + $notIndexed: Boolean, + $orderBy: RepositoryOrderBy, + $descending: Boolean, +) { + repositories( + first: $first, + query: $query, + cloned: $cloned, + notCloned: $notCloned, + indexed: $indexed, + notIndexed: $notIndexed, + orderBy: $orderBy, + descending: $descending, + ) { + nodes { + ...RepositoryFields + } + } +} +` + repositoryFragment + + var result repositoriesListResult + ok, err := client.NewRequest(query, map[string]any{ + "first": api.NullInt(params.first), + "query": api.NullString(params.query), + "cloned": params.cloned, + "notCloned": params.notCloned, + "indexed": params.indexed, + "notIndexed": params.notIndexed, + "orderBy": params.orderBy, + "descending": params.descending, + }).DoRaw(ctx, &result) + if err != nil || !ok { + return nil, nil, err + } + repos := result.Data.Repositories.Nodes + if len(result.Errors) == 0 { + return repos, nil, nil + } + + errors := api.NewGraphQlErrors(result.Errors) + if len(repos) > 0 { + return filterRepositoriesWithErrors(repos, errors), errors, nil + } + + return nil, nil, errors +} + +func filterRepositoriesWithErrors(repos []Repository, errors api.GraphQlErrors) []Repository { + if len(errors) == 0 || len(repos) == 0 { + return repos + } + + skip := make(map[int]struct{}, len(errors)) + for _, graphQLError := range errors { + index, ok := gqlRepositoryErrorIndex(graphQLError) + if !ok || index >= len(repos) { + continue + } + skip[index] = struct{}{} + } + + filtered := make([]Repository, 0, len(repos)) + for i, repo := range repos { + if _, ok := skip[i]; ok { + continue + } + filtered = append(filtered, repo) + } + + return filtered +} + +func gqlErrorPathString(pathSegment any) (string, bool) { + value, ok := pathSegment.(string) + return value, ok +} + +func gqlErrorIndex(pathSegment any) (int, bool) { + switch value := pathSegment.(type) { + case float64: + index := int(value) + return index, float64(index) == value && index >= 0 + case int: + return value, value >= 0 + default: + return 0, false + } +} + +func gqlRepositoryErrorIndex(graphQLError *api.GraphQlError) (int, bool) { + path, err := graphQLError.Path() + if err != nil || len(path) < 3 { + return 0, false + } + + pathRoot, ok := gqlErrorPathString(path[0]) + if !ok || pathRoot != "repositories" { + return 0, false + } + pathCollection, ok := gqlErrorPathString(path[1]) + if !ok || pathCollection != "nodes" { + return 0, false + } + + return gqlErrorIndex(path[2]) +} + func init() { usage := ` Examples: @@ -64,33 +200,6 @@ Examples: return err } - query := `query Repositories( - $first: Int, - $query: String, - $cloned: Boolean, - $notCloned: Boolean, - $indexed: Boolean, - $notIndexed: Boolean, - $orderBy: RepositoryOrderBy, - $descending: Boolean, -) { - repositories( - first: $first, - query: $query, - cloned: $cloned, - notCloned: $notCloned, - indexed: $indexed, - notIndexed: $notIndexed, - orderBy: $orderBy, - descending: $descending, - ) { - nodes { - ...RepositoryFields - } - } -} -` + repositoryFragment - var orderBy string switch *orderByFlag { case "name": @@ -101,25 +210,22 @@ Examples: return fmt.Errorf("invalid -order-by flag value: %q", *orderByFlag) } - var result struct { - Repositories struct { - Nodes []Repository - } - } - if ok, err := client.NewRequest(query, map[string]any{ - "first": api.NullInt(*firstFlag), - "query": api.NullString(*queryFlag), - "cloned": *clonedFlag, - "notCloned": *notClonedFlag, - "indexed": *indexedFlag, - "notIndexed": *notIndexedFlag, - "orderBy": orderBy, - "descending": *descendingFlag, - }).Do(context.Background(), &result); err != nil || !ok { + // if we get repos and errors during a listing, we consider the errors as warnings and the data partially complete + repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{ + first: *firstFlag, + query: *queryFlag, + cloned: *clonedFlag, + notCloned: *notClonedFlag, + indexed: *indexedFlag, + notIndexed: *notIndexedFlag, + orderBy: orderBy, + descending: *descendingFlag, + }) + if err != nil { return err } - for _, repo := range result.Repositories.Nodes { + for _, repo := range repos { if *namesWithoutHostFlag { firstSlash := strings.Index(repo.Name, "/") fmt.Println(repo.Name[firstSlash+len("/"):]) @@ -130,6 +236,16 @@ Examples: return err } } + if len(warnings) > 0 { + if *verbose { + fmt.Fprintf(flagSet.Output(), "warning: %d errors during listing:\n", len(warnings)) + for _, warning := range warnings { + fmt.Fprintln(flagSet.Output(), warning.Error()) + } + } else { + fmt.Fprintf(flagSet.Output(), "warning: %d errors during listing; rerun with -v to inspect them\n", len(warnings)) + } + } return nil } diff --git a/cmd/src/repos_list_test.go b/cmd/src/repos_list_test.go index 2866c95f4c..091272d296 100644 --- a/cmd/src/repos_list_test.go +++ b/cmd/src/repos_list_test.go @@ -83,7 +83,7 @@ func TestListRepositoriesSkipsRepositoryWhenDefaultBranchCannotBeResolved(t *tes Return(true, nil). Once() - repos, skipped, err := listRepositories(context.Background(), client, reposListOptions{ + repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{ first: 1000, cloned: true, notCloned: true, @@ -93,9 +93,140 @@ func TestListRepositoriesSkipsRepositoryWhenDefaultBranchCannotBeResolved(t *tes }) require.NoError(t, err) require.Len(t, repos, 1) - require.Len(t, skipped, 1) + require.Len(t, warnings, 1) require.Equal(t, "github.com/sourcegraph/ok", repos[0].Name) - require.Equal(t, "github.com/sourcegraph/broken", skipped[0].Name) + require.ErrorContains(t, warnings[0], "failed to resolve HEAD for github.com/sourcegraph/broken") + client.AssertExpectations(t) + request.AssertExpectations(t) +} + +func TestListRepositoriesFiltersNodeScopedFieldErrors(t *testing.T) { + client := new(mockapi.Client) + request := &mockapi.Request{Response: `{ + "data": { + "repositories": { + "nodes": [ + { + "id": "UmVwb3NpdG9yeTox", + "name": "github.com/sourcegraph/ok", + "url": "/github.com/sourcegraph/ok", + "description": "", + "language": "Go", + "createdAt": "2026-03-31T00:00:00Z", + "updatedAt": null, + "externalRepository": { + "id": "RXh0ZXJuYWxSZXBvc2l0b3J5OjE=", + "serviceType": "github", + "serviceID": "https://github.com/" + }, + "defaultBranch": { + "name": "refs/heads/main", + "displayName": "main" + }, + "viewerCanAdminister": false, + "keyValuePairs": [] + } + ] + } + }, + "errors": [ + { + "message": "viewer permissions unavailable", + "path": ["repositories", "nodes", 0, "viewerCanAdminister"] + } + ] + }`} + + client.On( + "NewRequest", + mock.MatchedBy(func(query string) bool { + return strings.Contains(query, "viewerCanAdminister") + }), + mock.Anything, + ).Return(request).Once() + + request.On("DoRaw", context.Background(), mock.Anything). + Return(true, nil). + Once() + + repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{ + first: 1000, + cloned: true, + notCloned: true, + indexed: true, + notIndexed: false, + orderBy: "REPOSITORY_NAME", + }) + require.NoError(t, err) + require.Empty(t, repos) + require.Len(t, warnings, 1) + require.ErrorContains(t, warnings[0], "viewer permissions unavailable") + client.AssertExpectations(t) + request.AssertExpectations(t) +} + +func TestListRepositoriesReturnsWarningsWithDataForNonNodeErrors(t *testing.T) { + client := new(mockapi.Client) + request := &mockapi.Request{Response: `{ + "data": { + "repositories": { + "nodes": [ + { + "id": "UmVwb3NpdG9yeTox", + "name": "github.com/sourcegraph/ok", + "url": "/github.com/sourcegraph/ok", + "description": "", + "language": "Go", + "createdAt": "2026-03-31T00:00:00Z", + "updatedAt": null, + "externalRepository": { + "id": "RXh0ZXJuYWxSZXBvc2l0b3J5OjE=", + "serviceType": "github", + "serviceID": "https://github.com/" + }, + "defaultBranch": { + "name": "refs/heads/main", + "displayName": "main" + }, + "viewerCanAdminister": false, + "keyValuePairs": [] + } + ] + } + }, + "errors": [ + { + "message": "listing timed out", + "path": ["repositories"] + } + ] + }`} + + client.On( + "NewRequest", + mock.MatchedBy(func(query string) bool { + return strings.Contains(query, "defaultBranch") + }), + mock.Anything, + ).Return(request).Once() + + request.On("DoRaw", context.Background(), mock.Anything). + Return(true, nil). + Once() + + repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{ + first: 1000, + cloned: true, + notCloned: true, + indexed: true, + notIndexed: false, + orderBy: "REPOSITORY_NAME", + }) + require.NoError(t, err) + require.Len(t, repos, 1) + require.Len(t, warnings, 1) + require.Equal(t, "github.com/sourcegraph/ok", repos[0].Name) + require.ErrorContains(t, warnings[0], "listing timed out") client.AssertExpectations(t) request.AssertExpectations(t) } From 65b9840cef8ae51665a3907cd39739ca4eef7529 Mon Sep 17 00:00:00 2001 From: William Bezuidenhout Date: Wed, 1 Apr 2026 16:41:22 +0200 Subject: [PATCH 4/4] add comment --- cmd/src/repos_list.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/src/repos_list.go b/cmd/src/repos_list.go index f887faa2e8..d11f40d469 100644 --- a/cmd/src/repos_list.go +++ b/cmd/src/repos_list.go @@ -30,6 +30,9 @@ type repositoriesListResult struct { Errors []json.RawMessage `json:"errors,omitempty"` } +// listRepositories returns the repositories from the response, any GraphQL +// errors returned alongside data (should be treated as warnings), and +// a hard error when the query fails without usable repository data. func listRepositories(ctx context.Context, client api.Client, params reposListOptions) ([]Repository, api.GraphQlErrors, error) { query := `query Repositories( $first: Int,