Permalink
Browse files

Don't read entire HTTP response body until necessary.

Get rid of ioutil.ReadAll(resp.Body) in client. We pass streams around
and read them only when necessary so that we don't allocate memory
unnecessarily.
  • Loading branch information...
chuyeow committed Sep 9, 2015
1 parent 9ff3a0b commit edc2bd38d8fa79581371ea78b3c09935877a1ade
Showing with 249 additions and 209 deletions.
  1. +2 −4 auth.go
  2. +27 −30 client.go
  3. +7 −1 client_test.go
  4. +103 −85 container.go
  5. +23 −18 exec.go
  6. +38 −30 image.go
  7. +7 −9 misc.go
  8. +17 −16 network.go
  9. +25 −16 volume.go
View
@@ -127,12 +127,10 @@ func (c *Client) AuthCheck(conf *AuthConfiguration) error {
if conf == nil {
return fmt.Errorf("conf is nil")
}
- body, statusCode, err := c.do("POST", "/auth", doOptions{data: conf})
+ resp, err := c.do("POST", "/auth", doOptions{data: conf})
if err != nil {
return err
}
- if statusCode > 400 {
- return fmt.Errorf("auth error (%d): %s", statusCode, body)
- }
+ resp.Body.Close()
return nil
}
View
@@ -334,27 +334,27 @@ func (c *Client) checkAPIVersion() error {
// See https://goo.gl/kQCfJj for more details.
func (c *Client) Ping() error {
path := "/_ping"
- body, status, err := c.do("GET", path, doOptions{})
+ resp, err := c.do("GET", path, doOptions{})
if err != nil {
return err
}
- if status != http.StatusOK {
- return newError(status, body)
+ if resp.StatusCode != http.StatusOK {
+ return newError(resp)
}
return nil
}
func (c *Client) getServerAPIVersionString() (version string, err error) {
- body, status, err := c.do("GET", "/version", doOptions{})
+ resp, err := c.do("GET", "/version", doOptions{})
if err != nil {
return "", err
}
- if status != http.StatusOK {
- return "", fmt.Errorf("Received unexpected status %d while trying to retrieve the server version", status)
+ defer resp.Body.Close()
+ if resp.StatusCode != http.StatusOK {
+ return "", fmt.Errorf("Received unexpected status %d while trying to retrieve the server version", resp.StatusCode)
}
var versionResponse map[string]interface{}
- err = json.Unmarshal(body, &versionResponse)
- if err != nil {
+ if err := json.NewDecoder(resp.Body).Decode(&versionResponse); err != nil {
return "", err
}
if version, ok := (versionResponse["ApiVersion"]).(string); ok {
@@ -368,24 +368,24 @@ type doOptions struct {
forceJSON bool
}
-func (c *Client) do(method, path string, doOptions doOptions) ([]byte, int, error) {
+func (c *Client) do(method, path string, doOptions doOptions) (*http.Response, error) {
var params io.Reader
if doOptions.data != nil || doOptions.forceJSON {
buf, err := json.Marshal(doOptions.data)
if err != nil {
- return nil, -1, err
+ return nil, err
}
params = bytes.NewBuffer(buf)
}
if path != "/version" && !c.SkipServerVersionCheck && c.expectedAPIVersion == nil {
err := c.checkAPIVersion()
if err != nil {
- return nil, -1, err
+ return nil, err
}
}
req, err := http.NewRequest(method, c.getURL(path), params)
if err != nil {
- return nil, -1, err
+ return nil, err
}
req.Header.Set("User-Agent", userAgent)
if doOptions.data != nil {
@@ -400,33 +400,29 @@ func (c *Client) do(method, path string, doOptions doOptions) ([]byte, int, erro
var dial net.Conn
dial, err = c.Dialer.Dial(protocol, address)
if err != nil {
- return nil, -1, err
+ return nil, err
}
defer dial.Close()
breader := bufio.NewReader(dial)
err = req.Write(dial)
if err != nil {
- return nil, -1, err
+ return nil, err
}
resp, err = http.ReadResponse(breader, req)
} else {
resp, err = c.HTTPClient.Do(req)
}
if err != nil {
if strings.Contains(err.Error(), "connection refused") {
- return nil, -1, ErrConnectionRefused
+ return nil, ErrConnectionRefused
}
- return nil, -1, err
- }
- defer resp.Body.Close()
- body, err := ioutil.ReadAll(resp.Body)
- if err != nil {
- return nil, -1, err
+ return nil, err
}
+
if resp.StatusCode < 200 || resp.StatusCode >= 400 {
- return nil, resp.StatusCode, newError(resp.StatusCode, body)
+ return nil, newError(resp)
}
- return body, resp.StatusCode, nil
+ return resp, nil
}
type streamOptions struct {
@@ -512,11 +508,7 @@ func (c *Client) stream(method, path string, streamOptions streamOptions) error
}
defer resp.Body.Close()
if resp.StatusCode < 200 || resp.StatusCode >= 400 {
- body, err := ioutil.ReadAll(resp.Body)
- if err != nil {
- return err
- }
- return newError(resp.StatusCode, body)
+ return newError(resp)
}
if streamOptions.useJSONDecoder || resp.Header.Get("Content-Type") == "application/json" {
// if we want to get raw json stream, just copy it back to output
@@ -750,8 +742,13 @@ type Error struct {
Message string
}
-func newError(status int, body []byte) *Error {
- return &Error{Status: status, Message: string(body)}
+func newError(resp *http.Response) *Error {
+ defer resp.Body.Close()
+ data, err := ioutil.ReadAll(resp.Body)
+ if err != nil {
+ return &Error{Status: resp.StatusCode, Message: fmt.Sprintf("cannot read body, err: %v", err)}
+ }
+ return &Error{Status: resp.StatusCode, Message: string(data)}
}
func (e *Error) Error() string {
View
@@ -5,6 +5,7 @@
package docker
import (
+ "bytes"
"fmt"
"io/ioutil"
"net"
@@ -186,7 +187,12 @@ func TestGetURL(t *testing.T) {
}
func TestError(t *testing.T) {
- err := newError(400, []byte("bad parameter"))
+ fakeBody := ioutil.NopCloser(bytes.NewBufferString("bad parameter"))
+ resp := &http.Response{
+ StatusCode: 400,
+ Body: fakeBody,
+ }
+ err := newError(resp)
expected := Error{Status: 400, Message: "bad parameter"}
if !reflect.DeepEqual(expected, *err) {
t.Errorf("Wrong error type. Want %#v. Got %#v.", expected, *err)
Oops, something went wrong.

0 comments on commit edc2bd3

Please sign in to comment.