Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions platform/git/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (r *Repo) outputOf(ctx context.Context, args ...string) (string, error) {
if message == "" {
message = err.Error()
}
return "", fmt.Errorf("git %s: %s", strings.Join(args, " "), message)
return "", fmt.Errorf("git %s: %w", strings.Join(args, " "), gitexec.CommandFailure(ctx, args, message, err))
}
return string(out), nil
}
Expand All @@ -269,7 +269,7 @@ func SetConfig(ctx context.Context, path, key, value string) error {
if message == "" {
message = err.Error()
}
return fmt.Errorf("git config %s: %s", key, message)
return fmt.Errorf("git config %s: %w", key, gitexec.CommandFailure(ctx, []string{"config"}, message, err))
}
return nil
}
68 changes: 68 additions & 0 deletions platform/git/repo/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package gitrepo
import (
"context"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -114,3 +115,70 @@ func TestNewRepo_RejectsAnIncompleteConfiguration(t *testing.T) {
})
}
}

func TestRunRaw_PreservesCommandFailure(t *testing.T) {
repo, err := NewRepo(RepoConfig{
Git: gitexectest.Git(t),
Path: t.TempDir(),
RemoteURL: "unused",
Target: "main",
})
require.NoError(t, err)

_, err = repo.RunRaw(context.Background(), "not-a-git-command")

var commandErr *gitexec.CommandError
require.ErrorAs(t, err, &commandErr)
assert.Equal(t, "not-a-git-command", commandErr.Operation())
var exitErr *exec.ExitError
assert.ErrorAs(t, err, &exitErr)
}

func TestSetConfig_PreservesCommandFailure(t *testing.T) {
t.Setenv("GIT_EXECUTABLE", gitexectest.Git(t))
err := SetConfig(context.Background(), t.TempDir(), "user.name", "Test")

var commandErr *gitexec.CommandError
require.ErrorAs(t, err, &commandErr)
assert.Equal(t, "config", commandErr.Operation())
var exitErr *exec.ExitError
assert.ErrorAs(t, err, &exitErr)
}

func TestCommands_PreserveCancellation(t *testing.T) {
git := gitexectest.Git(t)
t.Setenv("GIT_EXECUTABLE", git)
repo, err := NewRepo(RepoConfig{
Git: git,
Path: t.TempDir(),
RemoteURL: "unused",
Target: "main",
})
require.NoError(t, err)

for _, tt := range []struct {
name string
run func(context.Context) error
}{
{
name: "raw command",
run: func(ctx context.Context) error {
_, err := repo.RunRaw(ctx, "status")
return err
},
},
{
name: "set config",
run: func(ctx context.Context) error {
return SetConfig(ctx, repo.cfg.Path, "user.name", "Test")
},
},
} {
t.Run(tt.name, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()

assert.ErrorIs(t, tt.run(ctx), context.Canceled)
})
}
}
2 changes: 2 additions & 0 deletions service/submitqueue/orchestrator/server/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ go_library(
"//platform/buildkite:go_default_library",
"//platform/errs:go_default_library",
"//platform/errs/generic:go_default_library",
"//platform/errs/git:go_default_library",
"//platform/errs/http:go_default_library",
"//platform/errs/mysql:go_default_library",
"//platform/extension/consumergate:go_default_library",
Expand Down Expand Up @@ -123,6 +124,7 @@ go_test(
"//platform/base/change:go_default_library",
"//platform/errs:go_default_library",
"//platform/git/exectest:go_default_library",
"//platform/git/repo:go_default_library",
"//platform/githubactions:go_default_library",
"//submitqueue/entity:go_default_library",
"//submitqueue/extension/buildrunner:go_default_library",
Expand Down
2 changes: 2 additions & 0 deletions service/submitqueue/orchestrator/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
pb "github.com/uber/submitqueue/api/submitqueue/orchestrator/protopb"
"github.com/uber/submitqueue/platform/errs"
genericerrs "github.com/uber/submitqueue/platform/errs/generic"
giterrs "github.com/uber/submitqueue/platform/errs/git"
httperrs "github.com/uber/submitqueue/platform/errs/http"
mysqlerrs "github.com/uber/submitqueue/platform/errs/mysql"
"github.com/uber/submitqueue/platform/extension/consumergate"
Expand Down Expand Up @@ -430,6 +431,7 @@ func defaultProfilesConfig() profilesConfig {
func primaryErrorClassifiers() []errs.Classifier {
return []errs.Classifier{
genericerrs.Classifier,
giterrs.Classifier,
// HTTP must precede MySQL's broad net.Error match to retain dependency attribution.
httperrs.Classifier,
mysqlerrs.Classifier,
Expand Down
62 changes: 62 additions & 0 deletions service/submitqueue/orchestrator/server/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ import (
"fmt"
"io"
"net/http"
"os"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/uber/submitqueue/platform/errs"
gitrepo "github.com/uber/submitqueue/platform/git/repo"
"github.com/uber/submitqueue/platform/githubactions"
)

Expand Down Expand Up @@ -76,6 +79,65 @@ func TestPrimaryErrorClassifiers_Storage(t *testing.T) {
assert.False(t, errs.IsDependencyError(err))
}

func TestPrimaryErrorClassifiers_GitFetch(t *testing.T) {
processor := errs.NewClassifierProcessor(primaryErrorClassifiers()...)
for _, tt := range []struct {
name string
diagnostic string
cancelled bool
retryable bool
dependency bool
}{
{
name: "temporary DNS failure",
diagnostic: "fatal: Could not resolve host: git.example.invalid",
retryable: true,
dependency: true,
},
{
name: "authentication failure",
diagnostic: "fatal: Authentication failed for 'https://git.example.invalid/repo'",
dependency: true,
},
{
name: "unknown failure",
diagnostic: "fatal: unexpected failure",
dependency: true,
},
{
name: "shutdown",
cancelled: true,
retryable: true,
},
} {
t.Run(tt.name, func(t *testing.T) {
dir := t.TempDir()
git := filepath.Join(dir, "git")
diagnostic := "'" + strings.ReplaceAll(tt.diagnostic, "'", "'\\''") + "'"
script := "#!/bin/sh\nprintf '%s\\n' " + diagnostic + " >&2\nexit 128\n"
require.NoError(t, os.WriteFile(git, []byte(script), 0o755))
repo, err := gitrepo.NewRepo(gitrepo.RepoConfig{
Git: git,
Path: dir,
RemoteURL: "https://git.example.invalid/repo",
Target: "main",
})
require.NoError(t, err)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
if tt.cancelled {
cancel()
}
err = repo.FetchTarget(ctx)
require.Error(t, err)
out := processor.Process(fmt.Errorf("fetch target: %w", err))
assert.Equal(t, tt.retryable, errs.IsRetryable(out))
assert.Equal(t, tt.dependency, errs.IsDependencyError(out))
})
}
}

type roundTripFunc func(*http.Request) (*http.Response, error)

func (f roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) {
Expand Down
Loading