From e85f5a209483c74ad29a6e3b73cac300f6e57a45 Mon Sep 17 00:00:00 2001
From: Thorsten Ball <mrnugget@gmail.com>
Date: Fri, 3 Apr 2020 16:12:33 +0200
Subject: [PATCH] Skip repositories being cloned in action exec instead of
 returning error

This fixes https://github.com/sourcegraph/sourcegraph/issues/9149

When a repository is being cloned while we execute the `scopeQuery` of
an action, the GraphQL query returns something like this:

    {
      "errors": [
        {
          "message": "repository does not exist: github.com/sourcegraphtest/alwayscloningtest",
          "path": [ "search", "results", "results", 2, "defaultBranch" ]
        }
      ],
      "data": {
        "search": {
          "results": {
            "results": [
              {
                "__typename": "Repository",
                "id": "UmVwb3NpdG9yeTox",
                "name": "github.com/sourcegraph/automation-testing",
                "defaultBranch": { "name": "refs/heads/master", "target": { "oid": "3a0d12026c1349c4aefb712145433cc26c330a06" }
                }
              },
              {
                "__typename": "Repository",
                "id": "UmVwb3NpdG9yeTozOTU=",
                "name": "github.com/sourcegraphtest/AlwaysCloningTest",
                "defaultBranch": null
              }
            ]
          }
        }
      }
    }

There is an error for `AlwaysCloningTest` but `AlwaysCloningTest` is
also being returned in the data, except that its `defaultBranch` is
`null`.

So instead of returning an error when one of the repositories is being
cloned and doesn't have a default branch, we simply check for the
existence of the `defaultBranch` attribute and if it's not there, we
skip the repository.
---
 cmd/src/actions_exec.go | 57 +++++++++++++++++++++++++++++------------
 cmd/src/cmd.go          |  7 +++++
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/cmd/src/actions_exec.go b/cmd/src/actions_exec.go
index e06e18b62f..949dd54481 100644
--- a/cmd/src/actions_exec.go
+++ b/cmd/src/actions_exec.go
@@ -465,25 +465,32 @@ query ActionRepos($query: String!) {
 `
 	type Repository struct {
 		ID, Name      string
-		DefaultBranch struct {
+		DefaultBranch *struct {
 			Name   string
 			Target struct{ OID string }
 		}
 	}
 	var result struct {
-		Search struct {
-			Results struct {
-				Results []struct {
-					Typename      string `json:"__typename"`
-					ID, Name      string
-					DefaultBranch struct {
-						Name   string
-						Target struct{ OID string }
+		Data struct {
+			Search struct {
+				Results struct {
+					Results []struct {
+						Typename      string `json:"__typename"`
+						ID, Name      string
+						DefaultBranch *struct {
+							Name   string
+							Target struct{ OID string }
+						}
+						Repository Repository `json:"repository"`
 					}
-					Repository Repository `json:"repository"`
 				}
 			}
-		}
+		} `json:"data,omitempty"`
+
+		Errors []struct {
+			Message string
+			Path    []interface{}
+		} `json:"errors,omitempty"`
 	}
 
 	if err := (&apiRequest{
@@ -491,14 +498,32 @@ query ActionRepos($query: String!) {
 		vars: map[string]interface{}{
 			"query": scopeQuery,
 		},
-		result: &result,
+		// Do not unpack errors and return error. Instead we want to go through
+		// the results and check whether they're complete.
+		// If we don't do this and the query returns an error for _one_
+		// repository because that is still cloning, we don't get any repositories.
+		// Instead we simply want to skip those repositories that are still
+		// being cloned.
+		dontUnpackErrors: true,
+		result:           &result,
 	}).do(); err != nil {
-		return nil, nil, err
+
+		// Ignore exitCodeError with error == nil, because we explicitly set
+		// dontUnpackErrors, which can lead to an empty exitCodeErr being
+		// returned.
+		exitCodeErr, ok := err.(*exitCodeError)
+		if !ok {
+			return nil, nil, err
+		}
+		if exitCodeErr.error != nil {
+			return nil, nil, exitCodeErr
+		}
 	}
 
 	skipped := []string{}
 	reposByID := map[string]ActionRepo{}
-	for _, searchResult := range result.Search.Results.Results {
+	for _, searchResult := range result.Data.Search.Results.Results {
+
 		var repo Repository
 		if searchResult.Repository.ID != "" {
 			repo = searchResult.Repository
@@ -510,8 +535,8 @@ query ActionRepos($query: String!) {
 			}
 		}
 
-		if repo.DefaultBranch.Name == "" {
-			skipped = append(skipped, repo.Name)
+		if repo.DefaultBranch == nil || repo.DefaultBranch.Name == "" {
+			skipped = append(skipped, searchResult.Repository.Name)
 			continue
 		}
 
diff --git a/cmd/src/cmd.go b/cmd/src/cmd.go
index c84ab10963..5589a31c5a 100644
--- a/cmd/src/cmd.go
+++ b/cmd/src/cmd.go
@@ -124,6 +124,13 @@ type exitCodeError struct {
 	exitCode int
 }
 
+func (e *exitCodeError) Error() string {
+	if e.error != nil {
+		return fmt.Sprintf("%s (exit code: %d)", e.error, e.exitCode)
+	}
+	return fmt.Sprintf("exit code: %d", e.exitCode)
+}
+
 const (
 	graphqlErrorsExitCode = 2
 )