fix: do not retry if context has been cancelled

The additional retry logic introduced by #337 caused any error to become
retry-able if the request did not make it to the back end. This included
errors that came from context cancellation. This commit makes requests
non-retry-able as soon as the context has been cancelled.

Co-Authored-By: Dominik Froehlich <[email protected]>

Author: maxmoehl

integration/retry_test.go

@@ -3,6 +3,7 @@ package integration
 import (
 	"bytes"
 	"fmt"
+	"net"
 	"net/http"
 
 	"code.cloudfoundry.org/gorouter/route"
@@ -29,6 +30,44 @@ var _ = Describe("Retries", func() {
 		}
 	})
 
+	Context("when gorouter is called by a bad client", func() {
+		var appURL string
+		var app *common.TestApp
+
+		BeforeEach(func() {
+			appURL = "bad-app." + test_util.LocalhostDNS
+
+			app = common.NewTestApp([]route.Uri{route.Uri(appURL)}, testState.cfg.Port, testState.mbusClient, nil, "")
+			app.TlsRegister(testState.trustedBackendServerCertSAN)
+
+			app.AddHandler("/", func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(http.StatusTeapot)
+			})
+
+			err := app.TlsListen(testState.trustedBackendTLSConfig)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			app.Stop()
+		})
+
+		It("does not prune the endpoint on context cancelled", func() {
+			conn, err := net.Dial("tcp", fmt.Sprintf("%s:%d", appURL, testState.cfg.Port))
+			Expect(err).ToNot(HaveOccurred())
+
+			_, err = conn.Write([]byte(fmt.Sprintf("GET / HTTP/1.1\r\nHost: %s\r\n\r\n", appURL)))
+			Expect(err).ToNot(HaveOccurred())
+
+			_ = conn.Close()
+
+			Consistently(func() bool {
+				res, err := testState.client.Do(testState.newRequest("http://" + appURL))
+				return err == nil && res.StatusCode == http.StatusTeapot
+			}).Should(Equal(true))
+		})
+	})
+
 	Context("when gorouter talks to a broken app behind envoy", func() {
 		var appURL string
 		var badApp *common.TcpApp

proxy/round_tripper/proxy_round_tripper.go

@@ -441,6 +441,11 @@ func isIdempotent(request *http.Request) bool {
 }
 
 func (rt *roundTripper) isRetriable(request *http.Request, err error, trace *requestTracer) (bool, error) {
+	// if the context has been cancelled we do not perform further retries
+	if request.Context().Err() != nil {
+		return false, request.Context().Err()
+	}
+
 	// io.EOF errors are considered safe to retry for certain requests
 	// Replace the error here to track this state when classifying later.
 	if err == io.EOF && isIdempotent(request) {