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
7 changes: 1 addition & 6 deletions api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,10 @@ func (c Client) RESTWithNext(hostname string, method string, p string, body io.R

resp, err := restClient.Request(method, p, body)
if err != nil {
return "", err
return "", handleResponse(err)
}
defer resp.Body.Close()

success := resp.StatusCode >= 200 && resp.StatusCode < 300
if !success {
return "", HandleHTTPError(resp)
}

if resp.StatusCode == http.StatusNoContent {
return "", nil
}
Expand Down
95 changes: 95 additions & 0 deletions api/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func newTestClient(reg *httpmock.Registry) *Client {
Expand Down Expand Up @@ -138,6 +139,100 @@ func TestRESTError(t *testing.T) {
}
}

func TestRESTWithNextError(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
client := newTestClient(reg)

reg.Register(httpmock.MatchAny, func(req *http.Request) (*http.Response, error) {
return &http.Response{
Request: req,
StatusCode: http.StatusNotFound,
Body: io.NopCloser(bytes.NewBufferString(`{"message": "Not Found"}`)),
Header: http.Header{
"Content-Type": {"application/json"},
"X-Accepted-Oauth-Scopes": {"repo"},
"X-Oauth-Scopes": {"read:user"},
},
}, nil
})

_, err := client.RESTWithNext("github.com", http.MethodGet, "repos/owner/repo/items", nil, nil)

var httpErr HTTPError
require.ErrorAs(t, err, &httpErr)
assert.Equal(t, http.StatusNotFound, httpErr.StatusCode)
assert.Contains(t, err.Error(), "HTTP 404")
assert.Equal(t, `This API operation needs the "repo" scope. To request it, run: gh auth refresh -h github.com -s repo`, httpErr.ScopesSuggestion())
}

func TestRESTAndRESTWithNextErrorTypeParity(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
client := newTestClient(reg)

responder := func(req *http.Request) (*http.Response, error) {
return &http.Response{
Request: req,
StatusCode: http.StatusNotFound,
Body: io.NopCloser(bytes.NewBufferString(`{"message": "Not Found"}`)),
Header: http.Header{"Content-Type": {"application/json"}},
}, nil
}
reg.Register(httpmock.MatchAny, responder)
reg.Register(httpmock.MatchAny, responder)

restErr := client.REST("github.com", http.MethodGet, "repos/owner/repo/items", nil, nil)
_, restWithNextErr := client.RESTWithNext("github.com", http.MethodGet, "repos/owner/repo/items", nil, nil)

require.Error(t, restErr)
require.Error(t, restWithNextErr)
assert.IsType(t, restErr, restWithNextErr)
}

func TestRESTWithNext(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
client := newTestClient(reg)

reg.Register(httpmock.MatchAny, func(req *http.Request) (*http.Response, error) {
return &http.Response{
Request: req,
StatusCode: http.StatusOK,
Body: io.NopCloser(bytes.NewBufferString(`{"name": "item"}`)),
Header: http.Header{
"Content-Type": {"application/json"},
"Link": {`<https://api.github.com/repos/owner/repo/items?page=2>; rel="next", <https://api.github.com/repos/owner/repo/items?page=3>; rel="last"`},
},
}, nil
})

response := struct {
Name string `json:"name"`
}{}
next, err := client.RESTWithNext("github.com", http.MethodGet, "repos/owner/repo/items", nil, &response)

require.NoError(t, err)
assert.Equal(t, "item", response.Name)
assert.Equal(t, "https://api.github.com/repos/owner/repo/items?page=2", next)
}

func TestRESTWithNextNoContent(t *testing.T) {
reg := &httpmock.Registry{}
defer reg.Verify(t)
client := newTestClient(reg)

reg.Register(
httpmock.REST(http.MethodDelete, "repos/owner/repo/items/1"),
httpmock.StatusStringResponse(http.StatusNoContent, "not JSON"),
)

next, err := client.RESTWithNext("github.com", http.MethodDelete, "repos/owner/repo/items/1", nil, nil)

require.NoError(t, err)
assert.Empty(t, next)
}

func TestHandleHTTPError_GraphQL502(t *testing.T) {
req, err := http.NewRequest("GET", "https://api.github.com/user", nil)
if err != nil {
Expand Down
41 changes: 41 additions & 0 deletions pkg/cmd/attestation/api/client_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package api

import (
"net/http"
"testing"

cliAPI "github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/safeurl"
"github.com/cli/cli/v2/pkg/cmd/attestation/io"
"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -379,6 +382,44 @@ func TestGetAttestationsRetries(t *testing.T) {
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
}

func TestGetAttestationsRetriesRESTWithNextError(t *testing.T) {
originalRetryInterval := getAttestationRetryInterval
getAttestationRetryInterval = 0
t.Cleanup(func() {
getAttestationRetryInterval = originalRetryInterval
})

reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.MatchAny,
httpmock.StatusStringResponse(http.StatusInternalServerError, `{"message":"Internal Server Error"}`),
)
reg.Register(
httpmock.MatchAny,
httpmock.JSONResponse(map[string]any{
"attestations": []any{
map[string]any{"bundle_url": "https://example.com/bundle"},
},
}),
)

c := &LiveClient{
githubAPI: cliAPI.NewClientFromHTTP(&http.Client{Transport: reg}),
host: "github.com",
logger: io.NewTestHandler(),
}
attestations, err := c.getAttestations(FetchParams{
Digest: testDigest,
Limit: 1,
Repo: testRepo,
})

require.NoError(t, err)
require.Len(t, attestations, 1)
require.Len(t, reg.Requests, 2)
}

// test total retries
func TestGetAttestationsMaxRetries(t *testing.T) {
getAttestationRetryInterval = 0
Expand Down
19 changes: 19 additions & 0 deletions pkg/cmd/status/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,25 @@ func TestStatusRun(t *testing.T) {
opts: &StatusOptions{},
wantOut: "Assigned Issues │ Assigned Pull Requests \nNothing here ^_^ │ Nothing here ^_^ \n │ \nReview Requests │ Mentions \nNothing here ^_^ │ Nothing here ^_^ \n │ \nRepository Activity\nNothing here ^_^\n\n",
},
{
name: "notifications 404 is tolerated",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL("UserCurrent"),
httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`))
reg.Register(
httpmock.GraphQL("AssignedSearch"),
httpmock.StringResponse(`{"data": { "assignments": {"nodes": [] }, "reviewRequested": {"nodes": []}}}`))
reg.Register(
httpmock.REST("GET", "notifications"),
httpmock.StatusStringResponse(http.StatusNotFound, `{"message":"Not Found"}`))
reg.Register(
httpmock.REST("GET", "users/jillvalentine/received_events"),
httpmock.StringResponse(`[]`))
},
opts: &StatusOptions{},
wantOut: "Assigned Issues │ Assigned Pull Requests \nNothing here ^_^ │ Nothing here ^_^ \n │ \nReview Requests │ Mentions \nNothing here ^_^ │ Nothing here ^_^ \n │ \nRepository Activity\nNothing here ^_^\n\n",
},
{
name: "something",
httpStubs: func(reg *httpmock.Registry) {
Expand Down
Loading