Merge pull request #92217 from p0lyn0mial/proxy-transport-pass-err

Transport.RoundTrip should return a non-nil for failure to obtain a response.
This commit is contained in:
Kubernetes Prow Robot 2020-07-01 07:11:19 -07:00 committed by GitHub
commit c53ce48d48
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 58 additions and 12 deletions

View File

@ -101,15 +101,7 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
resp, err := rt.RoundTrip(req)
if err != nil {
message := fmt.Sprintf("Error trying to reach service: '%v'", err.Error())
resp = &http.Response{
Header: http.Header{},
StatusCode: http.StatusServiceUnavailable,
Body: ioutil.NopCloser(strings.NewReader(message)),
}
resp.Header.Set("Content-Type", "text/plain; charset=utf-8")
resp.Header.Set("X-Content-Type-Options", "nosniff")
return resp, nil
return nil, fmt.Errorf("error trying to reach service: %w", err)
}
if redirect := resp.Header.Get("Location"); redirect != "" {

View File

@ -232,6 +232,12 @@ func (h *UpgradeAwareHandler) ServeHTTP(w http.ResponseWriter, req *http.Request
proxy.Transport = h.Transport
proxy.FlushInterval = h.FlushInterval
proxy.ErrorLog = log.New(noSuppressPanicError{}, "", log.LstdFlags)
if h.Responder != nil {
// if an optional error interceptor/responder was provided wire it
// the custom responder might be used for providing a unified error reporting
// or supporting retry mechanisms by not sending non-fatal errors to the clients
proxy.ErrorHandler = h.Responder.Error
}
proxy.ServeHTTP(w, newReq)
}

View File

@ -901,7 +901,8 @@ func TestFlushIntervalHeaders(t *testing.T) {
t.Fatal(err)
}
proxyHandler := NewUpgradeAwareHandler(backendURL, nil, false, false, nil)
responder := &fakeResponder{t: t}
proxyHandler := NewUpgradeAwareHandler(backendURL, nil, false, false, responder)
frontend := httptest.NewServer(proxyHandler)
defer frontend.Close()
@ -924,6 +925,53 @@ func TestFlushIntervalHeaders(t *testing.T) {
}
}
type fakeRT struct {
err error
}
func (frt *fakeRT) RoundTrip(*http.Request) (*http.Response, error) {
return nil, frt.err
}
// TestErrorPropagation checks if the default transport doesn't swallow the errors by providing a fakeResponder that intercepts and stores the error.
func TestErrorPropagation(t *testing.T) {
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
panic("unreachable")
}))
defer backend.Close()
backendURL, err := url.Parse(backend.URL)
if err != nil {
t.Fatal(err)
}
responder := &fakeResponder{t: t}
expectedErr := errors.New("nasty error")
proxyHandler := NewUpgradeAwareHandler(backendURL, &fakeRT{err: expectedErr}, true, false, responder)
frontend := httptest.NewServer(proxyHandler)
defer frontend.Close()
req, _ := http.NewRequest("GET", frontend.URL, nil)
req.Close = true
ctx, cancel := context.WithTimeout(req.Context(), 10*time.Second)
defer cancel()
req = req.WithContext(ctx)
res, err := frontend.Client().Do(req)
if err != nil {
t.Fatalf("Get: %v", err)
}
defer res.Body.Close()
if res.StatusCode != fakeStatusCode {
t.Fatalf("unexpected HTTP status code returned: %v, expected: %v", res.StatusCode, fakeStatusCode)
}
if !strings.Contains(responder.err.Error(), expectedErr.Error()) {
t.Fatalf("responder got unexpected error: %v, expected the error to contain %q", responder.err.Error(), expectedErr.Error())
}
}
// exampleCert was generated from crypto/tls/generate_cert.go with the following command:
// go run generate_cert.go --rsa-bits 1024 --host example.com --ca --start-date "Jan 1 00:00:00 1970" --duration=1000000h
var exampleCert = []byte(`-----BEGIN CERTIFICATE-----

View File

@ -237,7 +237,7 @@ func (r *responder) Object(statusCode int, obj runtime.Object) {
}
func (r *responder) Error(_ http.ResponseWriter, _ *http.Request, err error) {
http.Error(r.w, err.Error(), http.StatusInternalServerError)
http.Error(r.w, err.Error(), http.StatusServiceUnavailable)
}
// these methods provide locked access to fields

View File

@ -561,7 +561,7 @@ func TestGetContextForNewRequest(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if !strings.Contains(string(body), "Error trying to reach service: 'context deadline exceeded'") {
if !strings.Contains(string(body), "context deadline exceeded") {
t.Error(string(body))
}