Skip to content

Commit 05ed4b1

Browse files
burmudarburmudar
committed
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
1 parent 9fcfd4c commit 05ed4b1

File tree

2 files changed

+293
-46
lines changed

2 files changed

+293
-46
lines changed

‎cmd/src/repos_list.go‎

Lines changed: 159 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,149 @@ package main
22

33
import (
44
"context"
5+
"encoding/json"
56
"flag"
67
"fmt"
78
"strings"
89

910
"github.com/sourcegraph/src-cli/internal/api"
1011
)
1112

13+
type reposListOptions struct {
14+
first int
15+
query string
16+
cloned bool
17+
notCloned bool
18+
indexed bool
19+
notIndexed bool
20+
orderBy string
21+
descending bool
22+
}
23+
24+
type repositoriesListResult struct {
25+
Data struct {
26+
Repositories struct {
27+
Nodes []Repository `json:"nodes"`
28+
} `json:"repositories"`
29+
} `json:"data"`
30+
Errors []json.RawMessage `json:"errors,omitempty"`
31+
}
32+
33+
func listRepositories(ctx context.Context, client api.Client, params reposListOptions) ([]Repository, api.GraphQlErrors, error) {
34+
query := `query Repositories(
35+
$first: Int,
36+
$query: String,
37+
$cloned: Boolean,
38+
$notCloned: Boolean,
39+
$indexed: Boolean,
40+
$notIndexed: Boolean,
41+
$orderBy: RepositoryOrderBy,
42+
$descending: Boolean,
43+
) {
44+
repositories(
45+
first: $first,
46+
query: $query,
47+
cloned: $cloned,
48+
notCloned: $notCloned,
49+
indexed: $indexed,
50+
notIndexed: $notIndexed,
51+
orderBy: $orderBy,
52+
descending: $descending,
53+
) {
54+
nodes {
55+
...RepositoryFields
56+
}
57+
}
58+
}
59+
` + repositoryFragment
60+
61+
var result repositoriesListResult
62+
ok, err := client.NewRequest(query, map[string]any{
63+
"first": api.NullInt(params.first),
64+
"query": api.NullString(params.query),
65+
"cloned": params.cloned,
66+
"notCloned": params.notCloned,
67+
"indexed": params.indexed,
68+
"notIndexed": params.notIndexed,
69+
"orderBy": params.orderBy,
70+
"descending": params.descending,
71+
}).DoRaw(ctx, &result)
72+
if err != nil || !ok {
73+
return nil, nil, err
74+
}
75+
repos := result.Data.Repositories.Nodes
76+
if len(result.Errors) == 0 {
77+
return repos, nil, nil
78+
}
79+
80+
errors := api.NewGraphQlErrors(result.Errors)
81+
if len(repos) > 0 {
82+
return filterRepositoriesWithErrors(repos, errors), errors, nil
83+
}
84+
85+
return nil, nil, errors
86+
}
87+
88+
func filterRepositoriesWithErrors(repos []Repository, errors api.GraphQlErrors) []Repository {
89+
if len(errors) == 0 || len(repos) == 0 {
90+
return repos
91+
}
92+
93+
skip := make(map[int]struct{}, len(errors))
94+
for _, graphQLError := range errors {
95+
index, ok := gqlRepositoryErrorIndex(graphQLError)
96+
if !ok || index >= len(repos) {
97+
continue
98+
}
99+
skip[index] = struct{}{}
100+
}
101+
102+
filtered := make([]Repository, 0, len(repos))
103+
for i, repo := range repos {
104+
if _, ok := skip[i]; ok {
105+
continue
106+
}
107+
filtered = append(filtered, repo)
108+
}
109+
110+
return filtered
111+
}
112+
113+
func gqlErrorPathString(pathSegment any) (string, bool) {
114+
value, ok := pathSegment.(string)
115+
return value, ok
116+
}
117+
118+
func gqlErrorIndex(pathSegment any) (int, bool) {
119+
switch value := pathSegment.(type) {
120+
case float64:
121+
index := int(value)
122+
return index, float64(index) == value && index >= 0
123+
case int:
124+
return value, value >= 0
125+
default:
126+
return 0, false
127+
}
128+
}
129+
130+
func gqlRepositoryErrorIndex(graphQLError *api.GraphQlError) (int, bool) {
131+
path, err := graphQLError.Path()
132+
if err != nil || len(path) < 3 {
133+
return 0, false
134+
}
135+
136+
pathRoot, ok := gqlErrorPathString(path[0])
137+
if !ok || pathRoot != "repositories" {
138+
return 0, false
139+
}
140+
pathCollection, ok := gqlErrorPathString(path[1])
141+
if !ok || pathCollection != "nodes" {
142+
return 0, false
143+
}
144+
145+
return gqlErrorIndex(path[2])
146+
}
147+
12148
func init() {
13149
usage := `
14150
Examples:
@@ -64,33 +200,6 @@ Examples:
64200
return err
65201
}
66202

67-
query := `query Repositories(
68-
$first: Int,
69-
$query: String,
70-
$cloned: Boolean,
71-
$notCloned: Boolean,
72-
$indexed: Boolean,
73-
$notIndexed: Boolean,
74-
$orderBy: RepositoryOrderBy,
75-
$descending: Boolean,
76-
) {
77-
repositories(
78-
first: $first,
79-
query: $query,
80-
cloned: $cloned,
81-
notCloned: $notCloned,
82-
indexed: $indexed,
83-
notIndexed: $notIndexed,
84-
orderBy: $orderBy,
85-
descending: $descending,
86-
) {
87-
nodes {
88-
...RepositoryFields
89-
}
90-
}
91-
}
92-
` + repositoryFragment
93-
94203
var orderBy string
95204
switch *orderByFlag {
96205
case "name":
@@ -101,25 +210,22 @@ Examples:
101210
return fmt.Errorf("invalid -order-by flag value: %q", *orderByFlag)
102211
}
103212

104-
var result struct {
105-
Repositories struct {
106-
Nodes []Repository
107-
}
108-
}
109-
if ok, err := client.NewRequest(query, map[string]any{
110-
"first": api.NullInt(*firstFlag),
111-
"query": api.NullString(*queryFlag),
112-
"cloned": *clonedFlag,
113-
"notCloned": *notClonedFlag,
114-
"indexed": *indexedFlag,
115-
"notIndexed": *notIndexedFlag,
116-
"orderBy": orderBy,
117-
"descending": *descendingFlag,
118-
}).Do(context.Background(), &result); err != nil || !ok {
213+
// if we get repos and errors during a listing, we consider the errors as warnings and the data partially complete
214+
repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{
215+
first: *firstFlag,
216+
query: *queryFlag,
217+
cloned: *clonedFlag,
218+
notCloned: *notClonedFlag,
219+
indexed: *indexedFlag,
220+
notIndexed: *notIndexedFlag,
221+
orderBy: orderBy,
222+
descending: *descendingFlag,
223+
})
224+
if err != nil {
119225
return err
120226
}
121227

122-
for _, repo := range result.Repositories.Nodes {
228+
for _, repo := range repos {
123229
if *namesWithoutHostFlag {
124230
firstSlash := strings.Index(repo.Name, "/")
125231
fmt.Println(repo.Name[firstSlash+len("/"):])
@@ -130,6 +236,16 @@ Examples:
130236
return err
131237
}
132238
}
239+
if len(warnings) > 0 {
240+
if *verbose {
241+
fmt.Fprintf(flagSet.Output(), "warning: %d errors during listing:\n", len(warnings))
242+
for _, warning := range warnings {
243+
fmt.Fprintln(flagSet.Output(), warning.Error())
244+
}
245+
} else {
246+
fmt.Fprintf(flagSet.Output(), "warning: %d errors during listing; rerun with -v to inspect them\n", len(warnings))
247+
}
248+
}
133249
return nil
134250
}
135251

‎cmd/src/repos_list_test.go‎

Lines changed: 134 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func TestListRepositoriesSkipsRepositoryWhenDefaultBranchCannotBeResolved(t *tes
8383
Return(true, nil).
8484
Once()
8585

86-
repos, skipped, err := listRepositories(context.Background(), client, reposListOptions{
86+
repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{
8787
first: 1000,
8888
cloned: true,
8989
notCloned: true,
@@ -93,9 +93,140 @@ func TestListRepositoriesSkipsRepositoryWhenDefaultBranchCannotBeResolved(t *tes
9393
})
9494
require.NoError(t, err)
9595
require.Len(t, repos, 1)
96-
require.Len(t, skipped, 1)
96+
require.Len(t, warnings, 1)
9797
require.Equal(t, "github.com/sourcegraph/ok", repos[0].Name)
98-
require.Equal(t, "github.com/sourcegraph/broken", skipped[0].Name)
98+
require.ErrorContains(t, warnings[0], "failed to resolve HEAD for github.com/sourcegraph/broken")
99+
client.AssertExpectations(t)
100+
request.AssertExpectations(t)
101+
}
102+
103+
func TestListRepositoriesFiltersNodeScopedFieldErrors(t *testing.T) {
104+
client := new(mockapi.Client)
105+
request := &mockapi.Request{Response: `{
106+
"data": {
107+
"repositories": {
108+
"nodes": [
109+
{
110+
"id": "UmVwb3NpdG9yeTox",
111+
"name": "github.com/sourcegraph/ok",
112+
"url": "/github.com/sourcegraph/ok",
113+
"description": "",
114+
"language": "Go",
115+
"createdAt": "2026-03-31T00:00:00Z",
116+
"updatedAt": null,
117+
"externalRepository": {
118+
"id": "RXh0ZXJuYWxSZXBvc2l0b3J5OjE=",
119+
"serviceType": "github",
120+
"serviceID": "https://github.com/"
121+
},
122+
"defaultBranch": {
123+
"name": "refs/heads/main",
124+
"displayName": "main"
125+
},
126+
"viewerCanAdminister": false,
127+
"keyValuePairs": []
128+
}
129+
]
130+
}
131+
},
132+
"errors": [
133+
{
134+
"message": "viewer permissions unavailable",
135+
"path": ["repositories", "nodes", 0, "viewerCanAdminister"]
136+
}
137+
]
138+
}`}
139+
140+
client.On(
141+
"NewRequest",
142+
mock.MatchedBy(func(query string) bool {
143+
return strings.Contains(query, "viewerCanAdminister")
144+
}),
145+
mock.Anything,
146+
).Return(request).Once()
147+
148+
request.On("DoRaw", context.Background(), mock.Anything).
149+
Return(true, nil).
150+
Once()
151+
152+
repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{
153+
first: 1000,
154+
cloned: true,
155+
notCloned: true,
156+
indexed: true,
157+
notIndexed: false,
158+
orderBy: "REPOSITORY_NAME",
159+
})
160+
require.NoError(t, err)
161+
require.Empty(t, repos)
162+
require.Len(t, warnings, 1)
163+
require.ErrorContains(t, warnings[0], "viewer permissions unavailable")
164+
client.AssertExpectations(t)
165+
request.AssertExpectations(t)
166+
}
167+
168+
func TestListRepositoriesReturnsWarningsWithDataForNonNodeErrors(t *testing.T) {
169+
client := new(mockapi.Client)
170+
request := &mockapi.Request{Response: `{
171+
"data": {
172+
"repositories": {
173+
"nodes": [
174+
{
175+
"id": "UmVwb3NpdG9yeTox",
176+
"name": "github.com/sourcegraph/ok",
177+
"url": "/github.com/sourcegraph/ok",
178+
"description": "",
179+
"language": "Go",
180+
"createdAt": "2026-03-31T00:00:00Z",
181+
"updatedAt": null,
182+
"externalRepository": {
183+
"id": "RXh0ZXJuYWxSZXBvc2l0b3J5OjE=",
184+
"serviceType": "github",
185+
"serviceID": "https://github.com/"
186+
},
187+
"defaultBranch": {
188+
"name": "refs/heads/main",
189+
"displayName": "main"
190+
},
191+
"viewerCanAdminister": false,
192+
"keyValuePairs": []
193+
}
194+
]
195+
}
196+
},
197+
"errors": [
198+
{
199+
"message": "listing timed out",
200+
"path": ["repositories"]
201+
}
202+
]
203+
}`}
204+
205+
client.On(
206+
"NewRequest",
207+
mock.MatchedBy(func(query string) bool {
208+
return strings.Contains(query, "defaultBranch")
209+
}),
210+
mock.Anything,
211+
).Return(request).Once()
212+
213+
request.On("DoRaw", context.Background(), mock.Anything).
214+
Return(true, nil).
215+
Once()
216+
217+
repos, warnings, err := listRepositories(context.Background(), client, reposListOptions{
218+
first: 1000,
219+
cloned: true,
220+
notCloned: true,
221+
indexed: true,
222+
notIndexed: false,
223+
orderBy: "REPOSITORY_NAME",
224+
})
225+
require.NoError(t, err)
226+
require.Len(t, repos, 1)
227+
require.Len(t, warnings, 1)
228+
require.Equal(t, "github.com/sourcegraph/ok", repos[0].Name)
229+
require.ErrorContains(t, warnings[0], "listing timed out")
99230
client.AssertExpectations(t)
100231
request.AssertExpectations(t)
101232
}

0 commit comments

Comments
 (0)