From 073ec3426be4e36fbd125d65f8a6aa76510eb85c Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Sun, 24 Sep 2023 09:22:55 -0700 Subject: [PATCH] Return HTTP errors properly for some commands (#8037) * Return HTTP error for `run watch` Partially fixes #8026 * Return HTTP error for `gpg-key delete` Partially fixes #8026 --- pkg/cmd/gpg-key/delete/delete.go | 2 +- pkg/cmd/gpg-key/delete/delete_test.go | 14 ++++++++++++++ pkg/cmd/run/watch/watch.go | 2 +- pkg/cmd/run/watch/watch_test.go | 28 +++++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gpg-key/delete/delete.go b/pkg/cmd/gpg-key/delete/delete.go index 0f152297a..f86957bdb 100644 --- a/pkg/cmd/gpg-key/delete/delete.go +++ b/pkg/cmd/gpg-key/delete/delete.go @@ -91,7 +91,7 @@ func deleteRun(opts *DeleteOptions) error { err = deleteGPGKey(httpClient, host, id) if err != nil { - return nil + return err } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/gpg-key/delete/delete_test.go b/pkg/cmd/gpg-key/delete/delete_test.go index a7f0fda67..c9c29aa3c 100644 --- a/pkg/cmd/gpg-key/delete/delete_test.go +++ b/pkg/cmd/gpg-key/delete/delete_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/go-gh/v2/pkg/api" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -170,6 +171,19 @@ func Test_deleteRun(t *testing.T) { wantErr: true, wantErrMsg: "unable to delete GPG key ABC123: either the GPG key is not found or it is not owned by you", }, + { + name: "delete failed", + opts: DeleteOptions{KeyID: "ABC123", Confirmed: true}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "user/gpg_keys"), httpmock.StatusStringResponse(200, keysResp)) + reg.Register(httpmock.REST("DELETE", "user/gpg_keys/123"), httpmock.StatusJSONResponse(404, api.HTTPError{ + StatusCode: 404, + Message: "GPG key 123 not found", + })) + }, + wantErr: true, + wantErrMsg: "HTTP 404: GPG key 123 not found (https://api.github.com/user/gpg_keys/123)", + }, } for _, tt := range tests { diff --git a/pkg/cmd/run/watch/watch.go b/pkg/cmd/run/watch/watch.go index 090737557..48d50ed57 100644 --- a/pkg/cmd/run/watch/watch.go +++ b/pkg/cmd/run/watch/watch.go @@ -173,7 +173,7 @@ func watchRun(opts *WatchOptions) error { opts.IO.StopAlternateScreenBuffer() if err != nil { - return nil + return err } // Write the last temporary buffer one last time diff --git a/pkg/cmd/run/watch/watch_test.go b/pkg/cmd/run/watch/watch_test.go index 8ff77516b..a73fd2949 100644 --- a/pkg/cmd/run/watch/watch_test.go +++ b/pkg/cmd/run/watch/watch_test.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/go-gh/v2/pkg/api" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -298,6 +299,33 @@ func TestWatchRun(t *testing.T) { wantErr: true, errMsg: "SilentError", }, + { + name: "failed to get run status", + tty: true, + opts: &WatchOptions{ + RunID: "1234", + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.JSONResponse(shared.TestRunWithCommit(1234, shared.InProgress, "", "commit2")), + ) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/workflows/123"), + httpmock.JSONResponse(shared.TestWorkflow), + ) + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"), + httpmock.StatusJSONResponse(404, api.HTTPError{ + StatusCode: 404, + Message: "run 1234 not found", + }), + ) + }, + wantOut: "\x1b[?1049h\x1b[?1049l", + wantErr: true, + errMsg: "failed to get run: HTTP 404: run 1234 not found (https://api.github.com/repos/OWNER/REPO/actions/runs/1234?exclude_pull_requests=true)", + }, } for _, tt := range tests {