Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
20 changes: 18 additions & 2 deletions internal/scm/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package github

import (
"bytes"
"context"
"encoding/json"
"errors"
Expand Down Expand Up @@ -151,8 +152,23 @@ func (h *Host) Available(ctx context.Context) error {
if h.host != "" {
authArgs = append(authArgs, "--hostname", h.host)
}
if err := h.cmd(ctx, "gh", authArgs...).Run(); err != nil {
return errors.New("gh CLI is not authenticated")
cmd := h.cmd(ctx, "gh", authArgs...)
var stderr bytes.Buffer
cmd.Stderr = &stderr
if err := cmd.Run(); err != nil {
// Keep timeout / missing-binary failures distinct from auth failure so a
// cancelled reconcile context is not reported as "log in again".
if ctx.Err() != nil {
return fmt.Errorf("gh auth status timed out: %w", ctx.Err())
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Outdated
if errors.Is(err, exec.ErrNotFound) {
return fmt.Errorf("gh CLI is not on PATH: %w", err)
}
detail := strings.TrimSpace(stderr.String())
if detail != "" {
return fmt.Errorf("gh CLI is not authenticated: %s: %w", detail, err)
}
return fmt.Errorf("gh CLI is not authenticated: %w", err)
}
return nil
}
Expand Down
67 changes: 67 additions & 0 deletions internal/scm/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package github
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -1513,6 +1514,72 @@ func TestAvailableFallsBackToUnscopedAuthWhenHostUnknown(t *testing.T) {
}
}

func TestAvailableReportsTimeoutInsteadOfAuthFailure(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithCancel(context.Background())
cancel()

host := New(githubTestCmdFactory(map[string]githubTestResponse{
"gh auth status": {},
}), func() bool { return true }, "", "")

err := host.Available(ctx)
if err == nil {
t.Fatal("Available() error = nil, want timeout error")
}
if !errors.Is(err, context.Canceled) {
t.Fatalf("Available() error = %v, want context.Canceled", err)
}
if !strings.Contains(err.Error(), "timed out") {
t.Fatalf("Available() error = %v, want timed out message", err)
}
if strings.Contains(err.Error(), "not authenticated") {
t.Fatalf("Available() error = %v, must not report auth failure on timeout", err)
}
}

func TestAvailableReportsMissingBinaryInsteadOfAuthFailure(t *testing.T) {
t.Parallel()

host := New(func(ctx context.Context, name string, args ...string) *exec.Cmd {
return exec.CommandContext(ctx, "no-mistakes-missing-gh-binary")
}, func() bool { return true }, "", "")

err := host.Available(context.Background())
if err == nil {
t.Fatal("Available() error = nil, want missing-binary error")
}
if !errors.Is(err, exec.ErrNotFound) {
t.Fatalf("Available() error = %v, want exec.ErrNotFound", err)
}
if !strings.Contains(err.Error(), "not on PATH") {
t.Fatalf("Available() error = %v, want not on PATH message", err)
}
if strings.Contains(err.Error(), "not authenticated") {
t.Fatalf("Available() error = %v, must not report auth failure when gh is missing", err)
}
}

func TestAvailableWrapsAuthFailureWithStderr(t *testing.T) {
t.Parallel()

host := New(githubTestCmdFactory(map[string]githubTestResponse{
"gh auth status": {stderr: "github.com\n X Failed to log in\n", code: 1},
}), func() bool { return true }, "", "")

err := host.Available(context.Background())
if err == nil {
t.Fatal("Available() error = nil, want auth failure")
}
if !strings.Contains(err.Error(), "not authenticated") {
t.Fatalf("Available() error = %v, want not authenticated", err)
}
if !strings.Contains(err.Error(), "Failed to log in") {
t.Fatalf("Available() error = %v, want stderr detail", err)
}
}

type githubTestResponse struct {
stdout string
stderr string
Expand Down