From 59d6343da9c5623a8d6621f972e628c4dd860da9 Mon Sep 17 00:00:00 2001 From: kpacha Date: Tue, 2 Oct 2018 19:44:45 +0200 Subject: [PATCH 01/10] create a response from the received http error --- proxy/http.go | 16 +++++++++++++++- proxy/http_status.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/proxy/http.go b/proxy/http.go index 9ba7d96e8..9a66e1730 100644 --- a/proxy/http.go +++ b/proxy/http.go @@ -38,9 +38,20 @@ func NewHTTPProxyWithHTTPExecutor(remote *config.Backend, requestExecutor HTTPRe return NewHTTPProxyDetailed(remote, requestExecutor, NoOpHTTPStatusHandler, NoOpHTTPResponseParser) } + sh := DefaultHTTPStatusHandler + if e, ok := remote.ExtraConfig[Namespace]; ok { + if m, ok := e.(map[string]interface{}); ok { + if v, ok := m["return_error_details"]; ok { + if b, ok := v.(bool); ok && b { + sh = DetailedHTTPStatusHandler + } + } + } + } + ef := NewEntityFormatter(remote) rp := DefaultHTTPResponseParserFactory(HTTPResponseParserConfig{dec, ef}) - return NewHTTPProxyDetailed(remote, requestExecutor, DefaultHTTPStatusHandler, rp) + return NewHTTPProxyDetailed(remote, requestExecutor, sh, rp) } // NewHTTPProxyDetailed creates a http proxy with the injected configuration, HTTPRequestExecutor, Decoder and HTTPResponseParser @@ -70,6 +81,9 @@ func NewHTTPProxyDetailed(remote *config.Backend, requestExecutor HTTPRequestExe resp, err = ch(ctx, resp) if err != nil { + if t, ok := err.(responseError); ok { + return t.Response(), err + } return nil, err } diff --git a/proxy/http_status.go b/proxy/http_status.go index c3213d69d..201a9ede5 100644 --- a/proxy/http_status.go +++ b/proxy/http_status.go @@ -2,6 +2,7 @@ package proxy import ( "context" + "io/ioutil" "net/http" ) @@ -21,3 +22,37 @@ func DefaultHTTPStatusHandler(ctx context.Context, resp *http.Response) (*http.R func NoOpHTTPStatusHandler(_ context.Context, resp *http.Response) (*http.Response, error) { return resp, nil } + +// DetailedHTTPStatusHandler is a HTTPStatusHandler implementation +func DetailedHTTPStatusHandler(ctx context.Context, resp *http.Response) (*http.Response, error) { + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { + body, _ := ioutil.ReadAll(resp.Body) + resp.Body.Close() + return resp, responseError{ + Code: resp.StatusCode, + Msg: string(body), + } + } + + return resp, nil +} + +type responseError struct { + Code int `json:"http_status_code"` + Msg string `json:"http_body,omitempty"` +} + +func (r responseError) Error() string { + return r.Msg +} + +func (r responseError) StatusCode() int { + return r.Code +} + +func (r responseError) Response() *Response { + return &Response{ + Data: map[string]interface{}{"error": r}, + Metadata: Metadata{StatusCode: r.Code}, + } +} From e5ebfa8cdaa5ac59bd424a5de21cc1f00cd1b397 Mon Sep 17 00:00:00 2001 From: kpacha Date: Tue, 2 Oct 2018 20:37:26 +0200 Subject: [PATCH 02/10] return the proper status code if there is an error and an empty or nil response --- router/gin/endpoint.go | 34 +++++++++++++++++++++------------- router/gin/render.go | 27 ++++++++++++++++----------- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/router/gin/endpoint.go b/router/gin/endpoint.go index 0406ca998..3e5a584f2 100644 --- a/router/gin/endpoint.go +++ b/router/gin/endpoint.go @@ -43,26 +43,35 @@ func CustomErrorEndpointHandler(configuration *config.EndpointConfig, prxy proxy default: } + complete := router.HeaderIncompleteResponseValue + if response != nil && len(response.Data) > 0 { if response.IsComplete { - c.Header(router.CompleteResponseHeaderName, router.HeaderCompleteResponseValue) + complete = router.HeaderCompleteResponseValue if isCacheEnabled { c.Header("Cache-Control", cacheControlHeaderValue) } - } else { - c.Header(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) } for k, v := range response.Metadata.Headers { c.Header(k, v[0]) } - } else { - if err != nil { - abort(c, errF(err)) + } + + c.Header(router.CompleteResponseHeaderName, complete) + + if err != nil { + c.Error(err) + + if response == nil { + if t, ok := err.(responseError); ok { + c.Status(t.StatusCode()) + } else { + c.Status(errF(err)) + } cancel() return } - c.Header(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) } render(c, response) @@ -70,12 +79,6 @@ func CustomErrorEndpointHandler(configuration *config.EndpointConfig, prxy proxy } } -func abort(c *gin.Context, code int) { - c.Status(code) - c.Header(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) - c.Abort() -} - // NewRequest gets a request from the current gin context and the received query string func NewRequest(headersToSend []string) func(*gin.Context, []string) *proxy.Request { if len(headersToSend) == 0 { @@ -115,3 +118,8 @@ func NewRequest(headersToSend []string) func(*gin.Context, []string) *proxy.Requ } } } + +type responseError interface { + Error() string + StatusCode() int +} diff --git a/router/gin/render.go b/router/gin/render.go index 0e49761d9..d3ce0a9d7 100644 --- a/router/gin/render.go +++ b/router/gin/render.go @@ -70,50 +70,55 @@ func negotiatedRender(c *gin.Context, response *proxy.Response) { } func stringRender(c *gin.Context, response *proxy.Response) { + status := c.Writer.Status() + if response == nil { - c.String(http.StatusOK, "") + c.String(status, "") return } d, ok := response.Data["content"] if !ok { - c.String(http.StatusOK, "") + c.String(status, "") return } msg, ok := d.(string) if !ok { - c.String(http.StatusOK, "") + c.String(status, "") return } - c.String(http.StatusOK, msg) + c.String(status, msg) } func jsonRender(c *gin.Context, response *proxy.Response) { + status := c.Writer.Status() if response == nil { - c.JSON(http.StatusOK, emptyResponse) + c.JSON(status, emptyResponse) return } - c.JSON(http.StatusOK, response.Data) + c.JSON(status, response.Data) } func xmlRender(c *gin.Context, response *proxy.Response) { + status := c.Writer.Status() if response == nil { - c.XML(http.StatusOK, nil) + c.XML(status, nil) return } d, ok := response.Data["content"] if !ok { - c.XML(http.StatusOK, nil) + c.XML(status, nil) return } - c.XML(http.StatusOK, d) + c.XML(status, d) } func yamlRender(c *gin.Context, response *proxy.Response) { + status := c.Writer.Status() if response == nil { - c.YAML(http.StatusOK, emptyResponse) + c.YAML(status, emptyResponse) return } - c.YAML(http.StatusOK, response.Data) + c.YAML(status, response.Data) } func noopRender(c *gin.Context, response *proxy.Response) { From 230c9ef1cdbd5ec35ac214e932267808c1112e53 Mon Sep 17 00:00:00 2001 From: kpacha Date: Tue, 2 Oct 2018 20:54:09 +0200 Subject: [PATCH 03/10] response error interface added, so external HTTPStatusHandler implementations can be easily introduced --- proxy/http.go | 6 ++++++ proxy/http_status.go | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/proxy/http.go b/proxy/http.go index 9a66e1730..dcbd3812a 100644 --- a/proxy/http.go +++ b/proxy/http.go @@ -106,3 +106,9 @@ func NewRequestBuilderMiddleware(remote *config.Backend) Middleware { } } } + +type responseError interface { + Error() string + StatusCode() int + Response() *Response +} diff --git a/proxy/http_status.go b/proxy/http_status.go index 201a9ede5..7a55af68c 100644 --- a/proxy/http_status.go +++ b/proxy/http_status.go @@ -28,7 +28,7 @@ func DetailedHTTPStatusHandler(ctx context.Context, resp *http.Response) (*http. if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { body, _ := ioutil.ReadAll(resp.Body) resp.Body.Close() - return resp, responseError{ + return resp, HTTPResponseError{ Code: resp.StatusCode, Msg: string(body), } @@ -37,20 +37,20 @@ func DetailedHTTPStatusHandler(ctx context.Context, resp *http.Response) (*http. return resp, nil } -type responseError struct { +type HTTPResponseError struct { Code int `json:"http_status_code"` Msg string `json:"http_body,omitempty"` } -func (r responseError) Error() string { +func (r HTTPResponseError) Error() string { return r.Msg } -func (r responseError) StatusCode() int { +func (r HTTPResponseError) StatusCode() int { return r.Code } -func (r responseError) Response() *Response { +func (r HTTPResponseError) Response() *Response { return &Response{ Data: map[string]interface{}{"error": r}, Metadata: Metadata{StatusCode: r.Code}, From c1f53c3e7145beea8b2dcfa0251f0a3446a4fa20 Mon Sep 17 00:00:00 2001 From: kpacha Date: Wed, 3 Oct 2018 15:08:16 +0200 Subject: [PATCH 04/10] HTTP status handler selector moved to a dedicated function --- proxy/http.go | 13 +------------ proxy/http_status.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/proxy/http.go b/proxy/http.go index dcbd3812a..bf333d2f7 100644 --- a/proxy/http.go +++ b/proxy/http.go @@ -38,20 +38,9 @@ func NewHTTPProxyWithHTTPExecutor(remote *config.Backend, requestExecutor HTTPRe return NewHTTPProxyDetailed(remote, requestExecutor, NoOpHTTPStatusHandler, NoOpHTTPResponseParser) } - sh := DefaultHTTPStatusHandler - if e, ok := remote.ExtraConfig[Namespace]; ok { - if m, ok := e.(map[string]interface{}); ok { - if v, ok := m["return_error_details"]; ok { - if b, ok := v.(bool); ok && b { - sh = DetailedHTTPStatusHandler - } - } - } - } - ef := NewEntityFormatter(remote) rp := DefaultHTTPResponseParserFactory(HTTPResponseParserConfig{dec, ef}) - return NewHTTPProxyDetailed(remote, requestExecutor, sh, rp) + return NewHTTPProxyDetailed(remote, requestExecutor, getHTTPStatusHandler(remote), rp) } // NewHTTPProxyDetailed creates a http proxy with the injected configuration, HTTPRequestExecutor, Decoder and HTTPResponseParser diff --git a/proxy/http_status.go b/proxy/http_status.go index 7a55af68c..9086a7fe9 100644 --- a/proxy/http_status.go +++ b/proxy/http_status.go @@ -4,11 +4,26 @@ import ( "context" "io/ioutil" "net/http" + + "github.com/devopsfaith/krakend/config" ) // HTTPStatusHandler defines how we tread the http response code type HTTPStatusHandler func(context.Context, *http.Response) (*http.Response, error) +func getHTTPStatusHandler(remote *config.Backend) HTTPStatusHandler { + if e, ok := remote.ExtraConfig[Namespace]; ok { + if m, ok := e.(map[string]interface{}); ok { + if v, ok := m["return_error_details"]; ok { + if b, ok := v.(bool); ok && b { + return DetailedHTTPStatusHandler + } + } + } + } + return DefaultHTTPStatusHandler +} + // DefaultHTTPStatusHandler is the default implementation of HTTPStatusHandler func DefaultHTTPStatusHandler(ctx context.Context, resp *http.Response) (*http.Response, error) { if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { From 5b35ebe8d7ab23c5b12e0b1d9eb55cf8839703c0 Mon Sep 17 00:00:00 2001 From: kpacha Date: Wed, 3 Oct 2018 16:11:34 +0200 Subject: [PATCH 05/10] basic test for the detailed http status handler added --- proxy/http.go | 13 +++-- proxy/http_status.go | 22 +++++--- proxy/http_status_test.go | 112 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 14 deletions(-) create mode 100644 proxy/http_status_test.go diff --git a/proxy/http.go b/proxy/http.go index bf333d2f7..01a62844f 100644 --- a/proxy/http.go +++ b/proxy/http.go @@ -33,18 +33,19 @@ func NewHTTPProxy(remote *config.Backend, clientFactory HTTPClientFactory, decod } // NewHTTPProxyWithHTTPExecutor creates a http proxy with the injected configuration, HTTPRequestExecutor and Decoder -func NewHTTPProxyWithHTTPExecutor(remote *config.Backend, requestExecutor HTTPRequestExecutor, dec encoding.Decoder) Proxy { +func NewHTTPProxyWithHTTPExecutor(remote *config.Backend, re HTTPRequestExecutor, dec encoding.Decoder) Proxy { if remote.Encoding == encoding.NOOP { - return NewHTTPProxyDetailed(remote, requestExecutor, NoOpHTTPStatusHandler, NoOpHTTPResponseParser) + return NewHTTPProxyDetailed(remote, re, NoOpHTTPStatusHandler, NoOpHTTPResponseParser) } ef := NewEntityFormatter(remote) rp := DefaultHTTPResponseParserFactory(HTTPResponseParserConfig{dec, ef}) - return NewHTTPProxyDetailed(remote, requestExecutor, getHTTPStatusHandler(remote), rp) + return NewHTTPProxyDetailed(remote, re, getHTTPStatusHandler(remote), rp) } -// NewHTTPProxyDetailed creates a http proxy with the injected configuration, HTTPRequestExecutor, Decoder and HTTPResponseParser -func NewHTTPProxyDetailed(remote *config.Backend, requestExecutor HTTPRequestExecutor, ch HTTPStatusHandler, rp HTTPResponseParser) Proxy { +// NewHTTPProxyDetailed creates a http proxy with the injected configuration, HTTPRequestExecutor, +// Decoder and HTTPResponseParser +func NewHTTPProxyDetailed(remote *config.Backend, re HTTPRequestExecutor, ch HTTPStatusHandler, rp HTTPResponseParser) Proxy { return func(ctx context.Context, request *Request) (*Response, error) { requestToBakend, err := http.NewRequest(request.Method, request.URL.String(), request.Body) if err != nil { @@ -57,7 +58,7 @@ func NewHTTPProxyDetailed(remote *config.Backend, requestExecutor HTTPRequestExe requestToBakend.Header[k] = tmp } - resp, err := requestExecutor(ctx, requestToBakend) + resp, err := re(ctx, requestToBakend) requestToBakend.Body.Close() select { case <-ctx.Done(): diff --git a/proxy/http_status.go b/proxy/http_status.go index 9086a7fe9..b3ac70f26 100644 --- a/proxy/http_status.go +++ b/proxy/http_status.go @@ -1,6 +1,7 @@ package proxy import ( + "bytes" "context" "io/ioutil" "net/http" @@ -40,16 +41,21 @@ func NoOpHTTPStatusHandler(_ context.Context, resp *http.Response) (*http.Respon // DetailedHTTPStatusHandler is a HTTPStatusHandler implementation func DetailedHTTPStatusHandler(ctx context.Context, resp *http.Response) (*http.Response, error) { - if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - body, _ := ioutil.ReadAll(resp.Body) - resp.Body.Close() - return resp, HTTPResponseError{ - Code: resp.StatusCode, - Msg: string(body), - } + if r, err := DefaultHTTPStatusHandler(ctx, resp); err == nil { + return r, nil } - return resp, nil + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + body = []byte{} + } + resp.Body.Close() + resp.Body = ioutil.NopCloser(bytes.NewBuffer(body)) + + return resp, HTTPResponseError{ + Code: resp.StatusCode, + Msg: string(body), + } } type HTTPResponseError struct { diff --git a/proxy/http_status_test.go b/proxy/http_status_test.go new file mode 100644 index 000000000..3da4e1258 --- /dev/null +++ b/proxy/http_status_test.go @@ -0,0 +1,112 @@ +package proxy + +import ( + "bytes" + "context" + "io/ioutil" + "net/http" + "reflect" + "testing" + + "github.com/devopsfaith/krakend/config" +) + +func TestDetailedHTTPStatusHandler(t *testing.T) { + cfg := &config.Backend{ + ExtraConfig: config.ExtraConfig{ + Namespace: map[string]interface{}{ + "return_error_details": true, + }, + }, + } + sh := getHTTPStatusHandler(cfg) + + for i, code := range statusCodes { + msg := http.StatusText(code) + + resp := &http.Response{ + StatusCode: code, + Body: ioutil.NopCloser(bytes.NewBufferString(msg)), + } + + r, err := sh(context.Background(), resp) + + if r != resp { + t.Errorf("#%d unexpected response: %v", i, r) + return + } + + e, ok := err.(HTTPResponseError) + if !ok { + t.Errorf("#%d unexpected error type %T: %s", i, err, err.Error()) + return + } + + if e.Code != code { + t.Errorf("#%d unexpected status code: %d", i, e.Code) + return + } + + if e.Msg != msg { + t.Errorf("#%d unexpected message: %s", i, e.Msg) + return + } + + expectedResponse := &Response{ + Data: map[string]interface{}{ + "error": HTTPResponseError{ + Code: code, + Msg: msg, + }, + }, + Metadata: Metadata{StatusCode: code}, + } + + if !reflect.DeepEqual(e.Response(), expectedResponse) { + t.Errorf("#%d unexpected response: %v", i, e.Response()) + } + } +} + +var statusCodes = []int{ + http.StatusBadRequest, + http.StatusUnauthorized, + http.StatusPaymentRequired, + http.StatusForbidden, + http.StatusNotFound, + http.StatusMethodNotAllowed, + http.StatusNotAcceptable, + http.StatusProxyAuthRequired, + http.StatusRequestTimeout, + http.StatusConflict, + http.StatusGone, + http.StatusLengthRequired, + http.StatusPreconditionFailed, + http.StatusRequestEntityTooLarge, + http.StatusRequestURITooLong, + http.StatusUnsupportedMediaType, + http.StatusRequestedRangeNotSatisfiable, + http.StatusExpectationFailed, + http.StatusTeapot, + http.StatusMisdirectedRequest, + http.StatusUnprocessableEntity, + http.StatusLocked, + http.StatusFailedDependency, + http.StatusUpgradeRequired, + http.StatusPreconditionRequired, + http.StatusTooManyRequests, + http.StatusRequestHeaderFieldsTooLarge, + http.StatusUnavailableForLegalReasons, + + http.StatusInternalServerError, + http.StatusNotImplemented, + http.StatusBadGateway, + http.StatusServiceUnavailable, + http.StatusGatewayTimeout, + http.StatusHTTPVersionNotSupported, + http.StatusVariantAlsoNegotiates, + http.StatusInsufficientStorage, + http.StatusLoopDetected, + http.StatusNotExtended, + http.StatusNetworkAuthenticationRequired, +} From 9dc03d70a0c7522fd97e664a242c0596bfeb8c58 Mon Sep 17 00:00:00 2001 From: kpacha Date: Wed, 3 Oct 2018 16:20:35 +0200 Subject: [PATCH 06/10] test the http executor with the new detailed http status handler --- proxy/http_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/proxy/http_test.go b/proxy/http_test.go index 0d9b851a1..e6566b38f 100644 --- a/proxy/http_test.go +++ b/proxy/http_test.go @@ -192,6 +192,56 @@ func TestNewHTTPProxy_badStatusCode(t *testing.T) { } } +func TestNewHTTPProxy_badStatusCode_detailed(t *testing.T) { + expectedMethod := "GET" + backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "booom", 500) + })) + defer backendServer.Close() + + rpURL, _ := url.Parse(backendServer.URL) + backend := config.Backend{ + Decoder: encoding.JSONDecoder, + ExtraConfig: config.ExtraConfig{ + Namespace: map[string]interface{}{ + "return_error_details": true, + }, + }, + } + request := Request{ + Method: expectedMethod, + Path: "/", + URL: rpURL, + Body: newDummyReadCloser(""), + } + mustEnd := time.After(time.Duration(150) * time.Millisecond) + + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(10)*time.Millisecond) + defer cancel() + response, err := httpProxy(&backend)(ctx, &request) + if err == nil { + t.Errorf("The proxy didn't propagate the backend error: %s", err) + } + errSH, ok := err.(responseError) + if !ok { + t.Errorf("The propagated error does not implement the expected interface: %T", err) + } + if errSH.Error() != "booom\n" { + t.Errorf("unexpected error msg: %s", errSH.Error()) + } + if errSH.StatusCode() != 500 { + t.Errorf("unexpected error code: %d", errSH.StatusCode()) + } + if response == nil { + t.Error("We were expecting a response but we got none") + } + select { + case <-mustEnd: + t.Errorf("Error: expected response") + default: + } +} + func TestNewHTTPProxy_decodingError(t *testing.T) { expectedMethod := "GET" backendServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From 5543db589e7fc1402ffd4e1b44fdce176ed98420 Mon Sep 17 00:00:00 2001 From: kpacha Date: Wed, 3 Oct 2018 16:25:01 +0200 Subject: [PATCH 07/10] StatusMisdirectedRequest status removed from the tests --- proxy/http_status_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/http_status_test.go b/proxy/http_status_test.go index 3da4e1258..4f1295391 100644 --- a/proxy/http_status_test.go +++ b/proxy/http_status_test.go @@ -88,7 +88,7 @@ var statusCodes = []int{ http.StatusRequestedRangeNotSatisfiable, http.StatusExpectationFailed, http.StatusTeapot, - http.StatusMisdirectedRequest, + // http.StatusMisdirectedRequest, http.StatusUnprocessableEntity, http.StatusLocked, http.StatusFailedDependency, From 4785492e4c90a99279998ac8479c7dcba7e48aca Mon Sep 17 00:00:00 2001 From: kpacha Date: Wed, 21 Nov 2018 18:01:38 +0100 Subject: [PATCH 08/10] error response simplified --- proxy/http.go | 3 +- proxy/http_test.go | 2 +- transport/http/client/http_status_test.go | 67 ++++++++++++++++++++++- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/proxy/http.go b/proxy/http.go index 36d4bb676..1cc71b334 100644 --- a/proxy/http.go +++ b/proxy/http.go @@ -2,6 +2,7 @@ package proxy import ( "context" + "fmt" "net/http" "strings" @@ -73,7 +74,7 @@ func NewHTTPProxyDetailed(remote *config.Backend, re client.HTTPRequestExecutor, if t, ok := err.(responseError); ok { return &Response{ Data: map[string]interface{}{ - requestToBakend.URL.String(): map[string]interface{}{"error": t}, + fmt.Sprintf("error_%s", requestToBakend.URL.String()): t, }, Metadata: Metadata{StatusCode: t.StatusCode()}, }, nil diff --git a/proxy/http_test.go b/proxy/http_test.go index b2d294240..74484baf8 100644 --- a/proxy/http_test.go +++ b/proxy/http_test.go @@ -231,7 +231,7 @@ func TestNewHTTPProxy_badStatusCode_detailed(t *testing.T) { t.Errorf("unexpected error code: %d", response.Metadata.StatusCode) } b, _ := json.Marshal(response.Data) - if string(b) != `{"`+rpURL.String()+`":{"error":{"http_status_code":500,"http_body":"booom\n"}}}` { + if string(b) != `{"error_`+rpURL.String()+`":{"http_status_code":500,"http_body":"booom\n"}}` { t.Errorf("unexpected response content: %s", string(b)) } select { diff --git a/transport/http/client/http_status_test.go b/transport/http/client/http_status_test.go index 7d9913f4e..5606a8f80 100644 --- a/transport/http/client/http_status_test.go +++ b/transport/http/client/http_status_test.go @@ -20,6 +20,25 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { } sh := GetHTTPStatusHandler(cfg) + for _, code := range []int{http.StatusOK, http.StatusCreated} { + resp := &http.Response{ + StatusCode: code, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"foo":"bar"}`)), + } + + r, err := sh(context.Background(), resp) + + if r != resp { + t.Errorf("#%d unexpected response: %v", code, r) + return + } + + if err != nil { + t.Errorf("#%d unexpected error: %s", code, err.Error()) + return + } + } + for i, code := range statusCodes { msg := http.StatusText(code) @@ -41,18 +60,62 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { return } - if e.Code != code { + if e.StatusCode() != code { t.Errorf("#%d unexpected status code: %d", i, e.Code) return } - if e.Msg != msg { + if e.Error() != msg { t.Errorf("#%d unexpected message: %s", i, e.Msg) return } } } +func TestDefaultHTTPStatusHandler(t *testing.T) { + sh := GetHTTPStatusHandler(&config.Backend{}) + + for _, code := range []int{http.StatusOK, http.StatusCreated} { + resp := &http.Response{ + StatusCode: code, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"foo":"bar"}`)), + } + + r, err := sh(context.Background(), resp) + + if r != resp { + t.Errorf("#%d unexpected response: %v", code, r) + return + } + + if err != nil { + t.Errorf("#%d unexpected error: %s", code, err.Error()) + return + } + } + + for _, code := range statusCodes { + msg := http.StatusText(code) + + resp := &http.Response{ + StatusCode: code, + Body: ioutil.NopCloser(bytes.NewBufferString(msg)), + } + + r, err := sh(context.Background(), resp) + + if r != nil { + t.Errorf("#%d unexpected response: %v", code, r) + return + } + + if err != ErrInvalidStatusCode { + t.Errorf("#%d unexpected error: %v", code, err) + return + } + } +} + var statusCodes = []int{ http.StatusBadRequest, http.StatusUnauthorized, From 9ed8097673b315fd8fa568a78dca32cab7020444 Mon Sep 17 00:00:00 2001 From: kpacha Date: Wed, 21 Nov 2018 23:59:43 +0100 Subject: [PATCH 09/10] map the error to the name defined at the extra_config --- Makefile | 1 + proxy/http.go | 3 +- proxy/http_test.go | 4 +- test/integration_test.go | 241 ++++++++++++++++++++++ transport/http/client/http_status_test.go | 2 +- transport/http/client/status.go | 12 +- 6 files changed, 256 insertions(+), 7 deletions(-) create mode 100644 test/integration_test.go diff --git a/Makefile b/Makefile index 75dc2242e..c2a38d4ff 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,7 @@ deps: test: go generate ./... go test -cover -race ./... + go test -coverprofile=coverage.out -tags integration ./... benchmark: @echo "Proxy middleware stack" diff --git a/proxy/http.go b/proxy/http.go index 1cc71b334..b2cf21cca 100644 --- a/proxy/http.go +++ b/proxy/http.go @@ -74,7 +74,7 @@ func NewHTTPProxyDetailed(remote *config.Backend, re client.HTTPRequestExecutor, if t, ok := err.(responseError); ok { return &Response{ Data: map[string]interface{}{ - fmt.Sprintf("error_%s", requestToBakend.URL.String()): t, + fmt.Sprintf("error_%s", t.Name()): t, }, Metadata: Metadata{StatusCode: t.StatusCode()}, }, nil @@ -104,5 +104,6 @@ func NewRequestBuilderMiddleware(remote *config.Backend) Middleware { type responseError interface { Error() string + Name() string StatusCode() int } diff --git a/proxy/http_test.go b/proxy/http_test.go index 74484baf8..74cd89daf 100644 --- a/proxy/http_test.go +++ b/proxy/http_test.go @@ -205,7 +205,7 @@ func TestNewHTTPProxy_badStatusCode_detailed(t *testing.T) { Decoder: encoding.JSONDecoder, ExtraConfig: config.ExtraConfig{ client.Namespace: map[string]interface{}{ - "return_error_details": true, + "return_error_details": "some", }, }, } @@ -231,7 +231,7 @@ func TestNewHTTPProxy_badStatusCode_detailed(t *testing.T) { t.Errorf("unexpected error code: %d", response.Metadata.StatusCode) } b, _ := json.Marshal(response.Data) - if string(b) != `{"error_`+rpURL.String()+`":{"http_status_code":500,"http_body":"booom\n"}}` { + if string(b) != `{"error_some":{"http_status_code":500,"http_body":"booom\n"}}` { t.Errorf("unexpected response content: %s", string(b)) } select { diff --git a/test/integration_test.go b/test/integration_test.go new file mode 100644 index 000000000..bf12c8520 --- /dev/null +++ b/test/integration_test.go @@ -0,0 +1,241 @@ +// +build integration !race + +package test + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "io/ioutil" + "math/rand" + "net/http" + "net/http/httptest" + "testing" + "text/template" + "time" + + "github.com/devopsfaith/krakend/config" + "github.com/devopsfaith/krakend/logging" + "github.com/devopsfaith/krakend/proxy" + "github.com/devopsfaith/krakend/router/gin" +) + +func TestKrakenD(t *testing.T) { + cfg, err := setupBackend(t) + if err != nil { + t.Error(err) + return + } + + buf := new(bytes.Buffer) + logger, err := logging.NewLogger("DEBUG", buf, "[KRAKEND]") + if err != nil { + t.Error(err) + return + } + + go func() { + gin.DefaultFactory(proxy.DefaultFactory(logger), logger).New().Run(*cfg) + }() + + for _, tc := range []struct { + name string + url string + method string + headers map[string]string + body string + expBody string + }{ + { + name: "static", + url: "/static", + headers: map[string]string{}, + expBody: `{"bar":"foobar","foo":42}`, + }, + { + name: "param_forwarding", + url: "/param_forwarding/foo", + method: "POST", + headers: map[string]string{ + "Content-Type": "application/json", + "Authorization": "bearer AuthorizationToken", + "X-Y-Z": "x-y-z", + }, + body: `{"foo":"bar"}`, + expBody: `{"path":"/foo"}`, + }, + { + name: "timeout", + url: "/timeout", + headers: map[string]string{}, + expBody: `{"email":"some@email.com","name":"a"}`, + }, + { + name: "partial_with_static", + url: "/partial/static", + headers: map[string]string{}, + expBody: `{"bar":"foobar","email":"some@email.com","foo":42,"name":"a"}`, + }, + { + name: "partial", + url: "/partial", + headers: map[string]string{}, + expBody: `{"email":"some@email.com","name":"a"}`, + }, + { + name: "combination", + url: "/combination", + headers: map[string]string{}, + expBody: `{"name":"a","personal_email":"some@email.com","posts":[{"body":"some content","date":"123456789"},{"body":"some other content","date":"123496789"}]}`, + }, + { + name: "detail_error", + url: "/detail_error", + headers: map[string]string{}, + expBody: `{"email":"some@email.com","error_backend_a":{"http_status_code":429,"http_body":"sad panda\n"},"name":"a"}`, + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + time.Sleep(300 * time.Millisecond) + + if tc.method == "" { + tc.method = "GET" + } + + var body io.Reader + if tc.body != "" { + body = bytes.NewBufferString(tc.body) + } + + r, _ := http.NewRequest(tc.method, fmt.Sprintf("http://localhost:%d%s", cfg.Port, tc.url), body) + for k, v := range tc.headers { + r.Header.Add(k, v) + } + + resp, err := http.DefaultClient.Do(r) + if err != nil { + t.Error(err) + return + } + if resp == nil { + t.Errorf("%s: nil response", resp.Request.URL.Path) + return + } + + if c := resp.Header.Get("Content-Type"); c != "application/json; charset=utf-8" { + t.Errorf("%s: unexpected header content-type: %s", resp.Request.URL.Path, c) + return + } + if resp.StatusCode != http.StatusOK { + t.Errorf("%s: unexpected status code: %d", resp.Request.URL.Path, resp.StatusCode) + return + } + b, _ := ioutil.ReadAll(resp.Body) + resp.Body.Close() + if tc.expBody != string(b) { + t.Errorf("%s: unexpected body: %s", resp.Request.URL.Path, string(b)) + return + } + }) + } + +} + +func setupBackend(t *testing.T) (*config.ServiceConfig, error) { + data := map[string]interface{}{"port": rand.Intn(2000) + 8080} + + // param forwarding validation backend + b1 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if c := r.Header.Get("Content-Type"); c != "application/json" { + t.Errorf("unexpected header content-type: %s", c) + http.Error(rw, "bad content-type", 400) + return + } + if c := r.Header.Get("Authorization"); c != "bearer AuthorizationToken" { + t.Errorf("unexpected header Authorization: %s", c) + http.Error(rw, "bad Authorization", 400) + return + } + if c := r.Header.Get("X-Y-Z"); c != "x-y-z" { + t.Errorf("unexpected header X-Y-Z: %s", c) + http.Error(rw, "bad X-Y-Z", 400) + return + } + body, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Error(err) + return + } + if string(body) != `{"foo":"bar"}` { + t.Errorf("unexpected request body: %s", string(body)) + return + } + rw.Header().Add("Content-Type", "application/json") + json.NewEncoder(rw).Encode(map[string]interface{}{"path": r.URL.Path}) + })) + data["b1"] = b1.URL + + // collection generator backend + b2 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.Header().Add("Content-Type", "application/json") + json.NewEncoder(rw).Encode([]interface{}{ + map[string]interface{}{"body": "some content", "date": "123456789"}, + map[string]interface{}{"body": "some other content", "date": "123496789"}, + }) + })) + data["b2"] = b2.URL + + // regular struct generator backend + b3 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.Header().Add("Content-Type", "application/json") + json.NewEncoder(rw).Encode(map[string]interface{}{"email": "some@email.com", "name": "a"}) + })) + data["b3"] = b3.URL + + // crasher backend + b4 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + http.Error(rw, "sad panda", 429) + })) + data["b4"] = b4.URL + + // slow backend + b5 := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + <-time.After(time.Second) + rw.Header().Add("Content-Type", "application/json") + json.NewEncoder(rw).Encode(map[string]interface{}{"email": "some@email.com", "name": "a"}) + })) + data["b5"] = b5.URL + + c, err := loadConfig(data) + if err != nil { + return nil, err + } + + return c, nil +} + +func loadConfig(data map[string]interface{}) (*config.ServiceConfig, error) { + content, _ := ioutil.ReadFile("krakend.json") + tmpl, err := template.New("test").Parse(string(content)) + if err != nil { + return nil, err + } + + buf := new(bytes.Buffer) + if err = tmpl.Execute(buf, data); err != nil { + return nil, err + } + + c, err := config.NewParserWithFileReader(func(s string) ([]byte, error) { + return []byte(s), nil + }).Parse(buf.String()) + + if err != nil { + return nil, err + } + + return &c, nil +} diff --git a/transport/http/client/http_status_test.go b/transport/http/client/http_status_test.go index 5606a8f80..ea2e3b9b4 100644 --- a/transport/http/client/http_status_test.go +++ b/transport/http/client/http_status_test.go @@ -14,7 +14,7 @@ func TestDetailedHTTPStatusHandler(t *testing.T) { cfg := &config.Backend{ ExtraConfig: config.ExtraConfig{ Namespace: map[string]interface{}{ - "return_error_details": true, + "return_error_details": "some", }, }, } diff --git a/transport/http/client/status.go b/transport/http/client/status.go index 6d06596a7..22364fd74 100644 --- a/transport/http/client/status.go +++ b/transport/http/client/status.go @@ -24,8 +24,8 @@ func GetHTTPStatusHandler(remote *config.Backend) HTTPStatusHandler { if e, ok := remote.ExtraConfig[Namespace]; ok { if m, ok := e.(map[string]interface{}); ok { if v, ok := m["return_error_details"]; ok { - if b, ok := v.(bool); ok && b { - return DetailedHTTPStatusHandler(DefaultHTTPStatusHandler) + if b, ok := v.(string); ok && b != "" { + return DetailedHTTPStatusHandler(DefaultHTTPStatusHandler, b) } } } @@ -48,7 +48,7 @@ func NoOpHTTPStatusHandler(_ context.Context, resp *http.Response) (*http.Respon } // DetailedHTTPStatusHandler is a HTTPStatusHandler implementation -func DetailedHTTPStatusHandler(next HTTPStatusHandler) HTTPStatusHandler { +func DetailedHTTPStatusHandler(next HTTPStatusHandler, name string) HTTPStatusHandler { return func(ctx context.Context, resp *http.Response) (*http.Response, error) { if r, err := next(ctx, resp); err == nil { return r, nil @@ -64,6 +64,7 @@ func DetailedHTTPStatusHandler(next HTTPStatusHandler) HTTPStatusHandler { return resp, HTTPResponseError{ Code: resp.StatusCode, Msg: string(body), + name: name, } } } @@ -71,12 +72,17 @@ func DetailedHTTPStatusHandler(next HTTPStatusHandler) HTTPStatusHandler { type HTTPResponseError struct { Code int `json:"http_status_code"` Msg string `json:"http_body,omitempty"` + name string `json:"-"` } func (r HTTPResponseError) Error() string { return r.Msg } +func (r HTTPResponseError) Name() string { + return r.name +} + func (r HTTPResponseError) StatusCode() int { return r.Code } From 1ee8283533f75a411046ef50cac296cd64a063eb Mon Sep 17 00:00:00 2001 From: kpacha Date: Thu, 22 Nov 2018 00:00:39 +0100 Subject: [PATCH 10/10] config template for the integration test added --- test/krakend.json | 162 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 test/krakend.json diff --git a/test/krakend.json b/test/krakend.json new file mode 100644 index 000000000..e0732dbe8 --- /dev/null +++ b/test/krakend.json @@ -0,0 +1,162 @@ +{ + "version": 2, + "name": "My lovely gateway", + "port": {{.port}}, + "cache_ttl": "3600s", + "timeout": "3s", + "endpoints": [ + { + "endpoint": "/static", + "backend": [ + { + "host": [ + "{{.b4}}" + ], + "url_pattern": "/" + } + ], + "extra_config": { + "github.com/devopsfaith/krakend/proxy": { + "static": { + "strategy": "errored", + "data": { + "foo": 42, + "bar": "foobar" + } + } + } + } + }, + { + "endpoint": "/param_forwarding/{name}", + "method": "POST", + "headers_to_pass": [ + "Content-Type", + "Authorization", + "X-Y-Z" + ], + "backend": [ + { + "host": [ + "{{.b1}}" + ], + "url_pattern": "/{name}" + } + ] + }, + { + "endpoint": "/combination", + "backend": [ + { + "host": [ + "{{.b2}}" + ], + "url_pattern": "/", + "is_collection": true, + "mapping": { + "collection": "posts" + } + }, + { + "host": [ + "{{.b3}}" + ], + "url_pattern": "/", + "mapping": { + "email": "personal_email" + } + } + ] + }, + { + "endpoint": "/timeout", + "timeout": "100ms", + "backend": [ + { + "host": [ + "{{.b5}}" + ], + "url_pattern": "/" + }, + { + "host": [ + "{{.b3}}" + ], + "url_pattern": "/" + } + ] + }, + { + "endpoint": "/partial/static", + "backend": [ + { + "host": [ + "{{.b4}}" + ], + "url_pattern": "/" + }, + { + "host": [ + "{{.b3}}" + ], + "url_pattern": "/" + } + ], + "extra_config": { + "github.com/devopsfaith/krakend/proxy": { + "static": { + "strategy": "incomplete", + "data": { + "foo": 42, + "bar": "foobar" + } + } + } + } + }, + { + "endpoint": "/detail_error", + "backend": [ + { + "host": [ + "{{.b4}}" + ], + "url_pattern": "/", + "extra_config": { + "github.com/devopsfaith/krakend/http": { + "return_error_details": "backend_a" + } + } + }, + { + "host": [ + "{{.b3}}" + ], + "url_pattern": "/", + "extra_config": { + "github.com/devopsfaith/krakend/http": { + "return_error_details": "backend_b" + } + } + } + ] + }, + { + "endpoint": "/partial", + "backend": [ + { + "host": [ + "{{.b4}}" + ], + "url_pattern": "/" + }, + { + "host": [ + "{{.b3}}" + ], + "url_pattern": "/" + } + ] + } + ] +} \ No newline at end of file