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
6 changes: 4 additions & 2 deletions pkg/actionpins/actionpins_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ func TestFormatPinnedActionReference_PanicsWhenSHAIsEmpty(t *testing.T) {
}

func TestPinContextEmitOnce_InitializesAndDeduplicates(t *testing.T) {
t.Parallel()
// Not parallel: CaptureStderr swaps the global os.Stderr and is not safe
// to run concurrently with other tests that write to stderr.

ctx := &PinContext{}

Expand Down Expand Up @@ -953,7 +954,8 @@ func TestApplyContainerPinMapping(t *testing.T) {
}

func TestApplyContainerPinMapping_DeduplicatesInvalidWarnings(t *testing.T) {
t.Parallel()
// Not parallel: CaptureStderr swaps the global os.Stderr and is not safe
// to run concurrently with other tests that write to stderr.

ctx := &PinContext{
ContainerMappings: map[string]string{
Expand Down
11 changes: 2 additions & 9 deletions pkg/cli/add_package_manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/goccy/go-yaml"

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/errorutil"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/parser"
"github.com/github/gh-aw/pkg/semverutil"
Expand Down Expand Up @@ -1005,20 +1006,12 @@ func listRepositoryPackageDirSubdirsForHost(ctx context.Context, owner, repo, re
}

func normalizeRepositoryPackageRemoteError(err error) error {
if err == nil || !isRepositoryPackageRemoteNotFound(err) {
if err == nil || !errorutil.IsNotFoundError(err) {
return err
}
return packageRemoteNotFoundError{cause: err}
}

func isRepositoryPackageRemoteNotFound(err error) bool {
if err == nil {
return false
}
errText := strings.ToLower(err.Error())
return strings.Contains(errText, "404") || strings.Contains(errText, "not found")
}

func resolveRepositoryPackageDefaultBranch(ctx context.Context, repoSlug, host string) (string, error) {
args := []string{"api", "/repos/" + repoSlug, "--jq", ".default_branch"}
var output []byte
Expand Down
8 changes: 2 additions & 6 deletions pkg/cli/branch_file_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
"strings"

"github.com/github/gh-aw/pkg/errorutil"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/workflow"
)
Expand All @@ -33,7 +34,7 @@ func readRemoteRepoBranchFileContext(ctx context.Context, repoOverride, branchNa
cmd := workflow.ExecGHContext(ctx, args...)
out, err := cmd.CombinedOutput()
if err != nil {
if isRemoteFileNotFoundOutput(string(out)) {
if errorutil.IsNotFoundOutput(string(out)) {
branchFileReaderLog.Printf("Remote file not found: path=%s, branch=%s", filePath, branchName)
return nil, os.ErrNotExist
}
Expand All @@ -55,8 +56,3 @@ func readRemoteRepoBranchFileContext(ctx context.Context, repoOverride, branchNa
func isRemoteFileNotFound(err error) bool {
return errors.Is(err, os.ErrNotExist)
}

func isRemoteFileNotFoundOutput(output string) bool {
s := strings.ToLower(output)
return strings.Contains(s, "404") || strings.Contains(s, "not found")
}
3 changes: 2 additions & 1 deletion pkg/cli/checks_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/errorutil"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/sliceutil"
"github.com/github/gh-aw/pkg/workflow"
Expand Down Expand Up @@ -240,7 +241,7 @@ func classifyGHAPIError(exitCode int, stderr string, prNumber string, repo strin
lower := strings.ToLower(stderr)

switch {
case strings.Contains(lower, "404") || strings.Contains(lower, "not found"):
case errorutil.IsNotFoundOutput(stderr):
repoHint := "the current repository"
if repo != "" {
repoHint = repo
Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/evals_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"strings"

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/errorutil"
"github.com/github/gh-aw/pkg/parser"
"github.com/github/gh-aw/pkg/stringutil"
"github.com/github/gh-aw/pkg/workflow"
Expand Down Expand Up @@ -141,7 +142,7 @@ func resolveRunStateBranchRef(ctx context.Context, repoOverride, branchName stri
cmd := workflow.ExecGHContext(ctx, args...)
out, err := cmd.CombinedOutput()
if err != nil {
if isRemoteFileNotFoundOutput(string(out)) {
if errorutil.IsNotFoundOutput(string(out)) {
return "", os.ErrNotExist
}
return "", fmt.Errorf("failed to list commits for state branch %s: %w", branchName, err)
Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/experiments_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/errorutil"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/parser"
"github.com/github/gh-aw/pkg/setutil"
Expand Down Expand Up @@ -715,7 +716,7 @@ func fetchRemoteExperimentDetails(repoOverride, branchName, workflowID string) (
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
stderr := strings.TrimSpace(string(exitErr.Stderr))
if strings.Contains(stderr, "404") || strings.Contains(stderr, "not found") {
if errorutil.IsNotFoundOutput(stderr) {
return nil, fmt.Errorf("experiment %q not found in %s", workflowID, repoOverride)
}
return nil, fmt.Errorf("failed to fetch experiment branch (exit %d): %s", exitErr.ExitCode(), stderr)
Expand Down
13 changes: 13 additions & 0 deletions pkg/cli/experiments_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,25 @@ package cli

import (
"encoding/json"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestFetchRemoteExperimentDetailsClassifiesTitleCaseNotFound(t *testing.T) {
fakeBinDir := t.TempDir()
fakeGH := filepath.Join(fakeBinDir, "gh")
require.NoError(t, os.WriteFile(fakeGH, []byte("#!/bin/sh\necho 'HTTP 404: Not Found' >&2\nexit 1\n"), 0o755))
t.Setenv("PATH", fakeBinDir+string(os.PathListSeparator)+os.Getenv("PATH"))

_, err := fetchRemoteExperimentDetails("octo/repo", "experiments/missing", "missing")

require.EqualError(t, err, `experiment "missing" not found in octo/repo`)
}

func TestExtractExperimentName(t *testing.T) {
tests := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/logs_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func downloadWorkflowRunLogs(ctx context.Context, runID int64, outputDir string,
// Check both the Go error (via errorutil.IsNotFoundError) and the raw CLI output,
// as the gh CLI may write "not found" to stdout without reflecting it in the error.
// Also treat HTTP 410 Gone as non-critical (logs may be expired).
if errorutil.IsNotFoundError(err) || errorutil.IsNotFoundError(errors.New(string(output))) || errorutil.IsGoneError(err) {
if errorutil.IsNotFoundError(err) || errorutil.IsNotFoundOutput(string(output)) || errorutil.IsGoneError(err) {
if verbose {
fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("No logs found for run %d (may be expired or unavailable)", runID)))
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/cli/setup_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/ctxutil"
"github.com/github/gh-aw/pkg/errorutil"
"github.com/github/gh-aw/pkg/gitutil"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/workflow"
Expand Down Expand Up @@ -104,8 +105,9 @@ func checkSetupRepositoryExists(ctx context.Context, repo string) (bool, error)
return strings.TrimSpace(string(output)) != "", nil
}

message := strings.ToLower(string(output))
if strings.Contains(message, "could not resolve to a repository") || strings.Contains(message, "http 404") || strings.Contains(message, "not found") {
outputStr := string(output)
message := strings.ToLower(outputStr)
if strings.Contains(message, "could not resolve to a repository") || errorutil.IsNotFoundOutput(outputStr) {
return false, nil
}
return false, fmt.Errorf("failed to check repository %s: %w", repo, err)
Expand Down
3 changes: 1 addition & 2 deletions pkg/cli/workflow_run_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cli
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -53,7 +52,7 @@ func buildWorkflowRunMetadataArgs(runID int64, owner, repo, hostname string) []s
func classifyWorkflowRunMetadataError(runID int64, err error, output []byte) error {
outputStr := string(output)
if errorutil.IsNotFoundError(err) ||
errorutil.IsNotFoundError(errors.New(outputStr)) ||
errorutil.IsNotFoundOutput(outputStr) ||
strings.Contains(outputStr, "Could not resolve") {
return fmt.Errorf("workflow run %d not found. Please verify the run ID is correct and that you have access to the repository", runID)
}
Expand Down
17 changes: 16 additions & 1 deletion pkg/errorutil/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ func IsNotFoundError(err error) bool {
return matched
}

// IsNotFoundOutput reports whether output represents an HTTP 404 / "not found" response.
// The check is case-insensitive and matches both the numeric literal "404" and
// the phrase "not found".
func IsNotFoundOutput(output string) bool {
Comment on lines +26 to +29
matched := containsSubstring(output, "404", "not found")
if matched {
errorutilLog.Printf("Classified output as not-found (404): %s", output)
}
return matched
}

// IsForbiddenError reports whether err represents an HTTP 403 / "forbidden" response.
// It returns false when err is nil.
// The check is case-insensitive and only matches HTTP-style 403 patterns such as
Expand Down Expand Up @@ -56,7 +67,11 @@ func containsErrorSubstring(err error, substrings ...string) bool {
if err == nil {
return false
}
msg := strings.ToLower(err.Error())
return containsSubstring(err.Error(), substrings...)
}

func containsSubstring(value string, substrings ...string) bool {
msg := strings.ToLower(value)
for _, substring := range substrings {
if strings.Contains(msg, substring) {
return true
Expand Down
20 changes: 20 additions & 0 deletions pkg/errorutil/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ func TestIsNotFoundError(t *testing.T) {
}
}

func TestIsNotFoundOutput(t *testing.T) {
tests := []struct {
name string
output string
want bool
}{
{name: "empty output", output: "", want: false},
{name: "404 numeric literal", output: "HTTP 404: Not Found", want: true},
{name: "title case not found", output: "GraphQL: Not Found", want: true},
{name: "uppercase not found", output: "RESOURCE NOT FOUND", want: true},
{name: "generic output", output: "something went wrong", want: false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, errorutil.IsNotFoundOutput(tt.output))
})
}
}

func TestIsForbiddenError(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading