Skip to content

Commit fa340cc

Browse files
JeffreyCACopilotCopilot
authored
Enhance GitHub API error handling (#7922)
* Enhance GitHub API error handling with structured ApiError type and actionable suggestions for SAML, rate-limit, and authorization errors * Fix cspell * Address PR review feedback for GitHub API error handling * Address feedback * Add test coverage and address review feedback - F1: Add gh_errors_test.go covering suggestionForApiError per-kind, suggestionForRepoNotAccessible, and withGitHubSuggestion dispatch. - F3: Add Test_ParseGitHubUrl_RepoAccessibleFallsThrough and Test_ParseGitHubUrl_RepoProbeReturnsClassifiedError covering the two remaining /repos/{slug} probe outcomes. - F4: Add TestClassifyKind_HttpStatusToKind covering all status->Kind mappings and TestClassifyKind_SAMLPhraseVariants for SAML detection. - F5: Add malformed-JSON, HTML-body, and message-only fallback rows to TestParseApiError_StatusFromJSONBody. - F6: Wrap ParseGitHubUrl error in newGhTemplateSource with the failing template URL so users see which operation failed. - F9: Tighten KindOther doc comment to match classifyKind behavior. - F13: Add TestParseApiError_StderrHttpStatusFallback covering the '(HTTP NNN)' regex fallback edge cases (multiple markers, missing marker, malformed patterns, case sensitivity). - Use t.Context() in new gh_source_test.go subtests to match repo convention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6e5059e commit fa340cc

9 files changed

Lines changed: 1340 additions & 8 deletions

File tree

‎cli/azd/pkg/templates/gh_errors.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package templates
5+
6+
import (
7+
"errors"
8+
9+
"github.com/azure/azure-dev/cli/azd/internal"
10+
"github.com/azure/azure-dev/cli/azd/pkg/errorhandler"
11+
"github.com/azure/azure-dev/cli/azd/pkg/tools/github"
12+
)
13+
14+
// withGitHubSuggestion wraps the supplied error in an *internal.ErrorWithSuggestion
15+
// when the error is a recognized GitHub failure (typed *github.ApiError or
16+
// *RepoNotAccessibleError). The wrapping carries actionable guidance inline
17+
// in the error chain, which means:
18+
//
19+
// - The core CLI ErrorMiddleware renders it directly without consulting the
20+
// YAML rules pipeline (ErrorMiddleware short-circuits on
21+
// *ErrorWithSuggestion).
22+
// - Callers that don't have access to the YAML pipeline (e.g., extensions
23+
// receiving stringified errors over gRPC) still see the suggestion as part
24+
// of the wrapped error, because *ApiError keeps formatting itself with
25+
// status + message.
26+
//
27+
// Returns the original error unchanged when no specific suggestion applies, so
28+
// other typed-error pipelines (auth, unknown gh failures) behave as before.
29+
func withGitHubSuggestion(err error) error {
30+
if err == nil {
31+
return nil
32+
}
33+
34+
if apiErr, ok := errors.AsType[*github.ApiError](err); ok {
35+
if s := suggestionForApiError(apiErr); s != nil {
36+
s.Err = err
37+
return s
38+
}
39+
}
40+
41+
if repoErr, ok := errors.AsType[*RepoNotAccessibleError](err); ok {
42+
s := suggestionForRepoNotAccessible(repoErr)
43+
s.Err = err
44+
return s
45+
}
46+
47+
return err
48+
}
49+
50+
func suggestionForApiError(apiErr *github.ApiError) *internal.ErrorWithSuggestion {
51+
switch apiErr.Kind {
52+
case github.KindSAMLBlocked:
53+
return &internal.ErrorWithSuggestion{
54+
Message: "The GitHub organization that owns this repository requires SAML SSO " +
55+
"authorization for your token before it can be used.",
56+
Suggestion: "If you signed in with `gh auth login`, run `gh auth refresh` and " +
57+
"complete the SSO authorization in the browser when prompted (use " +
58+
"`gh auth refresh -h <host>` for non-default hosts). If you're using a " +
59+
"personal access token, open your GitHub token settings, locate the " +
60+
"token, click 'Configure SSO', and authorize the organization that owns " +
61+
"this repository.",
62+
Links: []errorhandler.ErrorLink{
63+
{
64+
URL: "https://docs.github.com/enterprise-cloud@latest/authentication/" +
65+
"authenticating-with-single-sign-on/" +
66+
"about-authentication-with-single-sign-on",
67+
Title: "About authentication with single sign-on",
68+
},
69+
{
70+
URL: "https://docs.github.com/enterprise-cloud@latest/authentication/" +
71+
"authenticating-with-single-sign-on/" +
72+
"authorizing-a-personal-access-token-for-use-with-single-sign-on",
73+
Title: "Authorizing a personal access token for use with single sign-on",
74+
},
75+
},
76+
}
77+
case github.KindRateLimited:
78+
return &internal.ErrorWithSuggestion{
79+
Message: "GitHub API rate limit exceeded.",
80+
Suggestion: "Authenticated requests have a much higher limit than anonymous ones. " +
81+
"Run `gh auth login` (or set GITHUB_TOKEN / GH_TOKEN) and retry. " +
82+
"If you're already authenticated, wait for the rate-limit window to reset " +
83+
"(typically up to one hour).",
84+
Links: []errorhandler.ErrorLink{
85+
{
86+
URL: "https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api",
87+
Title: "Rate limits for the REST API",
88+
},
89+
},
90+
}
91+
case github.KindUnauthorized:
92+
return &internal.ErrorWithSuggestion{
93+
Message: "GitHub rejected the request as unauthenticated (HTTP 401).",
94+
Suggestion: "Run `gh auth login` to sign in, or refresh an expired token with " +
95+
"`gh auth refresh`. If you're using GITHUB_TOKEN / GH_TOKEN, regenerate the " +
96+
"token and ensure it has the required scopes.",
97+
}
98+
case github.KindForbidden:
99+
return &internal.ErrorWithSuggestion{
100+
Message: "GitHub denied access to the requested resource (HTTP 403). The " +
101+
"repository may be private, your token may be missing required scopes, or " +
102+
"your account may not have permission.",
103+
Suggestion: "Verify you can access the repository in a browser while signed in " +
104+
"as the same GitHub account. If you're using a personal access token, ensure " +
105+
"it includes the 'repo' scope. Run `gh auth status` to confirm which account " +
106+
"gh is using.",
107+
}
108+
case github.KindServerError:
109+
return &internal.ErrorWithSuggestion{
110+
Message: "GitHub returned a server error (HTTP 5xx). This usually indicates a " +
111+
"transient issue on GitHub's side rather than a problem with your request.",
112+
Suggestion: "Wait a few minutes and try again. If the problem persists, check " +
113+
"https://www.githubstatus.com/ for ongoing incidents.",
114+
Links: []errorhandler.ErrorLink{
115+
{
116+
URL: "https://www.githubstatus.com/",
117+
Title: "GitHub Status",
118+
},
119+
},
120+
}
121+
}
122+
return nil
123+
}
124+
125+
func suggestionForRepoNotAccessible(_ *RepoNotAccessibleError) *internal.ErrorWithSuggestion {
126+
// Leave Message empty so the renderer falls back to RepoNotAccessibleError.Error(),
127+
// which is already user-friendly and includes the repo slug — avoids duplication.
128+
return &internal.ErrorWithSuggestion{
129+
Suggestion: "Confirm the repository URL is correct and that the active gh account " +
130+
"can see it. Run `gh auth status` to check which account is active. For Enterprise " +
131+
"Managed Users (EMU), make sure the active account is the EMU account that owns " +
132+
"this repository — a github.com URL may need to target a different host.",
133+
}
134+
}

‎cli/azd/pkg/templates/gh_errors_test.go‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package templates
5+
6+
import (
7+
"errors"
8+
"strings"
9+
"testing"
10+
11+
"github.com/azure/azure-dev/cli/azd/internal"
12+
"github.com/azure/azure-dev/cli/azd/pkg/tools/github"
13+
"github.com/stretchr/testify/require"
14+
)
15+
16+
// TestSuggestionForApiError_PerKind verifies that every classified
17+
// ApiErrorKind that is meant to surface user guidance produces a non-nil
18+
// *internal.ErrorWithSuggestion containing the substrings users rely on
19+
// (the suggestion text and the relevant doc link). Kinds that intentionally
20+
// return nil (NotFound, Other, Unknown) are also asserted to ensure we
21+
// don't silently start emitting suggestions where none are expected.
22+
func TestSuggestionForApiError_PerKind(t *testing.T) {
23+
t.Parallel()
24+
cases := []struct {
25+
name string
26+
kind github.ApiErrorKind
27+
wantNil bool
28+
wantSnippet string // substring required in the rendered suggestion text
29+
wantLink string // substring required in at least one Links[].URL (empty = skip)
30+
}{
31+
{"SAMLBlocked", github.KindSAMLBlocked, false,
32+
"SAML SSO", "authenticating-with-single-sign-on"},
33+
{"RateLimited", github.KindRateLimited, false,
34+
"rate limit", "rate-limits-for-the-rest-api"},
35+
{"Unauthorized", github.KindUnauthorized, false,
36+
"gh auth login", ""},
37+
{"Forbidden", github.KindForbidden, false,
38+
"gh auth status", ""},
39+
{"ServerError", github.KindServerError, false,
40+
"server error", "githubstatus.com"},
41+
{"NotFound returns nil — RepoNotAccessibleError handles that path", github.KindNotFound, true, "", ""},
42+
{"Other returns nil — falls through to typed *ApiError.Error()", github.KindOther, true, "", ""},
43+
{"Unknown returns nil — surfaces underlying error", github.KindUnknown, true, "", ""},
44+
}
45+
for _, tc := range cases {
46+
t.Run(tc.name, func(t *testing.T) {
47+
t.Parallel()
48+
apiErr := &github.ApiError{
49+
URL: "https://api.github.com/repos/o/r",
50+
Kind: tc.kind,
51+
StatusCode: 500, // arbitrary, not asserted
52+
}
53+
got := suggestionForApiError(apiErr)
54+
if tc.wantNil {
55+
require.Nil(t, got, "expected nil suggestion for %s", tc.kind)
56+
return
57+
}
58+
require.NotNil(t, got, "expected non-nil suggestion for %s", tc.kind)
59+
combined := got.Message + " " + got.Suggestion
60+
require.Contains(t, combined, tc.wantSnippet)
61+
if tc.wantLink != "" {
62+
var found bool
63+
for _, l := range got.Links {
64+
if strings.Contains(l.URL, tc.wantLink) {
65+
found = true
66+
break
67+
}
68+
}
69+
require.True(t, found, "expected a Links[] URL containing %q, got %+v", tc.wantLink, got.Links)
70+
}
71+
})
72+
}
73+
}
74+
75+
// TestSuggestionForRepoNotAccessible verifies the dedicated
76+
// RepoNotAccessibleError suggestion is emitted with EMU/private-repo guidance
77+
// and a non-empty Suggestion field — Message is intentionally empty so the
78+
// renderer falls back to RepoNotAccessibleError.Error() (which already
79+
// contains the repo slug) and we don't duplicate the same text twice.
80+
func TestSuggestionForRepoNotAccessible(t *testing.T) {
81+
t.Parallel()
82+
got := suggestionForRepoNotAccessible(&RepoNotAccessibleError{
83+
Hostname: "github.com",
84+
RepoSlug: "owner/repo",
85+
})
86+
require.NotNil(t, got)
87+
require.Empty(t, got.Message, "Message must be empty so renderer uses RepoNotAccessibleError.Error()")
88+
require.Contains(t, got.Suggestion, "gh auth status")
89+
require.Contains(t, got.Suggestion, "EMU")
90+
}
91+
92+
// TestWithGitHubSuggestion_Dispatch verifies the wrapper:
93+
// - returns nil for nil input
94+
// - wraps *github.ApiError with a classified suggestion (preserving the
95+
// original error in ErrorWithSuggestion.Err so the chain still unwraps
96+
// to the typed *ApiError)
97+
// - wraps *RepoNotAccessibleError into a suggestion (always — never nil,
98+
// because RepoNotAccessibleError is itself a strong signal)
99+
// - returns the original error unchanged for unrecognized types
100+
// - returns the original error unchanged when the typed error has a kind
101+
// that intentionally has no suggestion (e.g., KindNotFound on its own —
102+
// RepoNotAccessibleError is the surface for the "repo invisible" case)
103+
func TestWithGitHubSuggestion_Dispatch(t *testing.T) {
104+
t.Parallel()
105+
106+
t.Run("nil in nil out", func(t *testing.T) {
107+
t.Parallel()
108+
require.NoError(t, withGitHubSuggestion(nil))
109+
})
110+
111+
t.Run("ApiError with suggestion-bearing kind is wrapped", func(t *testing.T) {
112+
t.Parallel()
113+
original := &github.ApiError{
114+
URL: "https://api.github.com/repos/o/r/branches/main",
115+
Kind: github.KindSAMLBlocked,
116+
StatusCode: 403,
117+
}
118+
wrapped := withGitHubSuggestion(original)
119+
ews, ok := errors.AsType[*internal.ErrorWithSuggestion](wrapped)
120+
require.True(t, ok, "expected *internal.ErrorWithSuggestion, got %T", wrapped)
121+
// The ErrorWithSuggestion wraps the original so error chain still
122+
// unwraps to the typed ApiError for downstream consumers.
123+
got, ok := errors.AsType[*github.ApiError](ews)
124+
require.True(t, ok, "ErrorWithSuggestion must preserve *ApiError in chain")
125+
require.Same(t, original, got)
126+
})
127+
128+
t.Run("ApiError without suggestion (KindNotFound) returned unchanged", func(t *testing.T) {
129+
t.Parallel()
130+
original := &github.ApiError{
131+
URL: "https://api.github.com/repos/o/r/branches/main",
132+
Kind: github.KindNotFound,
133+
StatusCode: 404,
134+
}
135+
got := withGitHubSuggestion(original)
136+
require.Same(t, original, got, "no suggestion → return original error untouched")
137+
})
138+
139+
t.Run("RepoNotAccessibleError is always wrapped", func(t *testing.T) {
140+
t.Parallel()
141+
original := &RepoNotAccessibleError{Hostname: "github.com", RepoSlug: "owner/repo"}
142+
wrapped := withGitHubSuggestion(original)
143+
ews, ok := errors.AsType[*internal.ErrorWithSuggestion](wrapped)
144+
require.True(t, ok)
145+
// Chain still unwraps to the typed RepoNotAccessibleError.
146+
got, ok := errors.AsType[*RepoNotAccessibleError](ews)
147+
require.True(t, ok)
148+
require.Same(t, original, got)
149+
})
150+
151+
t.Run("unrecognized error returned unchanged", func(t *testing.T) {
152+
t.Parallel()
153+
original := errors.New("some random error")
154+
got := withGitHubSuggestion(original)
155+
require.Same(t, original, got)
156+
})
157+
}

0 commit comments

Comments
 (0)