Skip to content

Commit

Permalink
fix: fix log nil error when http request failed
Browse files Browse the repository at this point in the history
  • Loading branch information
fm committed Nov 21, 2022
1 parent d7406e8 commit 56c7940
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 8 deletions.
5 changes: 4 additions & 1 deletion errors/errorhandling.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ func AsStatusCode(err error) int {

// AsStatusError transform resty response to status error
func AsStatusError(response *resty.Response, grs ...schema.GroupResource) error {

// adding GroupResource as a "optional" parameter only
// should never provide more than one
gr := RESTClientGroupResource
Expand All @@ -66,7 +67,9 @@ func AsStatusError(response *resty.Response, grs ...schema.GroupResource) error
)

if err, ok := response.Error().(error); ok {
statusError.ErrStatus.Message = err.Error()
if originalErr := err.Error(); originalErr != "" {
statusError.ErrStatus.Message = originalErr
}
}

return statusError
Expand Down
113 changes: 111 additions & 2 deletions errors/errorhandling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@ import (
goerrors "errors"
"fmt"
"net/http"

"net/http/httptest"
"testing"

"github.com/go-resty/resty/v2"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand All @@ -49,3 +50,111 @@ func TestAsStatusCode(t *testing.T) {
g.Expect(AsStatusCode(fmt.Errorf("anyerror"))).To(Equal(http.StatusInternalServerError))
g.Expect(AsStatusCode(errors.NewBadRequest("bad"))).To(Equal(http.StatusBadRequest))
}

var _ = Describe("Test AsStatusError", func() {
var (
server *httptest.Server
response *resty.Response
)

newTestServer := func(f func(w http.ResponseWriter, r *http.Request)) {
mux := http.NewServeMux()
server = httptest.NewServer(mux)
mux.HandleFunc("/test", f)
}

BeforeEach(func() {
server = &httptest.Server{}
response = &resty.Response{}
})

JustBeforeEach(func() {
response, _ = resty.New().R().Get(server.URL + "/test")
})

When("server return 200", func() {
BeforeEach(func() {
newTestServer(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
})
DeferCleanup(func() {
server.Close()
})
})

It("should return nil", func() {
Expect(response.StatusCode()).Should(Equal(http.StatusOK))
Expect(AsStatusError(response)).To(BeNil())
})
})

When("server return 301", func() {
BeforeEach(func() {
newTestServer(func(w http.ResponseWriter, r *http.Request) {
w.Header().Add("Location", "https://127.0.0.1")
w.WriteHeader(http.StatusMovedPermanently)
})
DeferCleanup(func() {
server.Close()
})
})

It("rawResponse should be nil", func() {
Expect(response.RawResponse).To(BeNil())
})
})

When("server return 400", func() {
BeforeEach(func() {
newTestServer(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte("test body"))
})
DeferCleanup(func() {
server.Close()
})
})

It("should return 400", func() {
Expect(response.StatusCode()).To(Equal(http.StatusBadRequest))
statusError := AsStatusError(response).(*errors.StatusError)
Expect(int(statusError.ErrStatus.Code)).To(Equal(http.StatusBadRequest))
Expect(statusError.ErrStatus.Message).To(ContainSubstring("the server rejected our request for an unknown reason"))
})
})
When("server return 403", func() {
BeforeEach(func() {
newTestServer(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
w.Write([]byte("test forbidden error"))
})
DeferCleanup(func() {
server.Close()
})
})

It("should return 403", func() {
Expect(response.StatusCode()).To(Equal(http.StatusForbidden))
statusError := AsStatusError(response).(*errors.StatusError)
Expect(int(statusError.ErrStatus.Code)).To(Equal(http.StatusForbidden))
Expect(statusError.ErrStatus.Message).To(ContainSubstring("test forbidden error"))
})
})
When("server return 500", func() {
BeforeEach(func() {
newTestServer(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
})
DeferCleanup(func() {
server.Close()
})
})

It("should return 500", func() {
Expect(response.StatusCode()).To(Equal(http.StatusInternalServerError))
statusError := AsStatusError(response).(*errors.StatusError)
Expect(int(statusError.ErrStatus.Code)).To(Equal(http.StatusInternalServerError))
Expect(statusError.ErrStatus.Message).To(ContainSubstring("an error on the server (\"\") has prevented the request from succeeding"))
})
})
})
9 changes: 4 additions & 5 deletions plugin/client/plugin_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,18 @@ import (
"strings"

"github.com/go-resty/resty/v2"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
duckv1 "knative.dev/pkg/apis/duck/v1"

metav1alpha1 "github.com/katanomi/pkg/apis/meta/v1alpha1"
"github.com/katanomi/pkg/client"
perrors "github.com/katanomi/pkg/errors"
"github.com/katanomi/pkg/tracing"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
duckv1 "knative.dev/pkg/apis/duck/v1"
)

var (
defaultOptions = []OptionFunc{
ErrorOpts(&errors.StatusError{}),
ErrorOpts(&metav1.Status{}),
HeaderOpts("Content-Type", "application/json"),
}
)
Expand Down

0 comments on commit 56c7940

Please sign in to comment.