Skip to content

Commit

Permalink
fix global cache returning duplicate responses (TykTechnologies#2871)
Browse files Browse the repository at this point in the history
  • Loading branch information
gernest authored Feb 19, 2020
1 parent eb495bf commit f308337
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 9 deletions.
26 changes: 26 additions & 0 deletions gateway/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2064,3 +2064,29 @@ func TestStripRegex(t *testing.T) {
}
}
}

func TestCache_singleErrorResponse(t *testing.T) {
ts := StartTest()
defer ts.Close()
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
defer srv.Close()
BuildAndLoadAPI(func(spec *APISpec) {
spec.UseKeylessAccess = true
spec.Proxy.ListenPath = "/"
spec.Proxy.TargetURL = srv.URL
spec.CacheOptions.CacheTimeout = 1
spec.CacheOptions.EnableCache = true
spec.CacheOptions.CacheAllSafeRequests = true
})
ts.Run(t,
test.TestCase{Method: http.MethodGet, Path: "/", Code: http.StatusOK},
)
time.Sleep(time.Second)
srv.Close()
wantBody := `{
"error": "There was a problem proxying the request"
}`
ts.Run(t,
test.TestCase{Method: http.MethodGet, Path: "/", Code: http.StatusInternalServerError, BodyMatch: wantBody},
)
}
4 changes: 2 additions & 2 deletions gateway/handler_success.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (s *SuccessHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) *http
// ServeHTTPWithCache will store the request details in the analytics store if necessary and proxy the request to it's
// final destination, this is invoked by the ProxyHandler or right at the start of a request chain if the URL
// Spec states the path is Ignored Itwill also return a response object for the cache
func (s *SuccessHandler) ServeHTTPWithCache(w http.ResponseWriter, r *http.Request) *http.Response {
func (s *SuccessHandler) ServeHTTPWithCache(w http.ResponseWriter, r *http.Request) ProxyResponse {

versionDef := s.Spec.VersionDefinition
if !s.Spec.VersionData.NotVersioned && versionDef.Location == "url" && versionDef.StripPath {
Expand Down Expand Up @@ -377,5 +377,5 @@ func (s *SuccessHandler) ServeHTTPWithCache(w http.ResponseWriter, r *http.Reque
s.RecordHit(r, latency, inRes.Response.StatusCode, inRes.Response)
}

return inRes.Response
return inRes
}
5 changes: 3 additions & 2 deletions gateway/mw_redis_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,15 +239,16 @@ func (m *RedisCacheMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Req
r.Method = newMethod
ctxSetTransformRequestMethod(r, "")
}
resVal = m.sh.ServeHTTPWithCache(w, r)
sr := m.sh.ServeHTTPWithCache(w, r)
resVal = sr.Response
}

cacheThisRequest := true
cacheTTL := m.Spec.CacheOptions.CacheTimeout

if resVal == nil {
log.Warning("Upstream request must have failed, response is empty")
return nil, http.StatusOK
return nil, mwStatusRespond
}

cacheOnlyResponseCodes := m.Spec.CacheOptions.CacheOnlyResponseCodes
Expand Down
5 changes: 0 additions & 5 deletions gateway/reverse_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,6 @@ var hopHeaders = []string{
func (p *ReverseProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) ProxyResponse {
startTime := time.Now()
p.logger.WithField("ts", startTime.UnixNano()).Debug("Started")

resp := p.WrappedServeHTTP(rw, req, recordDetail(req, p.TykAPISpec))

finishTime := time.Since(startTime)
Expand All @@ -461,7 +460,6 @@ func (p *ReverseProxy) ServeHTTPForCache(rw http.ResponseWriter, req *http.Reque

resp := p.WrappedServeHTTP(rw, req, true)
nopCloseResponseBody(resp.Response)

finishTime := time.Since(startTime)
p.logger.WithField("ns", finishTime.Nanoseconds()).Debug("Finished")

Expand Down Expand Up @@ -675,7 +673,6 @@ func (p *ReverseProxy) WrappedServeHTTP(rw http.ResponseWriter, req *http.Reques
ext.SpanKindRPCClient.Set(span)
req = req.WithContext(ctx)
}

var roundTripper http.RoundTripper

p.TykAPISpec.Lock()
Expand Down Expand Up @@ -860,7 +857,6 @@ func (p *ReverseProxy) WrappedServeHTTP(rw http.ResponseWriter, req *http.Reques
"org_id": p.TykAPISpec.OrgID,
"api_id": p.TykAPISpec.APIID,
}).Error("http: proxy error: ", err)

if strings.Contains(err.Error(), "timeout awaiting response headers") {
p.ErrorHandler.HandleError(rw, logreq, "Upstream service reached hard timeout.", http.StatusGatewayTimeout, true)

Expand All @@ -882,7 +878,6 @@ func (p *ReverseProxy) WrappedServeHTTP(rw http.ResponseWriter, req *http.Reques
p.ErrorHandler.HandleError(rw, logreq, "Upstream host lookup failed", http.StatusInternalServerError, true)
return ProxyResponse{UpstreamLatency: upstreamLatency}
}

p.ErrorHandler.HandleError(rw, logreq, "There was a problem proxying the request", http.StatusInternalServerError, true)
return ProxyResponse{UpstreamLatency: upstreamLatency}

Expand Down

0 comments on commit f308337

Please sign in to comment.