gh pr list response status code override if using --label parameter

[guitarrapc]

Describe the bug

gh pr list will return non-0 exit status if command failed for any reason, GH_REPO is invalid, GitHub response 500 because incident, etc. However gh pr list --label will override command status code with 0 even HTTP status is 500 or any.

$ gh version
gh version 2.47.0 (2024-04-03)
https://github.com/cli/cli/releases/tag/v2.47.0

Steps to reproduce the behavior

1. Type this 'GH_REPO=cli/cli123546 gh pr list --label foo"
2. View the output 'no pull requests match your search in cli/cli123546'
3. You will find exit code is 0 even command failed.

Expected vs actual behavior

actual

$ GH_REPO=cli/cli123546 gh pr list
GraphQL: Could not resolve to a Repository with the name 'cli/cli123546'. (repository)
$ echo $?
1
$ GH_REPO=cli/cli123546 gh pr list --label foo
no pull requests match your search in cli/cli123546
$ echo $?
0

Expected

gh pr list --label <foo> exit code should same as gh pr list when gh pr lit it self failed.

$ GH_REPO=cli/cli123546 gh pr list
GraphQL: Could not resolve to a Repository with the name 'cli/cli123546'. (repository)
$ echo $?
1
$ GH_REPO=cli/cli123546 gh pr list --label foo
GraphQL: Could not resolve to a Repository with the name 'cli/cli123546'. (repository)
$ echo $?
1

Even using --label or not, gh pr list command's oroginal exit-code SHOULD respected. --label override exit-code with 0 cause unexpected situations. I've notice this issue with incident that happen an hour ago, my command gh pr list --label foobar actually show following output, but exit-code was 0 and break workflows.

HTTP 500 (https://api.github.com/graphql)
  GraphQL: Something went wrong while executing your query. Please include `FOO:BAR:HOGE:PIYO:TAKO:YAKI` when reporting this issue.
  branches:

Logs

Paste the activity from your command line. Redact if needed.

[babakks]

I can confirm this.

@guitarrapc Note that the --label option is not literally masking/overriding the exit code. The behaviors are different because depending on a set of conditions (see the code below and note the len(filters.labels) > 0 at the end it) the CLI decides to call a different API endpoint, which is PullRequestSearch (instead of the PullRequestList), and that never returns an error because it's basically a search:

cli/pkg/cmd/pr/list/http.go

Lines 12 to 14 in 3be7a02

	func shouldUseSearch(filters prShared.FilterOptions) bool {
	return filters.Draft != nil || filters.Author != "" || filters.Assignee != "" || filters.Search != "" || len(filters.Labels) > 0
	}

Anyway, I agree with @guitarrapc that this should be improved. Since it's a requirement for the pr list command to be called with an explicit repo (correct me if I'm wrong), I think the fix is to check for the repo being there before attempting to call the search endpoint; I'm speaking of something like this:

// (pkg/cmd/pr/list/http.go)
func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) {
	// Check repo exists:
	client := api.NewClientFromHTTP(httpClient)
	if exists, err := api.RepoExists(client, repo); err != nil {
		return nil, err
	} else if !exists {
		return nil, fmt.Errorf("repository not found: %s/%s", repo.RepoOwner(), repo.RepoName())
	}
	
	// ...
}

If this is also seen as an issue by the maintainers, I'd love to submit a PR.

[guitarrapc]

@babakks Thank you for clarifying the root cause. I think your suggested fix will address the issue. Currently, there are two situations where the issue arises, but both are expected to be resolved by your solution:

1. Even when the repository is incorrect, using --labels never returns an error.
2. Even during a GitHub Incident when the PR API is unstable, using --labels never returns an error.