client: improve error messaging on crash
Repro steps:
- Run Docker Desktop
- Run `docker run busybox tail -f /dev/null`
- Run `pkill "Docker Desktop"
Expected:
An error message that indicates that Docker Desktop is shutting down.
Actual:
An error message that looks like this:
```
error waiting for container: invalid character 's' looking for beginning of value
```
here's an example:
https://github.com/docker/for-mac/issues/6575#issuecomment-1324879001
After this change, you get an error message like:
```
error waiting for container: copying response body from Docker: unexpected EOF
```
which is a bit more explicit.
Signed-off-by: Nick Santos <nick.santos@docker.com>
(cherry picked from commit 9900c7a348
)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
parent
9f62b37a62
commit
8b83e09b9c
2 changed files with 76 additions and 2 deletions
|
@ -1,14 +1,19 @@
|
|||
package client // import "github.com/docker/docker/client"
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"net/url"
|
||||
|
||||
"github.com/docker/docker/api/types/container"
|
||||
"github.com/docker/docker/api/types/versions"
|
||||
)
|
||||
|
||||
const containerWaitErrorMsgLimit = 2 * 1024 /* Max: 2KiB */
|
||||
|
||||
// ContainerWait waits until the specified container is in a certain state
|
||||
// indicated by the given condition, either "not-running" (default),
|
||||
// "next-exit", or "removed".
|
||||
|
@ -46,9 +51,23 @@ func (cli *Client) ContainerWait(ctx context.Context, containerID string, condit
|
|||
|
||||
go func() {
|
||||
defer ensureReaderClosed(resp)
|
||||
|
||||
body := resp.body
|
||||
responseText := bytes.NewBuffer(nil)
|
||||
stream := io.TeeReader(body, responseText)
|
||||
|
||||
var res container.WaitResponse
|
||||
if err := json.NewDecoder(resp.body).Decode(&res); err != nil {
|
||||
errC <- err
|
||||
if err := json.NewDecoder(stream).Decode(&res); err != nil {
|
||||
// NOTE(nicks): The /wait API does not work well with HTTP proxies.
|
||||
// At any time, the proxy could cut off the response stream.
|
||||
//
|
||||
// But because the HTTP status has already been written, the proxy's
|
||||
// only option is to write a plaintext error message.
|
||||
//
|
||||
// If there's a JSON parsing error, read the real error message
|
||||
// off the body and send it to the client.
|
||||
_, _ = io.ReadAll(io.LimitReader(stream, containerWaitErrorMsgLimit))
|
||||
errC <- errors.New(responseText.String())
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
@ -62,6 +62,61 @@ func TestContainerWait(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestContainerWaitProxyInterrupt(t *testing.T) {
|
||||
expectedURL := "/v1.30/containers/container_id/wait"
|
||||
msg := "copying response body from Docker: unexpected EOF"
|
||||
client := &Client{
|
||||
version: "1.30",
|
||||
client: newMockClient(func(req *http.Request) (*http.Response, error) {
|
||||
if !strings.HasPrefix(req.URL.Path, expectedURL) {
|
||||
return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL)
|
||||
}
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Body: io.NopCloser(strings.NewReader(msg)),
|
||||
}, nil
|
||||
}),
|
||||
}
|
||||
|
||||
resultC, errC := client.ContainerWait(context.Background(), "container_id", "")
|
||||
select {
|
||||
case err := <-errC:
|
||||
if !strings.Contains(err.Error(), msg) {
|
||||
t.Fatalf("Expected: %s, Actual: %s", msg, err.Error())
|
||||
}
|
||||
case result := <-resultC:
|
||||
t.Fatalf("Unexpected result: %v", result)
|
||||
}
|
||||
}
|
||||
|
||||
func TestContainerWaitProxyInterruptLong(t *testing.T) {
|
||||
expectedURL := "/v1.30/containers/container_id/wait"
|
||||
msg := strings.Repeat("x", containerWaitErrorMsgLimit*5)
|
||||
client := &Client{
|
||||
version: "1.30",
|
||||
client: newMockClient(func(req *http.Request) (*http.Response, error) {
|
||||
if !strings.HasPrefix(req.URL.Path, expectedURL) {
|
||||
return nil, fmt.Errorf("Expected URL '%s', got '%s'", expectedURL, req.URL)
|
||||
}
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Body: io.NopCloser(strings.NewReader(msg)),
|
||||
}, nil
|
||||
}),
|
||||
}
|
||||
|
||||
resultC, errC := client.ContainerWait(context.Background(), "container_id", "")
|
||||
select {
|
||||
case err := <-errC:
|
||||
// LimitReader limiting isn't exact, because of how the Readers do chunking.
|
||||
if len(err.Error()) > containerWaitErrorMsgLimit*2 {
|
||||
t.Fatalf("Expected error to be limited around %d, actual length: %d", containerWaitErrorMsgLimit, len(err.Error()))
|
||||
}
|
||||
case result := <-resultC:
|
||||
t.Fatalf("Unexpected result: %v", result)
|
||||
}
|
||||
}
|
||||
|
||||
func ExampleClient_ContainerWait_withTimeout() {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
|
|
Loading…
Add table
Reference in a new issue