fix/repo-list: print partial data when there are graphql errors

[burmudar]

Closes CPL-297

Update src repo list to not fail the whole listing if only some repositories failed.

Changes

• If we have data and errors in a result, treat the errors as warnings. This allows us to print partial results
• if we have NO data and errors, treat the errors as fatal errors

Test plan

• Unit test
• Made local SG return partial results and tested src repo list

./src -v repo list
bitbucket.org/sourcegraph-testing/cse_k8s_test
bitbucket.org/sourcegraph-testing/neko
bitbucket.org/sourcegraph-testing/pgx
bitbucket.org/sourcegraph-testing/prometheus
bitbucket.org/sourcegraph-testing/sourcegraph
bitbucket.org/sourcegraph-testing/src-cli
bitbucket.org/sourcegraph-testing/src-cli-fork-00
github.com/hashicorp/errwrap
github.com/hashicorp/go-multierror
github.com/sgtest/weird
github.com/sourcegraph-testing/etcd
github.com/sourcegraph-testing/nacelle
github.com/sourcegraph-testing/nacelle-config
github.com/sourcegraph-testing/nacelle-service
github.com/sourcegraph-testing/tidb
github.com/sourcegraph-testing/titan
github.com/sourcegraph-testing/zap
github.com/sourcegraph/code-intel-extensions
github.com/sourcegraph/sourcegraph
gitlab.sgdev.org/batch-changes-testing/audrey-test
gitlab.sgdev.org/batch-changes-testing/code-signing
warning: 1 errors during listing:
repositories.nodes.18.defaultBranch - simulated default branch failure
{
  "message": "simulated default branch failure",
  "path": [
    "repositories",
    "nodes",
    18,
    "defaultBranch"
  ]
}

[bahrmichael]

Looks good to me though I would opt for verbose-by-default if the errors are severe enough.

Amp found the issue below. Feel free to ignore if it's not valid.

High: cmd/src/repos_list.go#L83-L113 treats any error whose path starts with repositories.nodes[i] as a reason to drop the entire repo row. That is too aggressive for GraphQL partial-data semantics: this query asks for many fields in the shared fragment, but the default command only prints Name. If defaultBranch, viewerCanAdminister, or keyValuePairs fails for one repo, the repo name/id/url can still be valid and should still be listed with a warning. Instead, the PR silently omits that repo. This also means -first can return fewer repos than requested because filtering happens after the server already applied ordering and limits. The new test at cmd/src/repos_list_test.go#L103-L166 codifies the problematic behavior by expecting a repo to disappear when only viewerCanAdminister errors.

[burmudar]

Looks good to me though I would opt for verbose-by-default if the errors are severe enough.

Amp found the issue below. Feel free to ignore if it's not valid.

High: cmd/src/repos_list.go#L83-L113 treats any error whose path starts with repositories.nodes[i] as a reason to drop the entire repo row. That is too aggressive for GraphQL partial-data semantics: this query asks for many fields in the shared fragment, but the default command only prints Name. If defaultBranch, viewerCanAdminister, or keyValuePairs fails for one repo, the repo name/id/url can still be valid and should still be listed with a warning. Instead, the PR silently omits that repo. This also means -first can return fewer repos than requested because filtering happens after the server already applied ordering and limits. The new test at cmd/src/repos_list_test.go#L103-L166 codifies the problematic behavior by expecting a repo to disappear when only viewerCanAdminister errors.

Fixed it.

It will now always print the repos and will print that there are warnings if we got partial data + errors. If we got no data and errors it means it's a hard failure and then we print that.
If we are in verbose mode, we will print the data + print the error message along with raw json of the error incase it has more detail.