From 45557c5c7bf10a45dd45a9f2683465d017304488 Mon Sep 17 00:00:00 2001 From: kpacha Date: Fri, 31 Aug 2018 14:39:01 +0200 Subject: [PATCH] check the proxy response before returning an error --- router/gin/endpoint.go | 19 +++++++++---------- router/gin/endpoint_test.go | 29 ++++++++++++++++++++++++++--- router/mux/endpoint.go | 21 ++++++++++----------- router/mux/endpoint_test.go | 25 +++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 24 deletions(-) diff --git a/router/gin/endpoint.go b/router/gin/endpoint.go index 1a7afcf92..62f8e3601 100644 --- a/router/gin/endpoint.go +++ b/router/gin/endpoint.go @@ -3,7 +3,6 @@ package gin import ( "context" "fmt" - "net/http" "strings" "time" @@ -37,21 +36,16 @@ func CustomErrorEndpointHandler(configuration *config.EndpointConfig, prxy proxy c.Header(core.KrakendHeaderName, core.KrakendHeaderValue) response, err := prxy(requestCtx, requestGenerator(c, configuration.QueryString)) - if err != nil { - abort(c, errF(err)) - cancel() - return - } select { case <-requestCtx.Done(): - abort(c, http.StatusInternalServerError) - cancel() - return + if err == nil { + err = router.ErrInternalError + } default: } - if response != nil { + if response != nil && len(response.Data) > 0 { if response.IsComplete { c.Header(router.CompleteResponseHeaderName, router.HeaderCompleteResponseValue) if isCacheEnabled { @@ -65,6 +59,11 @@ func CustomErrorEndpointHandler(configuration *config.EndpointConfig, prxy proxy c.Header(k, v[0]) } } else { + if err != nil { + abort(c, errF(err)) + cancel() + return + } c.Header(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) } diff --git a/router/gin/endpoint_test.go b/router/gin/endpoint_test.go index 69e9b507e..44d4be29b 100644 --- a/router/gin/endpoint_test.go +++ b/router/gin/endpoint_test.go @@ -3,7 +3,7 @@ package gin import ( "bytes" "context" - "fmt" + "errors" "io/ioutil" "net/http" "net/http/httptest" @@ -59,12 +59,23 @@ func TestEndpointHandler_incomplete(t *testing.T) { func TestEndpointHandler_ko(t *testing.T) { p := func(_ context.Context, _ *proxy.Request) (*proxy.Response, error) { - return nil, fmt.Errorf("This is %s", "a dummy error") + return nil, errors.New("This is a dummy error") } testEndpointHandler(t, 10, p, "", "", "", http.StatusInternalServerError, false) } -func TestEndpointHandler_cancel(t *testing.T) { +func TestEndpointHandler_incompleteAndErrored(t *testing.T) { + p := func(_ context.Context, _ *proxy.Request) (*proxy.Response, error) { + return &proxy.Response{ + IsComplete: false, + Data: map[string]interface{}{"foo": "bar"}, + }, errors.New("This is a dummy error") + } + expectedBody := "{\"foo\":\"bar\"}" + testEndpointHandler(t, 10, p, expectedBody, "", "application/json; charset=utf-8", http.StatusOK, false) +} + +func TestEndpointHandler_cancelEmpty(t *testing.T) { p := func(_ context.Context, _ *proxy.Request) (*proxy.Response, error) { time.Sleep(100 * time.Millisecond) return nil, nil @@ -72,6 +83,18 @@ func TestEndpointHandler_cancel(t *testing.T) { testEndpointHandler(t, 0, p, "", "", "", http.StatusInternalServerError, false) } +func TestEndpointHandler_cancel(t *testing.T) { + p := func(_ context.Context, _ *proxy.Request) (*proxy.Response, error) { + time.Sleep(100 * time.Millisecond) + return &proxy.Response{ + IsComplete: false, + Data: map[string]interface{}{"foo": "bar"}, + }, nil + } + expectedBody := "{\"foo\":\"bar\"}" + testEndpointHandler(t, 0, p, expectedBody, "", "application/json; charset=utf-8", http.StatusOK, false) +} + func TestEndpointHandler_noop(t *testing.T) { testEndpointHandler(t, 10, proxy.NoopProxy, "{}", "", "application/json; charset=utf-8", http.StatusOK, false) } diff --git a/router/mux/endpoint.go b/router/mux/endpoint.go index a6ac20656..eeda9675d 100644 --- a/router/mux/endpoint.go +++ b/router/mux/endpoint.go @@ -48,23 +48,16 @@ func CustomEndpointHandlerWithHTTPError(rb RequestBuilder, errF router.ToHTTPErr requestCtx, cancel := context.WithTimeout(context.Background(), endpointTimeout) response, err := prxy(requestCtx, rb(r, configuration.QueryString, headersToSend)) - if err != nil { - w.Header().Set(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) - http.Error(w, err.Error(), errF(err)) - cancel() - return - } select { case <-requestCtx.Done(): - w.Header().Set(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) - http.Error(w, router.ErrInternalError.Error(), http.StatusInternalServerError) - cancel() - return + if err == nil { + err = router.ErrInternalError + } default: } - if response != nil { + if response != nil && len(response.Data) > 0 { if response.IsComplete { w.Header().Set(router.CompleteResponseHeaderName, router.HeaderCompleteResponseValue) if isCacheEnabled { @@ -78,6 +71,12 @@ func CustomEndpointHandlerWithHTTPError(rb RequestBuilder, errF router.ToHTTPErr w.Header().Set(k, v[0]) } } else { + if err != nil { + w.Header().Set(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) + http.Error(w, err.Error(), errF(err)) + cancel() + return + } w.Header().Set(router.CompleteResponseHeaderName, router.HeaderIncompleteResponseValue) } diff --git a/router/mux/endpoint_test.go b/router/mux/endpoint_test.go index 57685c5a4..c734262b7 100644 --- a/router/mux/endpoint_test.go +++ b/router/mux/endpoint_test.go @@ -3,6 +3,7 @@ package mux import ( "bytes" "context" + "errors" "fmt" "io/ioutil" "net/http" @@ -47,7 +48,31 @@ func TestEndpointHandler_ko(t *testing.T) { time.Sleep(5 * time.Millisecond) } +func TestEndpointHandler_incompleteAndErrored(t *testing.T) { + p := func(_ context.Context, _ *proxy.Request) (*proxy.Response, error) { + return &proxy.Response{ + IsComplete: false, + Data: map[string]interface{}{"foo": "bar"}, + }, errors.New("This is a dummy error") + } + expectedBody := "{\"foo\":\"bar\"}" + testEndpointHandler(t, 10, p, "GET", expectedBody, "", "application/json", http.StatusOK, false) + time.Sleep(5 * time.Millisecond) +} + func TestEndpointHandler_cancel(t *testing.T) { + p := func(_ context.Context, _ *proxy.Request) (*proxy.Response, error) { + time.Sleep(100 * time.Millisecond) + return &proxy.Response{ + IsComplete: false, + Data: map[string]interface{}{"foo": "bar"}, + }, nil + } + testEndpointHandler(t, 0, p, "GET", "{\"foo\":\"bar\"}", "", "application/json", http.StatusOK, false) + time.Sleep(5 * time.Millisecond) +} + +func TestEndpointHandler_cancelEmpty(t *testing.T) { p := func(_ context.Context, _ *proxy.Request) (*proxy.Response, error) { time.Sleep(100 * time.Millisecond) return nil, nil