Skip to content

Commit

Permalink
fix: nil apiresponses.FailureResponse should not panic
Browse files Browse the repository at this point in the history
Ideally folks should not create a nil apiresponses.FailureResponse, but
it is possible, and when that happens, calling methods on the nil
apiresponses.FailureResponse should not panic.

Relates to #252
  • Loading branch information
blgm committed Jun 6, 2023
1 parent fdb8510 commit f525b39
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
20 changes: 18 additions & 2 deletions domain/apiresponses/failure_responses.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewFailureResponse(err error, statusCode int, loggerAction string) *Failure
// ErrorResponse returns an interface{} which will be JSON encoded and form the body
// of the HTTP response
func (f *FailureResponse) ErrorResponse() interface{} {
if f.emptyResponse {
if f == nil || f.emptyResponse {
return EmptyResponse{}
}

Expand All @@ -46,7 +46,7 @@ func (f *FailureResponse) ErrorResponse() interface{} {
// ValidatedStatusCode returns the HTTP response status code. If the code is not 4xx
// or 5xx, an InternalServerError will be returned instead.
func (f *FailureResponse) ValidatedStatusCode(logger lager.Logger) int {
if f.statusCode < 400 || 600 <= f.statusCode {
if f == nil || f.statusCode < 400 || 600 <= f.statusCode {
if logger != nil {
logger.Error("validating-status-code", fmt.Errorf("Invalid failure http response code: 600, expected 4xx or 5xx, returning internal server error: 500."))
}
Expand All @@ -57,11 +57,19 @@ func (f *FailureResponse) ValidatedStatusCode(logger lager.Logger) int {

// LoggerAction returns the loggerAction, used as the action when logging
func (f *FailureResponse) LoggerAction() string {
if f == nil {
return ""
}

return f.loggerAction
}

// AppendErrorMessage returns an error with the message updated. All other properties are preserved.
func (f *FailureResponse) AppendErrorMessage(msg string) *FailureResponse {
if f == nil {
return &FailureResponse{error: fmt.Errorf("%s", msg)}
}

return &FailureResponse{
error: fmt.Errorf("%s %s", f.Error(), msg),
statusCode: f.statusCode,
Expand All @@ -71,6 +79,14 @@ func (f *FailureResponse) AppendErrorMessage(msg string) *FailureResponse {
}
}

// Error returns the error message
func (f *FailureResponse) Error() string {
if f == nil {
return "Error() called on uninitialized apiresponses.FailureResponse"
}
return f.error.Error()
}

// FailureResponseBuilder provides a fluent set of methods to build a *FailureResponse.
type FailureResponseBuilder struct {
error
Expand Down
26 changes: 26 additions & 0 deletions domain/apiresponses/failure_responses_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,30 @@ var _ = Describe("FailureResponse", func() {
Expect(failureResponse.LoggerAction()).To(Equal("log-key"))
})
})

Describe("nil", func() {
// Because a FailureResponse is public, it's possible to create a nil one.
// If someone does that, we should not panic.
var e *apiresponses.FailureResponse = nil

It("allows Error() to be called", func() {
Expect(e.Error()).To(Equal("Error() called on uninitialized apiresponses.FailureResponse"))
})

It("allows AppendErrorMessage() to be called", func() {
Expect(e.AppendErrorMessage("ok")).To(MatchError("ok"))
})

It("allows LoggerAction() to be called", func() {
Expect(e.LoggerAction()).To(BeEmpty())
})

It("allows ValidatedStatusCode() to be called", func() {
Expect(e.ValidatedStatusCode(nil)).To(Equal(http.StatusInternalServerError))
})

It("allows ErrorResponse() to be called", func() {
Expect(e.ErrorResponse()).To(Equal(apiresponses.EmptyResponse{}))
})
})
})

0 comments on commit f525b39

Please sign in to comment.