Skip to content

Commit

Permalink
New Error Scheme (#93)
Browse files Browse the repository at this point in the history
* New Error Scheme

* Linting

* Using as

* Get rid of useless field
  • Loading branch information
notanthony authored Dec 26, 2023
1 parent 6b1c443 commit 42d62d9
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 67 deletions.
4 changes: 2 additions & 2 deletions cmd/cone/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"os/signal"
"syscall"

"github.com/conductorone/cone/pkg/output"
"github.com/conductorone/cone/pkg/client"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -74,7 +74,7 @@ func runCli(ctx context.Context) int {
err = cliCmd.ExecuteContext(ctx)
if err != nil {
_, _, v, _ := cmdContext(cliCmd)
fmt.Fprintln(os.Stderr, output.HandleErrors(ctx, v, err))
fmt.Fprintln(os.Stderr, client.HandleErrors(ctx, v, err))
return 1
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/client/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (c *client) GetApp(ctx context.Context, appID string) (*shared.App, error)
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}
return resp.GetAppResponse.App, nil
Expand All @@ -33,7 +33,7 @@ func (c *client) ListApps(ctx context.Context) ([]shared.App, error) {
if err != nil {
return nil, err
}
if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down
17 changes: 0 additions & 17 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package client

import (
"context"
"fmt"
"io"
"net/http"
"net/url"

Expand Down Expand Up @@ -131,18 +129,3 @@ func New(

return c, nil
}

func handleBadStatus(resp *http.Response) error {
// This is added temporarily to ensure we return an error if we get a non-success status code.
// Eventually (ideally), we'll be generating this error handling as part of the SDK
if resp.StatusCode >= http.StatusBadRequest {
body, err := io.ReadAll(resp.Body)
if err != nil {
return err
}

return fmt.Errorf("status %d: %s", resp.StatusCode, string(body))
}

return nil
}
7 changes: 3 additions & 4 deletions pkg/client/entitlement.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (c *client) SearchEntitlements(ctx context.Context, filter *SearchEntitleme
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down Expand Up @@ -174,8 +174,7 @@ func (c *client) GetEntitlement(ctx context.Context, appId string, entitlementId
if err != nil {
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down Expand Up @@ -203,7 +202,7 @@ func (c *client) ListEntitlements(ctx context.Context, appId string) ([]shared.A
if err != nil {
return nil, err
}
if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down
73 changes: 73 additions & 0 deletions pkg/client/error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package client

import (
"context"
"errors"
"fmt"
"io"
"net/http"

"github.com/conductorone/cone/pkg/output"
"github.com/spf13/viper"
)

const defaultJSONError = `{"error": "unable to marshal error to JSON %s"}`

type JSONError struct {
Error string `json:"error"`
}

type HTTPError struct {
StatusCode int `json:"status_code"`
Body string `json:"body"`
}

func NewHTTPError(resp *http.Response) *HTTPError {
// This is added temporarily to ensure we return an error if we get a non-success status code.
// Eventually (ideally), we'll be generating this error handling as part of the SDK
if resp.StatusCode >= http.StatusBadRequest {
var httpErr HTTPError
body, err := io.ReadAll(resp.Body)
if err != nil {
httpErr = HTTPError{
StatusCode: resp.StatusCode,
Body: fmt.Errorf("unable to read response body: %w", err).Error(),
}
} else {
httpErr = HTTPError{
StatusCode: resp.StatusCode,
Body: string(body),
}
}
return &httpErr
}

return nil
}

func (e *HTTPError) Error() string {
return fmt.Sprintf("%d: %s", e.StatusCode, e.Body)
}

func HandleErrors(ctx context.Context, v *viper.Viper, input error) error {
outputType := v.GetString("output")
if outputType != "json" && outputType != output.JSONPretty {
return input
}
var jsonError []byte

var httpErr *HTTPError
if errors.As(input, &httpErr) {
jsonError, err := output.MakeJSONFromInterface(ctx, httpErr, outputType == output.JSONPretty)
if err != nil {
return fmt.Errorf(defaultJSONError, httpErr.Error())
}
return fmt.Errorf(string(jsonError))
}
jsonError, err := output.MakeJSONFromInterface(ctx, JSONError{Error: input.Error()}, outputType == output.JSONPretty)
if err != nil {
return fmt.Errorf(defaultJSONError, input.Error())
}

return fmt.Errorf(string(jsonError))
}
2 changes: 1 addition & 1 deletion pkg/client/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (c *client) GetGrantsForIdentity(ctx context.Context, appID string, appEnti
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/client/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (c *client) ListPolicies(ctx context.Context) ([]shared.Policy, error) {
if err != nil {
return nil, err
}
if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/client/resource_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func (c *client) GetResourceType(ctx context.Context, appID string, resourceType
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}
v := resp.AppResourceTypeServiceGetResponse.AppResourceTypeView
Expand All @@ -43,7 +43,7 @@ func (c *client) GetResource(ctx context.Context, appID string, resourceTypeID s
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}
v := resp.AppResourceServiceGetResponse.AppResourceView
Expand Down
16 changes: 8 additions & 8 deletions pkg/client/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ func (c *client) GetTask(ctx context.Context, taskId string) (*shared.TaskServic
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down Expand Up @@ -44,7 +44,7 @@ func (c *client) CreateGrantTask(
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand All @@ -69,7 +69,7 @@ func (c *client) CreateRevokeTask(
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand All @@ -82,7 +82,7 @@ func (c *client) SearchTasks(ctx context.Context, taskFilter shared.TaskSearchRe
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand All @@ -100,7 +100,7 @@ func (c *client) CommentOnTask(ctx context.Context, taskID string, comment strin
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}
return resp.TaskActionsServiceCommentResponse, nil
Expand All @@ -118,7 +118,7 @@ func (c *client) ApproveTask(ctx context.Context, taskId string, comment string,
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}
return resp.TaskActionsServiceApproveResponse, nil
Expand All @@ -136,7 +136,7 @@ func (c *client) DenyTask(ctx context.Context, taskId string, comment string, po
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}
return resp.TaskActionsServiceDenyResponse, nil
Expand All @@ -151,7 +151,7 @@ func (c *client) EscalateTask(ctx context.Context, taskID string) (*shared.TaskS
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}
return resp.TaskServiceActionResponse, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func (c *client) GetUser(ctx context.Context, userID string) (*shared.User, erro
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/client/whoami.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ func (c *client) AuthIntrospect(ctx context.Context) (*shared.IntrospectResponse
return nil, err
}

if err := handleBadStatus(resp.RawResponse); err != nil {
if err := NewHTTPError(resp.RawResponse); err != nil {
return nil, err
}

Expand Down
28 changes: 0 additions & 28 deletions pkg/output/error.go

This file was deleted.

0 comments on commit 42d62d9

Please sign in to comment.