From 708925b4978f85539fffca513c84434f00b5d08f Mon Sep 17 00:00:00 2001 From: Danny Olson Date: Thu, 28 Mar 2024 11:23:50 -0700 Subject: [PATCH] feat: better flag create error handling (#85) Handle 401 error consistent with other errors --- internal/errors/errors.go | 73 +++++++++++++++++++++++++++++++--- internal/errors/errors_test.go | 57 ++++++++++++++++++++++++++ internal/flags/client.go | 4 +- internal/members/members.go | 2 +- internal/projects/projects.go | 4 +- 5 files changed, 129 insertions(+), 11 deletions(-) create mode 100644 internal/errors/errors_test.go diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 2ca6ccda..b37143eb 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -1,12 +1,11 @@ package errors import ( + "encoding/json" "fmt" "github.com/pkg/errors" - ldapi "github.com/launchdarkly/api-client-go/v14" - "ldcli/cmd/cliflags" ) @@ -45,12 +44,74 @@ func NewErrorWrapped(message string, underlying error) error { }) } -func NewAPIError(err error) error { - var ldErr *ldapi.GenericOpenAPIError - ok := errors.As(err, &ldErr) +// LDAPIError is an error from the LaunchDarkly API client. +type LDAPIError interface { + Body() []byte + Error() string + Model() interface{} +} + +type APIError struct { + body []byte + err error + model interface{} +} + +func NewAPIError(body []byte, err error, model interface{}) APIError { + return APIError{ + body: body, + err: err, + model: model, + } +} + +func (e APIError) Error() string { + return e.err.Error() +} + +func (e APIError) Body() []byte { + return e.body +} + +func (e APIError) Model() interface{} { + return e.model +} + +// NewLDAPIError converts the error returned from API calls to LaunchDarkly to have a +// consistent Error() JSON structure. +func NewLDAPIError(err error) error { + var apiErr LDAPIError + ok := errors.As(err, &apiErr) if ok { - return NewErrorWrapped(string(ldErr.Body()), ldErr) + // the 401 response does not have a body, so we need to create one for a + // consistent response + if err.Error() == "401 Unauthorized" { + errMsg, err := normalizeUnauthorizedJSON() + if err != nil { + return err + } + + return NewError(string(errMsg)) + } + + return NewErrorWrapped(string(apiErr.Body()), apiErr) } return err } + +func normalizeUnauthorizedJSON() ([]byte, error) { + e := struct { + Code string `json:"code"` + Message string `json:"message"` + }{ + Code: "unauthorized", + Message: "You do not have access to perform this action", + } + errMsg, err := json.Marshal(e) + if err != nil { + return nil, err + } + + return errMsg, nil +} diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go new file mode 100644 index 00000000..bcbead87 --- /dev/null +++ b/internal/errors/errors_test.go @@ -0,0 +1,57 @@ +package errors_test + +import ( + "encoding/json" + "errors" + "testing" + + ldapi "github.com/launchdarkly/api-client-go/v14" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + errs "ldcli/internal/errors" +) + +func TestAPIError(t *testing.T) { + t.Run("with a 400 error has a JSON response", func(t *testing.T) { + underlying := errs.NewAPIError( + []byte(`{"code":"conflict","message":"an error"}`), + errors.New("409 Conflict"), + []string{}, + ) + + err := errs.NewLDAPIError(underlying) + + require.Error(t, err) + assert.JSONEq(t, `{"code": "conflict", "message": "an error"}`, err.Error()) + }) + + t.Run("with a 401 error has a JSON response", func(t *testing.T) { + rep := ldapi.UnauthorizedErrorRep{} + repBytes, err := json.Marshal(rep) + require.NoError(t, err) + underlying := errs.NewAPIError( + repBytes, + errors.New("401 Unauthorized"), + []string{}, + ) + + err = errs.NewLDAPIError(underlying) + + require.Error(t, err) + assert.JSONEq(t, `{"code": "unauthorized", "message": "You do not have access to perform this action"}`, err.Error()) + }) + + t.Run("with a 403 error has a JSON response", func(t *testing.T) { + underlying := errs.NewAPIError( + []byte(`{"code": "forbidden", "message": "an error"}`), + errors.New("409 Conflict"), + []string{}, + ) + + err := errs.NewLDAPIError(underlying) + + require.Error(t, err) + assert.JSONEq(t, `{"code": "forbidden", "message": "an error"}`, err.Error()) + }) +} diff --git a/internal/flags/client.go b/internal/flags/client.go index 22a090ee..c9c80e67 100644 --- a/internal/flags/client.go +++ b/internal/flags/client.go @@ -42,7 +42,7 @@ func (c FlagsClient) Create( post := ldapi.NewFeatureFlagBody(name, key) flag, _, err := client.FeatureFlagsApi.PostFeatureFlag(ctx, projectKey).FeatureFlagBody(*post).Execute() if err != nil { - return nil, errors.NewAPIError(err) + return nil, errors.NewLDAPIError(err) } @@ -68,7 +68,7 @@ func (c FlagsClient) Update( PatchWithComment(*ldapi.NewPatchWithComment(patch)). Execute() if err != nil { - return nil, errors.NewAPIError(err) + return nil, errors.NewLDAPIError(err) } responseJSON, err := json.Marshal(flag) diff --git a/internal/members/members.go b/internal/members/members.go index e050dddf..e17eb1ac 100644 --- a/internal/members/members.go +++ b/internal/members/members.go @@ -25,7 +25,7 @@ func (c MembersClient) Create(ctx context.Context, accessToken, baseURI, email, memberForm := ldapi.NewMemberForm{Email: email, Role: &role} members, _, err := client.AccountMembersApi.PostMembers(ctx).NewMemberForm([]ldapi.NewMemberForm{memberForm}).Execute() if err != nil { - return nil, errors.NewAPIError(err) + return nil, errors.NewLDAPIError(err) } memberJson, err := json.Marshal(members.Items[0]) if err != nil { diff --git a/internal/projects/projects.go b/internal/projects/projects.go index f0c3eb11..3586b0d1 100644 --- a/internal/projects/projects.go +++ b/internal/projects/projects.go @@ -34,7 +34,7 @@ func (c ProjectsClient) Create( projectPost := ldapi.NewProjectPost(name, key) project, _, err := client.ProjectsApi.PostProject(ctx).ProjectPost(*projectPost).Execute() if err != nil { - return nil, errors.NewAPIError(err) + return nil, errors.NewLDAPIError(err) } projectJSON, err := json.Marshal(project) if err != nil { @@ -55,7 +55,7 @@ func (c ProjectsClient) List( Limit(2). Execute() if err != nil { - return nil, errors.NewAPIError(err) + return nil, errors.NewLDAPIError(err) } projectsJSON, err := json.Marshal(projects)