From f1a5bf88e7a3044e0dc66c92117b68c16f6c3af6 Mon Sep 17 00:00:00 2001 From: zyfy29 Date: Sat, 27 Sep 2025 11:12:09 +0900 Subject: [PATCH 1/4] refactor: Use `errors` package to compare and assert error types --- example/basicauth/main.go | 4 +- github/actions_workflow_runs_test.go | 4 +- github/github.go | 21 +++--- github/github_test.go | 86 +++++++++++++---------- github/issue_import.go | 5 +- github/issue_import_test.go | 4 +- github/orgs_outside_collaborators_test.go | 16 ++--- github/pulls_reviews_test.go | 3 +- github/repos.go | 4 +- github/repos_forks.go | 4 +- github/repos_forks_test.go | 4 +- github/repos_stats_test.go | 4 +- github/repos_test.go | 6 +- github/security_advisories.go | 7 +- github/security_advisories_test.go | 4 +- 15 files changed, 104 insertions(+), 72 deletions(-) diff --git a/example/basicauth/main.go b/example/basicauth/main.go index 0d30c288f29..5b78de6cb19 100644 --- a/example/basicauth/main.go +++ b/example/basicauth/main.go @@ -17,6 +17,7 @@ package main import ( "bufio" "context" + "errors" "fmt" "os" "strings" @@ -43,7 +44,8 @@ func main() { user, _, err := client.Users.Get(ctx, "") // Is this a two-factor auth error? If so, prompt for OTP and try again. - if _, ok := err.(*github.TwoFactorAuthError); ok { + var twoFactorAuthError *github.TwoFactorAuthError + if errors.As(err, &twoFactorAuthError) { fmt.Print("\nGitHub OTP: ") otp, _ := r.ReadString('\n') tp.OTP = strings.TrimSpace(otp) diff --git a/github/actions_workflow_runs_test.go b/github/actions_workflow_runs_test.go index 103d6f20421..c8b54feb676 100644 --- a/github/actions_workflow_runs_test.go +++ b/github/actions_workflow_runs_test.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -482,7 +483,8 @@ func TestActionsService_CancelWorkflowRunByID(t *testing.T) { ctx := context.Background() resp, err := client.Actions.CancelWorkflowRunByID(ctx, "o", "r", 3434) - if _, ok := err.(*AcceptedError); !ok { + var aerr *AcceptedError + if !errors.As(err, &aerr) { t.Errorf("Actions.CancelWorkflowRunByID returned error: %v (want AcceptedError)", err) } if resp.StatusCode != http.StatusAccepted { diff --git a/github/github.go b/github/github.go index 9a1272537b9..2b5bdfbaf0e 100644 --- a/github/github.go +++ b/github/github.go @@ -896,7 +896,8 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ } // If the error type is *url.Error, sanitize its URL before returning. - if e, ok := err.(*url.Error); ok { + var e *url.Error + if errors.As(err, &e) { if url, err := url.Parse(e.URL); err == nil { e.URL = sanitizeURL(url).String() return response, e @@ -923,7 +924,8 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ // added to the AcceptedError and returned. // // Issue #1022 - aerr, ok := err.(*AcceptedError) + var aerr *AcceptedError + ok := errors.As(err, &aerr) if ok { b, readErr := io.ReadAll(resp.Body) if readErr != nil { @@ -934,8 +936,8 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ err = aerr } - rateLimitError, ok := err.(*RateLimitError) - if ok && req.Context().Value(SleepUntilPrimaryRateLimitResetWhenRateLimited) != nil { + var rateLimitError *RateLimitError + if ok := errors.As(err, &rateLimitError); ok && req.Context().Value(SleepUntilPrimaryRateLimitResetWhenRateLimited) != nil { if err := sleepUntilResetWithBuffer(req.Context(), rateLimitError.Rate.Reset.Time); err != nil { return response, err } @@ -944,8 +946,8 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ } // Update the secondary rate limit if we hit it. - rerr, ok := err.(*AbuseRateLimitError) - if ok && rerr.RetryAfter != nil { + var rerr *AbuseRateLimitError + if errors.As(err, &rerr) && rerr.RetryAfter != nil { // if a max duration is specified, make sure that we are waiting at most this duration if c.MaxSecondaryRateLimitRetryAfterDuration > 0 && rerr.GetRetryAfter() > c.MaxSecondaryRateLimitRetryAfterDuration { rerr.RetryAfter = &c.MaxSecondaryRateLimitRetryAfterDuration @@ -992,8 +994,8 @@ var errInvalidLocation = errors.New("invalid or empty Location header in redirec func (c *Client) bareDoUntilFound(ctx context.Context, req *http.Request, maxRedirects int) (*url.URL, *Response, error) { response, err := c.bareDoIgnoreRedirects(ctx, req) if err != nil { - rerr, ok := err.(*RedirectionError) - if ok { + var rerr *RedirectionError + if errors.As(err, &rerr) { // If we receive a 302, transform potential relative locations into absolute and return it. if rerr.StatusCode == http.StatusFound { if rerr.Location == nil { @@ -1486,7 +1488,8 @@ func parseBoolResponse(err error) (bool, error) { return true, nil } - if err, ok := err.(*ErrorResponse); ok && err.Response.StatusCode == http.StatusNotFound { + var rerr *ErrorResponse + if errors.As(err, &rerr) && rerr.Response.StatusCode == http.StatusNotFound { // Simply false. In this one case, we do not pass the error through. return false, nil } diff --git a/github/github_test.go b/github/github_test.go index 4cadc0b732b..6faadefb200 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -144,7 +144,8 @@ func testURLParseError(t *testing.T, err error) { if err == nil { t.Error("Expected error to be returned") } - if err, ok := err.(*url.Error); !ok || err.Op != "parse" { + var uerr *url.Error + if !errors.As(err, &uerr) || uerr.Op != "parse" { t.Errorf("Expected URL parse error, got %+v", err) } } @@ -282,11 +283,15 @@ func testErrorResponseForStatusCode(t *testing.T, code int) { ctx := context.Background() _, _, err := client.Repositories.ListHooks(ctx, "o", "r", nil) - switch e := err.(type) { - case *ErrorResponse: - case *RateLimitError: - case *AbuseRateLimitError: - if code != e.Response.StatusCode { + var errResp *ErrorResponse + var rateLimitErr *RateLimitError + var abuseErr *AbuseRateLimitError + + switch { + case errors.As(err, &errResp): + case errors.As(err, &rateLimitErr): + case errors.As(err, &abuseErr): + if code != abuseErr.Response.StatusCode { t.Error("Error response does not contain status code") } default: @@ -577,7 +582,8 @@ func TestNewRequest_invalidJSON(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - if err, ok := err.(*json.UnsupportedTypeError); !ok { + var jerr *json.UnsupportedTypeError + if !errors.As(err, &jerr) { t.Errorf("Expected a JSON error; got %#v.", err) } } @@ -1129,7 +1135,8 @@ func TestDo_redirectLoop(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - if err, ok := err.(*url.Error); !ok { + var uerr *url.Error + if !errors.As(err, &uerr) { t.Errorf("Expected a URL error; got %#v.", err) } } @@ -1157,8 +1164,8 @@ func TestDo_preservesResponseInHTTPError(t *testing.T) { } // Verify error type and access to status code - errResp, ok := err.(*ErrorResponse) - if !ok { + var errResp *ErrorResponse + if errors.As(err, &errResp) { t.Fatalf("Expected *ErrorResponse error, got %T", err) } @@ -1336,7 +1343,8 @@ func TestDo_rateLimit_errorResponse(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - if _, ok := err.(*RateLimitError); ok { + var rerr *RateLimitError + if errors.As(err, &rerr) { t.Errorf("Did not expect a *RateLimitError error; got %#v.", err) } if got, want := resp.Rate.Limit, 60; got != want { @@ -1383,8 +1391,8 @@ func TestDo_rateLimit_rateLimitError(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - rateLimitErr, ok := err.(*RateLimitError) - if !ok { + var rateLimitErr *RateLimitError + if !errors.As(err, &rateLimitErr) { t.Fatalf("Expected a *RateLimitError error; got %#v.", err) } if got, want := rateLimitErr.Rate.Limit, 60; got != want { @@ -1450,8 +1458,8 @@ func TestDo_rateLimit_noNetworkCall(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - rateLimitErr, ok := err.(*RateLimitError) - if !ok { + var rateLimitErr *RateLimitError + if errors.As(err, &rateLimitErr) { t.Fatalf("Expected a *RateLimitError error; got %#v.", err) } if got, want := rateLimitErr.Rate.Limit, 60; got != want { @@ -1688,8 +1696,8 @@ func TestDo_rateLimit_abortSleepContextCancelledClientLimit(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) defer cancel() _, err := client.Do(context.WithValue(ctx, SleepUntilPrimaryRateLimitResetWhenRateLimited, true), req, nil) - rateLimitError, ok := err.(*RateLimitError) - if !ok { + var rateLimitError *RateLimitError + if errors.As(err, &rateLimitError) { t.Fatalf("Expected a *rateLimitError error; got %#v.", err) } if got, wantSuffix := rateLimitError.Message, "Context cancelled while waiting for rate limit to reset until"; !strings.HasPrefix(got, wantSuffix) { @@ -1724,8 +1732,8 @@ func TestDo_rateLimit_abuseRateLimitError(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - abuseRateLimitErr, ok := err.(*AbuseRateLimitError) - if !ok { + var abuseRateLimitErr *AbuseRateLimitError + if errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if got, want := abuseRateLimitErr.RetryAfter, (*time.Duration)(nil); got != want { @@ -1759,8 +1767,8 @@ func TestDo_rateLimit_abuseRateLimitErrorEnterprise(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - abuseRateLimitErr, ok := err.(*AbuseRateLimitError) - if !ok { + var abuseRateLimitErr *AbuseRateLimitError + if errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if got, want := abuseRateLimitErr.RetryAfter, (*time.Duration)(nil); got != want { @@ -1790,8 +1798,8 @@ func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - abuseRateLimitErr, ok := err.(*AbuseRateLimitError) - if !ok { + var abuseRateLimitErr *AbuseRateLimitError + if errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1805,8 +1813,7 @@ func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) { if _, err = client.Do(ctx, req, nil); err == nil { t.Error("Expected error to be returned.") } - abuseRateLimitErr, ok = err.(*AbuseRateLimitError) - if !ok { + if errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1847,8 +1854,8 @@ func TestDo_rateLimit_abuseRateLimitError_xRateLimitReset(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - abuseRateLimitErr, ok := err.(*AbuseRateLimitError) - if !ok { + var abuseRateLimitErr *AbuseRateLimitError + if errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1863,8 +1870,7 @@ func TestDo_rateLimit_abuseRateLimitError_xRateLimitReset(t *testing.T) { if _, err = client.Do(ctx, req, nil); err == nil { t.Error("Expected error to be returned.") } - abuseRateLimitErr, ok = err.(*AbuseRateLimitError) - if !ok { + if errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1907,8 +1913,8 @@ func TestDo_rateLimit_abuseRateLimitError_maxDuration(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - abuseRateLimitErr, ok := err.(*AbuseRateLimitError) - if !ok { + var abuseRateLimitErr *AbuseRateLimitError + if errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -2082,7 +2088,8 @@ func TestCheckResponse(t *testing.T) { "errors": [{"resource": "r", "field": "f", "code": "c"}], "block": {"reason": "dmca", "created_at": "2016-03-17T15:39:46Z"}}`)), } - err := CheckResponse(res).(*ErrorResponse) + var err *ErrorResponse + errors.As(CheckResponse(res), &err) if err == nil { t.Error("Expected error response.") @@ -2117,7 +2124,8 @@ func TestCheckResponse_RateLimit(t *testing.T) { res.Header.Set(headerRateReset, "243424") res.Header.Set(headerRateResource, "core") - err := CheckResponse(res).(*RateLimitError) + var err *RateLimitError + errors.As(CheckResponse(res), &err) if err == nil { t.Error("Expected error response.") @@ -2141,7 +2149,8 @@ func TestCheckResponse_AbuseRateLimit(t *testing.T) { Body: io.NopCloser(strings.NewReader(`{"message":"m", "documentation_url": "docs.github.com/en/rest/overview/resources-in-the-rest-api#abuse-rate-limits"}`)), } - err := CheckResponse(res).(*AbuseRateLimitError) + var err *AbuseRateLimitError + errors.As(CheckResponse(res), &err) if err == nil { t.Error("Expected error response.") @@ -2167,7 +2176,8 @@ func TestCheckResponse_RedirectionError(t *testing.T) { Body: io.NopCloser(strings.NewReader(``)), } res.Header.Set("Location", urlStr) - err := CheckResponse(res).(*RedirectionError) + var err *RedirectionError + errors.As(CheckResponse(res), &err) if err == nil { t.Error("Expected error response.") @@ -2576,7 +2586,8 @@ func TestCheckResponse_noBody(t *testing.T) { StatusCode: http.StatusBadRequest, Body: io.NopCloser(strings.NewReader("")), } - err := CheckResponse(res).(*ErrorResponse) + var err *ErrorResponse + errors.As(CheckResponse(res), &err) if err == nil { t.Error("Expected error response.") @@ -2598,7 +2609,8 @@ func TestCheckResponse_unexpectedErrorStructure(t *testing.T) { StatusCode: http.StatusBadRequest, Body: io.NopCloser(strings.NewReader(httpBody)), } - err := CheckResponse(res).(*ErrorResponse) + var err *ErrorResponse + errors.As(CheckResponse(res), &err) if err == nil { t.Error("Expected error response.") diff --git a/github/issue_import.go b/github/issue_import.go index 4f06371085e..843a8198941 100644 --- a/github/issue_import.go +++ b/github/issue_import.go @@ -9,6 +9,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" ) @@ -86,8 +87,8 @@ func (s *IssueImportService) Create(ctx context.Context, owner, repo string, iss i := new(IssueImportResponse) resp, err := s.client.Do(ctx, req, i) if err != nil { - aerr, ok := err.(*AcceptedError) - if ok { + var aerr *AcceptedError + if errors.As(err, &aerr) { if err := json.Unmarshal(aerr.Raw, i); err != nil { return i, resp, err } diff --git a/github/issue_import_test.go b/github/issue_import_test.go index 4694df7c336..8fe8ec77203 100644 --- a/github/issue_import_test.go +++ b/github/issue_import_test.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" "net/http" "testing" @@ -110,7 +111,8 @@ func TestIssueImportService_Create_deferred(t *testing.T) { ctx := context.Background() got, _, err := client.IssueImport.Create(ctx, "o", "r", input) - if _, ok := err.(*AcceptedError); !ok { + var aerr *AcceptedError + if !errors.As(err, &aerr) { t.Errorf("Create returned error: %v (want AcceptedError)", err) } diff --git a/github/orgs_outside_collaborators_test.go b/github/orgs_outside_collaborators_test.go index 7dd5618f697..c7b5d33276d 100644 --- a/github/orgs_outside_collaborators_test.go +++ b/github/orgs_outside_collaborators_test.go @@ -7,6 +7,7 @@ package github import ( "context" + "errors" "fmt" "net/http" "testing" @@ -104,10 +105,9 @@ func TestOrganizationsService_RemoveOutsideCollaborator_NonMember(t *testing.T) ctx := context.Background() _, err := client.Organizations.RemoveOutsideCollaborator(ctx, "o", "u") - if err, ok := err.(*ErrorResponse); !ok { + var rerr *ErrorResponse + if !errors.As(err, &rerr) { t.Error("Organizations.RemoveOutsideCollaborator did not return an error") - } else if err.Response.StatusCode != http.StatusNotFound { - t.Error("Organizations.RemoveOutsideCollaborator did not return 404 status code") } } @@ -123,10 +123,9 @@ func TestOrganizationsService_RemoveOutsideCollaborator_Member(t *testing.T) { ctx := context.Background() _, err := client.Organizations.RemoveOutsideCollaborator(ctx, "o", "u") - if err, ok := err.(*ErrorResponse); !ok { + var rerr *ErrorResponse + if !errors.As(err, &rerr) { t.Error("Organizations.RemoveOutsideCollaborator did not return an error") - } else if err.Response.StatusCode != http.StatusUnprocessableEntity { - t.Error("Organizations.RemoveOutsideCollaborator did not return 422 status code") } } @@ -168,9 +167,8 @@ func TestOrganizationsService_ConvertMemberToOutsideCollaborator_NonMemberOrLast ctx := context.Background() _, err := client.Organizations.ConvertMemberToOutsideCollaborator(ctx, "o", "u") - if err, ok := err.(*ErrorResponse); !ok { + var rerr *ErrorResponse + if !errors.As(err, &rerr) { t.Error("Organizations.ConvertMemberToOutsideCollaborator did not return an error") - } else if err.Response.StatusCode != http.StatusForbidden { - t.Error("Organizations.ConvertMemberToOutsideCollaborator did not return 403 status code") } } diff --git a/github/pulls_reviews_test.go b/github/pulls_reviews_test.go index f460389ee39..3593506c47c 100644 --- a/github/pulls_reviews_test.go +++ b/github/pulls_reviews_test.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" "net/http" "testing" @@ -334,7 +335,7 @@ func TestPullRequestReviewRequest_isComfortFadePreview(t *testing.T) { t.Parallel() gotBool, gotErr := tc.review.isComfortFadePreview() if tc.wantErr != nil { - if gotErr != tc.wantErr { + if !errors.Is(gotErr, tc.wantErr) { t.Errorf("isComfortFadePreview() = %v, wanted %v", gotErr, tc.wantErr) } } else { diff --git a/github/repos.go b/github/repos.go index 26dc657f086..03780fee4fb 100644 --- a/github/repos.go +++ b/github/repos.go @@ -2367,8 +2367,8 @@ func (s *RepositoriesService) Dispatch(ctx context.Context, owner, repo string, // isBranchNotProtected determines whether a branch is not protected // based on the error message returned by GitHub API. func isBranchNotProtected(err error) bool { - errorResponse, ok := err.(*ErrorResponse) - return ok && errorResponse.Message == githubBranchNotProtected + var errorResponse *ErrorResponse + return errors.As(err, &errorResponse) && errorResponse.Message == githubBranchNotProtected } // EnablePrivateReporting enables private reporting of vulnerabilities for a diff --git a/github/repos_forks.go b/github/repos_forks.go index 60fb49da5ab..268233c37e0 100644 --- a/github/repos_forks.go +++ b/github/repos_forks.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" ) @@ -83,7 +84,8 @@ func (s *RepositoriesService) CreateFork(ctx context.Context, owner, repo string resp, err := s.client.Do(ctx, req, fork) if err != nil { // Persist AcceptedError's metadata to the Repository object. - if aerr, ok := err.(*AcceptedError); ok { + var aerr *AcceptedError + if errors.As(err, &aerr) { if err := json.Unmarshal(aerr.Raw, fork); err != nil { return fork, resp, err } diff --git a/github/repos_forks_test.go b/github/repos_forks_test.go index d90fbfd9a0d..d8bff57d3a1 100644 --- a/github/repos_forks_test.go +++ b/github/repos_forks_test.go @@ -7,6 +7,7 @@ package github import ( "context" + "errors" "fmt" "net/http" "testing" @@ -119,7 +120,8 @@ func TestRepositoriesService_CreateFork_deferred(t *testing.T) { opt := &RepositoryCreateForkOptions{Organization: "o", Name: "n", DefaultBranchOnly: true} ctx := context.Background() repo, _, err := client.Repositories.CreateFork(ctx, "o", "r", opt) - if _, ok := err.(*AcceptedError); !ok { + var aerr *AcceptedError + if !errors.As(err, &aerr) { t.Errorf("Repositories.CreateFork returned error: %v (want AcceptedError)", err) } diff --git a/github/repos_stats_test.go b/github/repos_stats_test.go index d1cec541a72..b3f34154ed5 100644 --- a/github/repos_stats_test.go +++ b/github/repos_stats_test.go @@ -7,6 +7,7 @@ package github import ( "context" + "errors" "fmt" "net/http" "testing" @@ -305,7 +306,8 @@ func TestRepositoriesService_AcceptedError(t *testing.T) { t.Error("RepositoriesService.AcceptedError should have returned an error") } - if _, ok := err.(*AcceptedError); !ok { + var aerr *AcceptedError + if !errors.As(err, &aerr) { t.Errorf("RepositoriesService.AcceptedError returned an AcceptedError: %v", err) } diff --git a/github/repos_test.go b/github/repos_test.go index e2e9d78031a..b0dc6401dd8 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1417,7 +1417,7 @@ func TestRepositoriesService_GetBranchProtection_branchNotProtected(t *testing.T t.Error("Repositories.GetBranchProtection returned non-nil protection data") } - if err != ErrBranchNotProtected { + if !errors.Is(err, ErrBranchNotProtected) { t.Errorf("Repositories.GetBranchProtection returned an invalid error: %v", err) } }) @@ -2497,7 +2497,7 @@ func TestRepositoriesService_GetRequiredStatusChecks_branchNotProtected(t *testi t.Error("Repositories.GetRequiredStatusChecks returned non-nil status-checks data") } - if err != ErrBranchNotProtected { + if !errors.Is(err, ErrBranchNotProtected) { t.Errorf("Repositories.GetRequiredStatusChecks returned an invalid error: %v", err) } }) @@ -2793,7 +2793,7 @@ func TestRepositoriesService_ListRequiredStatusChecksContexts_branchNotProtected t.Error("Repositories.ListRequiredStatusChecksContexts returned non-nil contexts data") } - if err != ErrBranchNotProtected { + if !errors.Is(err, ErrBranchNotProtected) { t.Errorf("Repositories.ListRequiredStatusChecksContexts returned an invalid error: %v", err) } }) diff --git a/github/security_advisories.go b/github/security_advisories.go index d783345de86..8810ee19c53 100644 --- a/github/security_advisories.go +++ b/github/security_advisories.go @@ -8,6 +8,7 @@ package github import ( "context" "encoding/json" + "errors" "fmt" ) @@ -143,7 +144,8 @@ func (s *SecurityAdvisoriesService) RequestCVE(ctx context.Context, owner, repo, resp, err := s.client.Do(ctx, req, nil) if err != nil { - if _, ok := err.(*AcceptedError); ok { + var aerr *AcceptedError + if errors.As(err, &aerr) { return resp, nil } @@ -170,7 +172,8 @@ func (s *SecurityAdvisoriesService) CreateTemporaryPrivateFork(ctx context.Conte fork := new(Repository) resp, err := s.client.Do(ctx, req, fork) if err != nil { - if aerr, ok := err.(*AcceptedError); ok { + var aerr *AcceptedError + if errors.As(err, &aerr) { if err := json.Unmarshal(aerr.Raw, fork); err != nil { return fork, resp, err } diff --git a/github/security_advisories_test.go b/github/security_advisories_test.go index 15befb62b64..e3482c45601 100644 --- a/github/security_advisories_test.go +++ b/github/security_advisories_test.go @@ -7,6 +7,7 @@ package github import ( "context" + "errors" "fmt" "net/http" "strings" @@ -403,7 +404,8 @@ func TestSecurityAdvisoriesService_CreateTemporaryPrivateFork_deferred(t *testin ctx := context.Background() fork, _, err := client.SecurityAdvisories.CreateTemporaryPrivateFork(ctx, "o", "r", "ghsa_id") - if _, ok := err.(*AcceptedError); !ok { + var aerr *AcceptedError + if !errors.As(err, &aerr) { t.Errorf("SecurityAdvisoriesService.CreateTemporaryPrivateFork returned error: %v (want AcceptedError)", err) } From 8587e701ad24bf5746305da541c8740e888023b5 Mon Sep 17 00:00:00 2001 From: zyfy29 Date: Sun, 28 Sep 2025 11:09:53 +0900 Subject: [PATCH 2/4] fix test and lint error --- github/github.go | 6 +++--- github/github_test.go | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/github/github.go b/github/github.go index 2b5bdfbaf0e..63944828e1c 100644 --- a/github/github.go +++ b/github/github.go @@ -925,8 +925,7 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ // // Issue #1022 var aerr *AcceptedError - ok := errors.As(err, &aerr) - if ok { + if errors.As(err, &aerr) { b, readErr := io.ReadAll(resp.Body) if readErr != nil { return response, readErr @@ -937,7 +936,8 @@ func (c *Client) bareDo(ctx context.Context, caller *http.Client, req *http.Requ } var rateLimitError *RateLimitError - if ok := errors.As(err, &rateLimitError); ok && req.Context().Value(SleepUntilPrimaryRateLimitResetWhenRateLimited) != nil { + if errors.As(err, &rateLimitError) && + req.Context().Value(SleepUntilPrimaryRateLimitResetWhenRateLimited) != nil { if err := sleepUntilResetWithBuffer(req.Context(), rateLimitError.Rate.Reset.Time); err != nil { return response, err } diff --git a/github/github_test.go b/github/github_test.go index 6faadefb200..d4779b888de 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -1165,7 +1165,7 @@ func TestDo_preservesResponseInHTTPError(t *testing.T) { // Verify error type and access to status code var errResp *ErrorResponse - if errors.As(err, &errResp) { + if !errors.As(err, &errResp) { t.Fatalf("Expected *ErrorResponse error, got %T", err) } @@ -1459,7 +1459,7 @@ func TestDo_rateLimit_noNetworkCall(t *testing.T) { t.Error("Expected error to be returned.") } var rateLimitErr *RateLimitError - if errors.As(err, &rateLimitErr) { + if !errors.As(err, &rateLimitErr) { t.Fatalf("Expected a *RateLimitError error; got %#v.", err) } if got, want := rateLimitErr.Rate.Limit, 60; got != want { @@ -1697,7 +1697,7 @@ func TestDo_rateLimit_abortSleepContextCancelledClientLimit(t *testing.T) { defer cancel() _, err := client.Do(context.WithValue(ctx, SleepUntilPrimaryRateLimitResetWhenRateLimited, true), req, nil) var rateLimitError *RateLimitError - if errors.As(err, &rateLimitError) { + if !errors.As(err, &rateLimitError) { t.Fatalf("Expected a *rateLimitError error; got %#v.", err) } if got, wantSuffix := rateLimitError.Message, "Context cancelled while waiting for rate limit to reset until"; !strings.HasPrefix(got, wantSuffix) { @@ -1733,7 +1733,7 @@ func TestDo_rateLimit_abuseRateLimitError(t *testing.T) { t.Error("Expected error to be returned.") } var abuseRateLimitErr *AbuseRateLimitError - if errors.As(err, &abuseRateLimitErr) { + if !errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if got, want := abuseRateLimitErr.RetryAfter, (*time.Duration)(nil); got != want { @@ -1768,7 +1768,7 @@ func TestDo_rateLimit_abuseRateLimitErrorEnterprise(t *testing.T) { t.Error("Expected error to be returned.") } var abuseRateLimitErr *AbuseRateLimitError - if errors.As(err, &abuseRateLimitErr) { + if !errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if got, want := abuseRateLimitErr.RetryAfter, (*time.Duration)(nil); got != want { @@ -1799,7 +1799,7 @@ func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) { t.Error("Expected error to be returned.") } var abuseRateLimitErr *AbuseRateLimitError - if errors.As(err, &abuseRateLimitErr) { + if !errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1813,7 +1813,7 @@ func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) { if _, err = client.Do(ctx, req, nil); err == nil { t.Error("Expected error to be returned.") } - if errors.As(err, &abuseRateLimitErr) { + if !errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1855,7 +1855,7 @@ func TestDo_rateLimit_abuseRateLimitError_xRateLimitReset(t *testing.T) { t.Error("Expected error to be returned.") } var abuseRateLimitErr *AbuseRateLimitError - if errors.As(err, &abuseRateLimitErr) { + if !errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1870,7 +1870,7 @@ func TestDo_rateLimit_abuseRateLimitError_xRateLimitReset(t *testing.T) { if _, err = client.Do(ctx, req, nil); err == nil { t.Error("Expected error to be returned.") } - if errors.As(err, &abuseRateLimitErr) { + if !errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { @@ -1914,7 +1914,7 @@ func TestDo_rateLimit_abuseRateLimitError_maxDuration(t *testing.T) { t.Error("Expected error to be returned.") } var abuseRateLimitErr *AbuseRateLimitError - if errors.As(err, &abuseRateLimitErr) { + if !errors.As(err, &abuseRateLimitErr) { t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err) } if abuseRateLimitErr.RetryAfter == nil { From 532142e46d47324ccb4bb759efd66de279d410c6 Mon Sep 17 00:00:00 2001 From: zyfy29 Date: Mon, 29 Sep 2025 14:54:05 +0900 Subject: [PATCH 3/4] reduce unneeded error value pointer lifecycle --- example/basicauth/main.go | 3 +-- github/actions_workflow_runs_test.go | 3 +-- github/github.go | 15 ++++++++++----- github/github_test.go | 22 +++++++--------------- github/issue_import_test.go | 3 +-- github/orgs_outside_collaborators_test.go | 9 +++------ github/repos_forks_test.go | 3 +-- github/repos_stats_test.go | 3 +-- github/security_advisories.go | 3 +-- github/security_advisories_test.go | 3 +-- 10 files changed, 27 insertions(+), 40 deletions(-) diff --git a/example/basicauth/main.go b/example/basicauth/main.go index 5b78de6cb19..8795a5ddb08 100644 --- a/example/basicauth/main.go +++ b/example/basicauth/main.go @@ -44,8 +44,7 @@ func main() { user, _, err := client.Users.Get(ctx, "") // Is this a two-factor auth error? If so, prompt for OTP and try again. - var twoFactorAuthError *github.TwoFactorAuthError - if errors.As(err, &twoFactorAuthError) { + if errors.As(err, new(*github.TwoFactorAuthError)) { fmt.Print("\nGitHub OTP: ") otp, _ := r.ReadString('\n') tp.OTP = strings.TrimSpace(otp) diff --git a/github/actions_workflow_runs_test.go b/github/actions_workflow_runs_test.go index c8b54feb676..47dd48ae42c 100644 --- a/github/actions_workflow_runs_test.go +++ b/github/actions_workflow_runs_test.go @@ -483,8 +483,7 @@ func TestActionsService_CancelWorkflowRunByID(t *testing.T) { ctx := context.Background() resp, err := client.Actions.CancelWorkflowRunByID(ctx, "o", "r", 3434) - var aerr *AcceptedError - if !errors.As(err, &aerr) { + if !errors.As(err, new(*AcceptedError)) { t.Errorf("Actions.CancelWorkflowRunByID returned error: %v (want AcceptedError)", err) } if resp.StatusCode != http.StatusAccepted { diff --git a/github/github.go b/github/github.go index 63944828e1c..5b7c744f387 100644 --- a/github/github.go +++ b/github/github.go @@ -1183,7 +1183,8 @@ func (r *ErrorResponse) Error() string { // Is returns whether the provided error equals this error. func (r *ErrorResponse) Is(target error) bool { - v, ok := target.(*ErrorResponse) + var v *ErrorResponse + ok := errors.As(target, &v) if !ok { return false } @@ -1248,7 +1249,8 @@ func (r *RateLimitError) Error() string { // Is returns whether the provided error equals this error. func (r *RateLimitError) Is(target error) bool { - v, ok := target.(*RateLimitError) + var v *RateLimitError + ok := errors.As(target, &v) if !ok { return false } @@ -1275,7 +1277,8 @@ func (*AcceptedError) Error() string { // Is returns whether the provided error equals this error. func (ae *AcceptedError) Is(target error) bool { - v, ok := target.(*AcceptedError) + var v *AcceptedError + ok := errors.As(target, &v) if !ok { return false } @@ -1302,7 +1305,8 @@ func (r *AbuseRateLimitError) Error() string { // Is returns whether the provided error equals this error. func (r *AbuseRateLimitError) Is(target error) bool { - v, ok := target.(*AbuseRateLimitError) + var v *AbuseRateLimitError + ok := errors.As(target, &v) if !ok { return false } @@ -1336,7 +1340,8 @@ func (r *RedirectionError) Error() string { // Is returns whether the provided error equals this error. func (r *RedirectionError) Is(target error) bool { - v, ok := target.(*RedirectionError) + var v *RedirectionError + ok := errors.As(target, &v) if !ok { return false } diff --git a/github/github_test.go b/github/github_test.go index d4779b888de..a72d55a933e 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -283,13 +283,10 @@ func testErrorResponseForStatusCode(t *testing.T, code int) { ctx := context.Background() _, _, err := client.Repositories.ListHooks(ctx, "o", "r", nil) - var errResp *ErrorResponse - var rateLimitErr *RateLimitError var abuseErr *AbuseRateLimitError - switch { - case errors.As(err, &errResp): - case errors.As(err, &rateLimitErr): + case errors.As(err, new(*ErrorResponse)): + case errors.As(err, new(*RateLimitError)): case errors.As(err, &abuseErr): if code != abuseErr.Response.StatusCode { t.Error("Error response does not contain status code") @@ -582,8 +579,7 @@ func TestNewRequest_invalidJSON(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - var jerr *json.UnsupportedTypeError - if !errors.As(err, &jerr) { + if !errors.As(err, new(*json.UnsupportedTypeError)) { t.Errorf("Expected a JSON error; got %#v.", err) } } @@ -1135,8 +1131,7 @@ func TestDo_redirectLoop(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - var uerr *url.Error - if !errors.As(err, &uerr) { + if !errors.As(err, new(*url.Error)) { t.Errorf("Expected a URL error; got %#v.", err) } } @@ -1343,8 +1338,7 @@ func TestDo_rateLimit_errorResponse(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - var rerr *RateLimitError - if errors.As(err, &rerr) { + if errors.As(err, new(*RateLimitError)) { t.Errorf("Did not expect a *RateLimitError error; got %#v.", err) } if got, want := resp.Rate.Limit, 60; got != want { @@ -2032,8 +2026,7 @@ func TestBareDoUntilFound_redirectLoop(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - var rerr *RedirectionError - if !errors.As(err, &rerr) { + if !errors.As(err, new(*RedirectionError)) { t.Errorf("Expected a Redirection error; got %#v.", err) } } @@ -2053,8 +2046,7 @@ func TestBareDoUntilFound_UnexpectedRedirection(t *testing.T) { if err == nil { t.Error("Expected error to be returned.") } - var rerr *RedirectionError - if !errors.As(err, &rerr) { + if !errors.As(err, new(*RedirectionError)) { t.Errorf("Expected a Redirection error; got %#v.", err) } } diff --git a/github/issue_import_test.go b/github/issue_import_test.go index 8fe8ec77203..584d272c03a 100644 --- a/github/issue_import_test.go +++ b/github/issue_import_test.go @@ -111,8 +111,7 @@ func TestIssueImportService_Create_deferred(t *testing.T) { ctx := context.Background() got, _, err := client.IssueImport.Create(ctx, "o", "r", input) - var aerr *AcceptedError - if !errors.As(err, &aerr) { + if !errors.As(err, new(*AcceptedError)) { t.Errorf("Create returned error: %v (want AcceptedError)", err) } diff --git a/github/orgs_outside_collaborators_test.go b/github/orgs_outside_collaborators_test.go index c7b5d33276d..45195c3bbc8 100644 --- a/github/orgs_outside_collaborators_test.go +++ b/github/orgs_outside_collaborators_test.go @@ -105,8 +105,7 @@ func TestOrganizationsService_RemoveOutsideCollaborator_NonMember(t *testing.T) ctx := context.Background() _, err := client.Organizations.RemoveOutsideCollaborator(ctx, "o", "u") - var rerr *ErrorResponse - if !errors.As(err, &rerr) { + if !errors.As(err, new(*ErrorResponse)) { t.Error("Organizations.RemoveOutsideCollaborator did not return an error") } } @@ -123,8 +122,7 @@ func TestOrganizationsService_RemoveOutsideCollaborator_Member(t *testing.T) { ctx := context.Background() _, err := client.Organizations.RemoveOutsideCollaborator(ctx, "o", "u") - var rerr *ErrorResponse - if !errors.As(err, &rerr) { + if !errors.As(err, new(*ErrorResponse)) { t.Error("Organizations.RemoveOutsideCollaborator did not return an error") } } @@ -167,8 +165,7 @@ func TestOrganizationsService_ConvertMemberToOutsideCollaborator_NonMemberOrLast ctx := context.Background() _, err := client.Organizations.ConvertMemberToOutsideCollaborator(ctx, "o", "u") - var rerr *ErrorResponse - if !errors.As(err, &rerr) { + if !errors.As(err, new(*ErrorResponse)) { t.Error("Organizations.ConvertMemberToOutsideCollaborator did not return an error") } } diff --git a/github/repos_forks_test.go b/github/repos_forks_test.go index d8bff57d3a1..27ae3eb7960 100644 --- a/github/repos_forks_test.go +++ b/github/repos_forks_test.go @@ -120,8 +120,7 @@ func TestRepositoriesService_CreateFork_deferred(t *testing.T) { opt := &RepositoryCreateForkOptions{Organization: "o", Name: "n", DefaultBranchOnly: true} ctx := context.Background() repo, _, err := client.Repositories.CreateFork(ctx, "o", "r", opt) - var aerr *AcceptedError - if !errors.As(err, &aerr) { + if !errors.As(err, new(*AcceptedError)) { t.Errorf("Repositories.CreateFork returned error: %v (want AcceptedError)", err) } diff --git a/github/repos_stats_test.go b/github/repos_stats_test.go index b3f34154ed5..cade28b02dc 100644 --- a/github/repos_stats_test.go +++ b/github/repos_stats_test.go @@ -306,8 +306,7 @@ func TestRepositoriesService_AcceptedError(t *testing.T) { t.Error("RepositoriesService.AcceptedError should have returned an error") } - var aerr *AcceptedError - if !errors.As(err, &aerr) { + if !errors.As(err, new(*AcceptedError)) { t.Errorf("RepositoriesService.AcceptedError returned an AcceptedError: %v", err) } diff --git a/github/security_advisories.go b/github/security_advisories.go index 8810ee19c53..d99a43832e2 100644 --- a/github/security_advisories.go +++ b/github/security_advisories.go @@ -144,8 +144,7 @@ func (s *SecurityAdvisoriesService) RequestCVE(ctx context.Context, owner, repo, resp, err := s.client.Do(ctx, req, nil) if err != nil { - var aerr *AcceptedError - if errors.As(err, &aerr) { + if errors.As(err, new(*AcceptedError)) { return resp, nil } diff --git a/github/security_advisories_test.go b/github/security_advisories_test.go index e3482c45601..6df379b9f0a 100644 --- a/github/security_advisories_test.go +++ b/github/security_advisories_test.go @@ -404,8 +404,7 @@ func TestSecurityAdvisoriesService_CreateTemporaryPrivateFork_deferred(t *testin ctx := context.Background() fork, _, err := client.SecurityAdvisories.CreateTemporaryPrivateFork(ctx, "o", "r", "ghsa_id") - var aerr *AcceptedError - if !errors.As(err, &aerr) { + if !errors.As(err, new(*AcceptedError)) { t.Errorf("SecurityAdvisoriesService.CreateTemporaryPrivateFork returned error: %v (want AcceptedError)", err) } From cfb2efb958c7ca100b24bcca7f08f69774b1a94e Mon Sep 17 00:00:00 2001 From: zyfy29 Date: Tue, 30 Sep 2025 15:08:32 +0900 Subject: [PATCH 4/4] fix IDE quick-fix problems --- github/github.go | 15 +++++---------- github/orgs_outside_collaborators_test.go | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/github/github.go b/github/github.go index 5b7c744f387..02602142887 100644 --- a/github/github.go +++ b/github/github.go @@ -1184,8 +1184,7 @@ func (r *ErrorResponse) Error() string { // Is returns whether the provided error equals this error. func (r *ErrorResponse) Is(target error) bool { var v *ErrorResponse - ok := errors.As(target, &v) - if !ok { + if !errors.As(target, &v) { return false } @@ -1250,8 +1249,7 @@ func (r *RateLimitError) Error() string { // Is returns whether the provided error equals this error. func (r *RateLimitError) Is(target error) bool { var v *RateLimitError - ok := errors.As(target, &v) - if !ok { + if !errors.As(target, &v) { return false } @@ -1278,8 +1276,7 @@ func (*AcceptedError) Error() string { // Is returns whether the provided error equals this error. func (ae *AcceptedError) Is(target error) bool { var v *AcceptedError - ok := errors.As(target, &v) - if !ok { + if !errors.As(target, &v) { return false } return bytes.Equal(ae.Raw, v.Raw) @@ -1306,8 +1303,7 @@ func (r *AbuseRateLimitError) Error() string { // Is returns whether the provided error equals this error. func (r *AbuseRateLimitError) Is(target error) bool { var v *AbuseRateLimitError - ok := errors.As(target, &v) - if !ok { + if !errors.As(target, &v) { return false } @@ -1341,8 +1337,7 @@ func (r *RedirectionError) Error() string { // Is returns whether the provided error equals this error. func (r *RedirectionError) Is(target error) bool { var v *RedirectionError - ok := errors.As(target, &v) - if !ok { + if !errors.As(target, &v) { return false } diff --git a/github/orgs_outside_collaborators_test.go b/github/orgs_outside_collaborators_test.go index 45195c3bbc8..211f244a6d6 100644 --- a/github/orgs_outside_collaborators_test.go +++ b/github/orgs_outside_collaborators_test.go @@ -105,8 +105,11 @@ func TestOrganizationsService_RemoveOutsideCollaborator_NonMember(t *testing.T) ctx := context.Background() _, err := client.Organizations.RemoveOutsideCollaborator(ctx, "o", "u") - if !errors.As(err, new(*ErrorResponse)) { + var rerr *ErrorResponse + if !errors.As(err, &rerr) { t.Error("Organizations.RemoveOutsideCollaborator did not return an error") + } else if rerr.Response.StatusCode != http.StatusNotFound { + t.Error("Organizations.RemoveOutsideCollaborator did not return 404 status code") } } @@ -122,8 +125,11 @@ func TestOrganizationsService_RemoveOutsideCollaborator_Member(t *testing.T) { ctx := context.Background() _, err := client.Organizations.RemoveOutsideCollaborator(ctx, "o", "u") - if !errors.As(err, new(*ErrorResponse)) { + var rerr *ErrorResponse + if !errors.As(err, &rerr) { t.Error("Organizations.RemoveOutsideCollaborator did not return an error") + } else if rerr.Response.StatusCode != http.StatusUnprocessableEntity { + t.Error("Organizations.RemoveOutsideCollaborator did not return 422 status code") } } @@ -165,7 +171,10 @@ func TestOrganizationsService_ConvertMemberToOutsideCollaborator_NonMemberOrLast ctx := context.Background() _, err := client.Organizations.ConvertMemberToOutsideCollaborator(ctx, "o", "u") - if !errors.As(err, new(*ErrorResponse)) { + var rerr *ErrorResponse + if !errors.As(err, &rerr) { t.Error("Organizations.ConvertMemberToOutsideCollaborator did not return an error") + } else if rerr.Response.StatusCode != http.StatusForbidden { + t.Error("Organizations.ConvertMemberToOutsideCollaborator did not return 403 status code") } }