diff --git a/pkg/client/request.go b/pkg/client/request.go index 715b7f7d0de..d0016aedaf2 100644 --- a/pkg/client/request.go +++ b/pkg/client/request.go @@ -105,6 +105,10 @@ type Request struct { // output err error body io.Reader + + // The constructed request and the response + req *http.Request + resp *http.Response } // NewRequest creates a new request helper object for accessing runtime.Objects on a server. @@ -482,16 +486,8 @@ func (r *Request) Upgrade(config *Config, newRoundTripperFunc func(*tls.Config) return upgradeRoundTripper.NewConnection(resp) } -// Do formats and executes the request. Returns a Result object for easy response -// processing. -// -// Error type: -// * If the request can't be constructed, or an error happened earlier while building its -// arguments: *RequestConstructionError -// * If the server responds with a status: *errors.StatusError or *errors.UnexpectedObjectError -// * If the status code and body don't make sense together: *UnexpectedStatusError -// * http.Client.Do errors are returned directly. -func (r *Request) Do() Result { +// DoRaw executes a raw request which is not subject to interpretation as an API response. +func (r *Request) DoRaw() ([]byte, error) { client := r.client if client == nil { client = http.DefaultClient @@ -503,34 +499,34 @@ func (r *Request) Do() Result { for { if r.err != nil { - return Result{err: &RequestConstructionError{r.err}} + return nil, r.err } // TODO: added to catch programmer errors (invoking operations with an object with an empty namespace) if (r.verb == "GET" || r.verb == "PUT" || r.verb == "DELETE") && r.namespaceSet && len(r.resourceName) > 0 && len(r.namespace) == 0 { - return Result{err: &RequestConstructionError{fmt.Errorf("an empty namespace may not be set when a resource name is provided")}} + return nil, fmt.Errorf("an empty namespace may not be set when a resource name is provided") } if (r.verb == "POST") && r.namespaceSet && len(r.namespace) == 0 { - return Result{err: &RequestConstructionError{fmt.Errorf("an empty namespace may not be set during creation")}} + return nil, fmt.Errorf("an empty namespace may not be set during creation") } - req, err := http.NewRequest(r.verb, r.finalURL(), r.body) + var err error + r.req, err = http.NewRequest(r.verb, r.finalURL(), r.body) if err != nil { - return Result{err: &RequestConstructionError{err}} + return nil, err } - resp, err := client.Do(req) + r.resp, err = client.Do(r.req) if err != nil { - return Result{err: err} + return nil, err } - - respBody, created, err := r.transformResponse(resp, req) + defer r.resp.Body.Close() // Check to see if we got a 429 Too Many Requests response code. - if resp.StatusCode == errors.StatusTooManyRequests { + if r.resp.StatusCode == errors.StatusTooManyRequests { if retries < 10 { retries++ - if waitFor := resp.Header.Get("Retry-After"); waitFor != "" { + if waitFor := r.resp.Header.Get("Retry-After"); waitFor != "" { delay, err := strconv.Atoi(waitFor) if err == nil { glog.V(4).Infof("Got a Retry-After %s response for attempt %d to %v", waitFor, retries, r.finalURL()) @@ -540,19 +536,34 @@ func (r *Request) Do() Result { } } } - return Result{respBody, created, err, r.codec} + body, err := ioutil.ReadAll(r.resp.Body) + if err != nil { + return nil, err + } + return body, err } } -// transformResponse converts an API response into a structured API object. -func (r *Request) transformResponse(resp *http.Response, req *http.Request) ([]byte, bool, error) { - defer resp.Body.Close() - - body, err := ioutil.ReadAll(resp.Body) +// Do formats and executes the request. Returns a Result object for easy response +// processing. +// +// Error type: +// * If the request can't be constructed, or an error happened earlier while building its +// arguments: *RequestConstructionError +// * If the server responds with a status: *errors.StatusError or *errors.UnexpectedObjectError +// * If the status code and body don't make sense together: *UnexpectedStatusError +// * http.Client.Do errors are returned directly. +func (r *Request) Do() Result { + body, err := r.DoRaw() if err != nil { - return nil, false, err + return Result{err: err} } + respBody, created, err := r.transformResponse(body, r.resp, r.req) + return Result{respBody, created, err, r.codec} +} +// transformResponse converts an API response into a structured API object. +func (r *Request) transformResponse(body []byte, resp *http.Response, req *http.Request) ([]byte, bool, error) { // Did the server give us a status response? isStatusResponse := false var status api.Status @@ -589,14 +600,15 @@ func (r *Request) transformResponse(resp *http.Response, req *http.Request) ([]b } // If the server gave us a status back, look at what it was. - if isStatusResponse && status.Status != api.StatusSuccess { + success := resp.StatusCode >= http.StatusOK && resp.StatusCode <= http.StatusPartialContent + if isStatusResponse && (status.Status != api.StatusSuccess && !success) { // "Working" requests need to be handled specially. // "Failed" requests are clearly just an error and it makes sense to return them as such. return nil, false, errors.FromObject(&status) } created := resp.StatusCode == http.StatusCreated - return body, created, err + return body, created, nil } // Result contains the result of calling Request.Do(). diff --git a/pkg/client/request_test.go b/pkg/client/request_test.go index c24158f0bee..4c1bfe642a8 100644 --- a/pkg/client/request_test.go +++ b/pkg/client/request_test.go @@ -233,7 +233,11 @@ func TestTransformResponse(t *testing.T) { if test.Response.Body == nil { test.Response.Body = ioutil.NopCloser(bytes.NewReader([]byte{})) } - response, created, err := r.transformResponse(test.Response, &http.Request{}) + body, err := ioutil.ReadAll(test.Response.Body) + if err != nil { + t.Errorf("failed to read body of response: %v", err) + } + response, created, err := r.transformResponse(body, test.Response, &http.Request{}) hasErr := err != nil if hasErr != test.Error { t.Errorf("%d: unexpected error: %t %v", i, test.Error, err) @@ -307,7 +311,11 @@ func TestTransformUnstructuredError(t *testing.T) { resourceName: testCase.Name, resource: testCase.Resource, } - _, _, err := r.transformResponse(testCase.Res, testCase.Req) + body, err := ioutil.ReadAll(testCase.Res.Body) + if err != nil { + t.Errorf("failed to read body: %v", err) + } + _, _, err = r.transformResponse(body, testCase.Res, testCase.Req) if !testCase.ErrFn(err) { t.Errorf("unexpected error: %v", err) continue