Permalink
Browse files

Set ping version even on error

In some cases a server may return an error on the ping response but
still provide version details. The client should use these values when
available.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information...
cpuguy83 committed Jun 26, 2017
1 parent 8a672ba commit 27ef09a46ffeb8ba42548de937b68351009f30ea
Showing with 132 additions and 33 deletions.
  1. +8 −6 client/ping.go
  2. +82 −0 client/ping_test.go
  3. +42 −27 client/request.go
View
@@ -18,13 +18,15 @@ func (cli *Client) Ping(ctx context.Context) (types.Ping, error) {
}
defer ensureReaderClosed(serverResp)
ping.APIVersion = serverResp.header.Get("API-Version")
if serverResp.header != nil {
ping.APIVersion = serverResp.header.Get("API-Version")
if serverResp.header.Get("Docker-Experimental") == "true" {
ping.Experimental = true
if serverResp.header.Get("Docker-Experimental") == "true" {
ping.Experimental = true
}
ping.OSType = serverResp.header.Get("OSType")
}
ping.OSType = serverResp.header.Get("OSType")
return ping, nil
err = cli.checkResponseErr(serverResp)
return ping, err
}
View
@@ -0,0 +1,82 @@
package client
import (
"errors"
"io/ioutil"
"net/http"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"golang.org/x/net/context"
)
// TestPingFail tests that when a server sends a non-successful response that we
// can still grab API details, when set.
// Some of this is just excercising the code paths to make sure there are no
// panics.
func TestPingFail(t *testing.T) {
var withHeader bool
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
resp := &http.Response{StatusCode: http.StatusInternalServerError}
if withHeader {
resp.Header = http.Header{}
resp.Header.Set("API-Version", "awesome")
resp.Header.Set("Docker-Experimental", "true")
}
resp.Body = ioutil.NopCloser(strings.NewReader("some error with the server"))
return resp, nil
}),
}
ping, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, false, ping.Experimental)
assert.Equal(t, "", ping.APIVersion)
withHeader = true
ping2, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, true, ping2.Experimental)
assert.Equal(t, "awesome", ping2.APIVersion)
}
// TestPingWithError tests the case where there is a protocol error in the ping.
// This test is mostly just testing that there are no panics in this code path.
func TestPingWithError(t *testing.T) {
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
resp := &http.Response{StatusCode: http.StatusInternalServerError}
resp.Header = http.Header{}
resp.Header.Set("API-Version", "awesome")
resp.Header.Set("Docker-Experimental", "true")
resp.Body = ioutil.NopCloser(strings.NewReader("some error with the server"))
return resp, errors.New("some error")
}),
}
ping, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, false, ping.Experimental)
assert.Equal(t, "", ping.APIVersion)
}
// TestPingSuccess tests that we are able to get the expected API headers/ping
// details on success.
func TestPingSuccess(t *testing.T) {
client := &Client{
client: newMockClient(func(req *http.Request) (*http.Response, error) {
resp := &http.Response{StatusCode: http.StatusInternalServerError}
resp.Header = http.Header{}
resp.Header.Set("API-Version", "awesome")
resp.Header.Set("Docker-Experimental", "true")
resp.Body = ioutil.NopCloser(strings.NewReader("some error with the server"))
return resp, nil
}),
}
ping, err := client.Ping(context.Background())
assert.Error(t, err)
assert.Equal(t, true, ping.Experimental)
assert.Equal(t, "awesome", ping.APIVersion)
}
View
@@ -24,6 +24,7 @@ type serverResponse struct {
body io.ReadCloser
header http.Header
statusCode int
reqURL *url.URL
}
// head sends an http request to the docker API using the method HEAD.
@@ -118,11 +119,18 @@ func (cli *Client) sendRequest(ctx context.Context, method, path string, query u
if err != nil {
return serverResponse{}, err
}
return cli.doRequest(ctx, req)
resp, err := cli.doRequest(ctx, req)
if err != nil {
return resp, err
}
if err := cli.checkResponseErr(resp); err != nil {
return resp, err
}
return resp, nil
}
func (cli *Client) doRequest(ctx context.Context, req *http.Request) (serverResponse, error) {
serverResp := serverResponse{statusCode: -1}
serverResp := serverResponse{statusCode: -1, reqURL: req.URL}
resp, err := ctxhttp.Do(ctx, cli.client, req)
if err != nil {
@@ -179,35 +187,42 @@ func (cli *Client) doRequest(ctx context.Context, req *http.Request) (serverResp
if resp != nil {
serverResp.statusCode = resp.StatusCode
serverResp.body = resp.Body
serverResp.header = resp.Header
}
return serverResp, nil
}
if serverResp.statusCode < 200 || serverResp.statusCode >= 400 {
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
return serverResp, err
}
if len(body) == 0 {
return serverResp, fmt.Errorf("Error: request returned %s for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), req.URL)
}
func (cli *Client) checkResponseErr(serverResp serverResponse) error {
if serverResp.statusCode >= 200 && serverResp.statusCode < 400 {
return nil
}
var errorMessage string
if (cli.version == "" || versions.GreaterThan(cli.version, "1.23")) &&
resp.Header.Get("Content-Type") == "application/json" {
var errorResponse types.ErrorResponse
if err := json.Unmarshal(body, &errorResponse); err != nil {
return serverResp, fmt.Errorf("Error reading JSON: %v", err)
}
errorMessage = errorResponse.Message
} else {
errorMessage = string(body)
}
body, err := ioutil.ReadAll(serverResp.body)
if err != nil {
return err
}
if len(body) == 0 {
return fmt.Errorf("Error: request returned %s for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), serverResp.reqURL)
}
return serverResp, fmt.Errorf("Error response from daemon: %s", strings.TrimSpace(errorMessage))
var ct string
if serverResp.header != nil {
ct = serverResp.header.Get("Content-Type")
}
serverResp.body = resp.Body
serverResp.header = resp.Header
return serverResp, nil
var errorMessage string
if (cli.version == "" || versions.GreaterThan(cli.version, "1.23")) && ct == "application/json" {
var errorResponse types.ErrorResponse
if err := json.Unmarshal(body, &errorResponse); err != nil {
return fmt.Errorf("Error reading JSON: %v", err)
}
errorMessage = errorResponse.Message
} else {
errorMessage = string(body)
}
return fmt.Errorf("Error response from daemon: %s", strings.TrimSpace(errorMessage))
}
func (cli *Client) addHeaders(req *http.Request, headers headers) *http.Request {
@@ -239,9 +254,9 @@ func encodeData(data interface{}) (*bytes.Buffer, error) {
}
func ensureReaderClosed(response serverResponse) {
if body := response.body; body != nil {
if response.body != nil {
// Drain up to 512 bytes and close the body to let the Transport reuse the connection
io.CopyN(ioutil.Discard, body, 512)
io.CopyN(ioutil.Discard, response.body, 512)
response.body.Close()
}
}

0 comments on commit 27ef09a

Please sign in to comment.