Skip to content

Commit

Permalink
Remove Error logging in Grafton (#119)
Browse files Browse the repository at this point in the history 
We handle these errors in marketplace, and trust marketplace to be the source of true errors.
Grafton was logging errors and this generated rollbar events when there wasn't an issue because the error was handled by marketplace.
This PR removes those error logs.
  • Loading branch information
@tim-speed
tim-speed authored May 2, 2019
1 parent 81dbe18 commit 3eaee40
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 46 deletions.
30 changes: 0 additions & 30 deletions client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,6 @@ func (c *Client) ProvisionResource(ctx context.Context, cbID manifold.ID,
return "", false, err
}

if graftonErr == ErrMissingMsg {
c.log.WithError(err).Error("Missing error message in response")
return "", false, graftonErr
}

return graftonErr.Error(), false, graftonErr
}

Expand Down Expand Up @@ -222,11 +217,6 @@ func (c *Client) ProvisionCredentials(ctx context.Context, cbID, resID, credID m
return nil, "", false, err
}

if graftonErr == ErrMissingMsg {
c.log.WithError(err).Error("Missing error message in response")
return nil, "", false, graftonErr
}

return nil, graftonErr.Error(), false, graftonErr
}

Expand Down Expand Up @@ -294,11 +284,6 @@ func (c *Client) ChangePlan(ctx context.Context, cbID, resourceID manifold.ID, n
return "", false, err
}

if graftonErr == ErrMissingMsg {
c.log.WithError(err).Error("No error message in response")
return "", false, graftonErr
}

return graftonErr.Error(), false, graftonErr
}

Expand Down Expand Up @@ -355,11 +340,6 @@ func (c *Client) DeprovisionCredentials(ctx context.Context, cbID, credentialID
return "", false, err
}

if graftonErr == ErrMissingMsg {
c.log.WithError(err).Error("No error message in response")
return "", false, graftonErr
}

return graftonErr.Error(), false, graftonErr
}

Expand Down Expand Up @@ -410,11 +390,6 @@ func (c *Client) DeprovisionResource(ctx context.Context, cbID, resourceID manif
return "", false, err
}

if graftonErr == ErrMissingMsg {
c.log.WithError(err).Error("No error message in response")
return "", false, graftonErr
}

return graftonErr.Error(), false, graftonErr
}

Expand Down Expand Up @@ -480,11 +455,6 @@ func (c *Client) PullResourceMeasures(ctx context.Context, rid manifold.ID,
return nil, err
}

if graftonErr == ErrMissingMsg {
c.log.WithError(err).Error("Missing error message in response")
return nil, graftonErr
}

return nil, graftonErr
}

Expand Down
35 changes: 20 additions & 15 deletions client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ func TestProvisionResource(t *testing.T) {

msg, _, err := callProvision(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(err).To(gm.MatchError(ErrMissingMsg))
expectedMessage := typeToNiceString(errors.BadRequestError)
gm.Expect(msg).To(gm.Equal(expectedMessage))
gm.Expect(err).To(gm.MatchError(NewError(errors.BadRequestError, expectedMessage)))
})

t.Run("401 unauthorized valid response", withCode(http.StatusUnauthorized, func(url string) {
Expand Down Expand Up @@ -327,7 +328,7 @@ func TestProvisionResource(t *testing.T) {

msg, async, err := callProvision(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(msg).To(gm.Equal(typeToNiceString(errors.InternalServerError)))
gm.Expect(IsFatal(err)).To(gm.BeFalse())
gm.Expect(async).To(gm.BeFalse())
})
Expand Down Expand Up @@ -470,8 +471,9 @@ func TestProvisionCredentials(t *testing.T) {

_, msg, _, err := callProvisionCredentials(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(err).To(gm.MatchError(ErrMissingMsg))
expectedMessage := typeToNiceString(errors.BadRequestError)
gm.Expect(msg).To(gm.Equal(expectedMessage))
gm.Expect(err).To(gm.MatchError(NewError(errors.BadRequestError, expectedMessage)))
})

t.Run("404 not found valid response", withCode(http.StatusNotFound, func(url string) {
Expand Down Expand Up @@ -505,7 +507,7 @@ func TestProvisionCredentials(t *testing.T) {

_, msg, async, err := callProvisionCredentials(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(msg).To(gm.Equal(typeToNiceString(errors.InternalServerError)))
gm.Expect(IsFatal(err)).To(gm.BeFalse())
gm.Expect(async).To(gm.BeFalse())
})
Expand Down Expand Up @@ -630,8 +632,9 @@ func TestChangePlan(t *testing.T) {

msg, _, err := callChangePlan(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(err).To(gm.MatchError(ErrMissingMsg))
expectedMessage := typeToNiceString(errors.BadRequestError)
gm.Expect(msg).To(gm.Equal(expectedMessage))
gm.Expect(err).To(gm.MatchError(NewError(errors.BadRequestError, expectedMessage)))
})

t.Run("401 bad unauthorized valid response", withCode(http.StatusUnauthorized, func(url string) {
Expand Down Expand Up @@ -665,7 +668,7 @@ func TestChangePlan(t *testing.T) {

msg, async, err := callChangePlan(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(msg).To(gm.Equal(typeToNiceString(errors.InternalServerError)))
gm.Expect(IsFatal(err)).To(gm.BeFalse())
gm.Expect(async).To(gm.BeFalse())
})
Expand Down Expand Up @@ -753,8 +756,9 @@ func TestDeprovisionCredentials(t *testing.T) {

msg, _, err := callDeprovisionCredentials(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(err).To(gm.MatchError(ErrMissingMsg))
expectedMessage := typeToNiceString(errors.BadRequestError)
gm.Expect(msg).To(gm.Equal(expectedMessage))
gm.Expect(err).To(gm.MatchError(NewError(errors.BadRequestError, expectedMessage)))
})

t.Run("401 unauthorized valid response", withCode(http.StatusUnauthorized, func(url string) {
Expand Down Expand Up @@ -788,7 +792,7 @@ func TestDeprovisionCredentials(t *testing.T) {

msg, async, err := callDeprovisionCredentials(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(msg).To(gm.Equal(typeToNiceString(errors.InternalServerError)))
gm.Expect(IsFatal(err)).To(gm.BeFalse())
gm.Expect(async).To(gm.BeFalse())
})
Expand Down Expand Up @@ -876,8 +880,9 @@ func TestDeprovisionResource(t *testing.T) {

msg, _, err := callDeprovisionResource(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(err).To(gm.MatchError(ErrMissingMsg))
expectedMessage := typeToNiceString(errors.BadRequestError)
gm.Expect(msg).To(gm.Equal(expectedMessage))
gm.Expect(err).To(gm.MatchError(NewError(errors.BadRequestError, expectedMessage)))
})

t.Run("400 unauthorized valid response", withCode(http.StatusUnauthorized, func(url string) {
Expand Down Expand Up @@ -911,7 +916,7 @@ func TestDeprovisionResource(t *testing.T) {

msg, async, err := callDeprovisionResource(srv.URL)

gm.Expect(msg).To(gm.Equal(""))
gm.Expect(msg).To(gm.Equal(typeToNiceString(errors.InternalServerError)))
gm.Expect(IsFatal(err)).To(gm.BeFalse())
gm.Expect(async).To(gm.BeFalse())
})
Expand Down
10 changes: 9 additions & 1 deletion errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package grafton

import (
"errors"
"fmt"
"net/http"
"strings"

swagerrs "github.com/go-openapi/errors"
"github.com/go-openapi/runtime"
Expand All @@ -14,11 +16,17 @@ import (
// ErrMissingMsg occurs when a provider's response is missing the Message field
var ErrMissingMsg = errors.New("`message` field was missing from the response")

func typeToNiceString(t merrors.Type) string {
nice := strings.Replace(string(t), "_", " ", -1)
return fmt.Sprintf("%d - %s%s", t.Code(), strings.ToUpper(nice[:1]), nice[1:])
}

// NewErrWithMsg creates a new Error from a string pointer, if the pointer is
// nil then an ErrMissingMsg is returned instead.
func NewErrWithMsg(t merrors.Type, m *string) error {
if m == nil {
return ErrMissingMsg
// Substitute the message with a human readable version of the type
return NewError(t, typeToNiceString(t))
}

return NewError(t, *m)
Expand Down

0 comments on commit 3eaee40

Please sign in to comment.