kotovalexarian-likes-github/moby--moby
1
0
Fork 0
mirror of https://github.com/moby/moby.git synced 2022-11-09 12:21:53 -05:00
Code Releases Activity

client: remove wrapResponseError()

Browse source 
The wrapResponseError() utility converted some specific errors, but in
doing so, could hide the actual error message returned by the daemon.
In addition, starting with 38e6d474af,
HTTP status codes were already mapped to their corresponding errdefs
types on the client-side, making this conversion redundant.

This patch removes the wrapResponseError() utility; it's worth noting
that some error-messages will change slightly (as they now return the
error as returned by the daemon), but may cointain more details as
before, and in some cases prevents hiding the actual error.

Before this change:

    docker container rm nosuchcontainer
    Error: No such container: nosuchcontainer

    docker container cp mycontainer:/no/such/path .
    Error: No such container:path: mycontainer:/no/such/path

    docker container cp ./Dockerfile mycontainer:/no/such/path
    Error: No such container:path: mycontainer:/no/such

    docker image rm nosuchimage
    Error: No such image: nosuchimage

    docker network rm nosuchnetwork
    Error: No such network: nosuchnetwork

    docker volume rm nosuchvolume
    Error: No such volume: nosuchvolume

    docker plugin rm nosuchplugin
    Error: No such plugin: nosuchplugin

    docker checkpoint rm nosuchcontainer nosuchcheckpoint
    Error response from daemon: No such container: nosuchcontainer

    docker checkpoint rm mycontainer nosuchcheckpoint
    Error response from daemon: checkpoint nosuchcheckpoint does not exist for container mycontainer

    docker service rm nosuchservice
    Error: No such service: nosuchservice

    docker node rm nosuchnode
    Error: No such node: nosuchnode

    docker config rm nosuschconfig
    Error: No such config: nosuschconfig

    docker secret rm nosuchsecret
    Error: No such secret: nosuchsecret

After this change:

    docker container rm nosuchcontainer
    Error response from daemon: No such container: nosuchcontainer

    docker container cp mycontainer:/no/such/path .
    Error response from daemon: Could not find the file /no/such/path in container mycontainer

    docker container cp ./Dockerfile mycontainer:/no/such/path
    Error response from daemon: Could not find the file /no/such in container mycontainer

    docker image rm nosuchimage
    Error response from daemon: No such image: nosuchimage:latest

    docker network rm nosuchnetwork
    Error response from daemon: network nosuchnetwork not found

    docker volume rm nosuchvolume
    Error response from daemon: get nosuchvolume: no such volume

    docker plugin rm nosuchplugin
    Error response from daemon: plugin "nosuchplugin" not found

    docker checkpoint rm nosuchcontainer nosuchcheckpoint
    Error response from daemon: No such container: nosuchcontainer

    docker checkpoint rm mycontainer nosuchcheckpoint
    Error response from daemon: checkpoint nosuchcheckpoint does not exist for container mycontainer

    docker service rm nosuchservice
    Error response from daemon: service nosuchservice not found

    docker node rm nosuchnode
    Error response from daemon: node nosuchnode not found

    docker config rm nosuchconfig
    Error response from daemon: config nosuchconfig not found

    docker secret rm nosuchsecret
    Error response from daemon: secret nosuchsecret not found

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2022-03-20 16:55:42 +01:00
parent d5d5f258df
commit 45067cda33
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
30 changed files with 39 additions and 59 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
client/checkpoint_list.go
View file

@ -20,7 +20,7 @@ func (cli *Client) CheckpointList(ctx context.Context, container string, options
resp, err := cli.get(ctx, "/containers/"+container+"/checkpoints", query, nil)
defer ensureReaderClosed(resp)
if err != nil {
return checkpoints, wrapResponseError(err, resp, "container", container)
return checkpoints, err
}
err = json.NewDecoder(resp.body).Decode(&checkpoints)

2
client/config_inspect.go
View file

@ -20,7 +20,7 @@ func (cli *Client) ConfigInspectWithRaw(ctx context.Context, id string) (swarm.C
resp, err := cli.get(ctx, "/configs/"+id, nil, nil)
defer ensureReaderClosed(resp)
if err != nil {
return swarm.Config{}, nil, wrapResponseError(err, resp, "config", id)
return swarm.Config{}, nil, err
}
body, err := io.ReadAll(resp.body)

2
client/config_remove.go
View file

@ -9,5 +9,5 @@ func (cli *Client) ConfigRemove(ctx context.Context, id string) error {
}
resp, err := cli.delete(ctx, "/configs/"+id, nil, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "config", id)
return err
}

6
client/container_copy.go
View file

@ -23,7 +23,7 @@ func (cli *Client) ContainerStatPath(ctx context.Context, containerID, path stri
response, err := cli.head(ctx, urlStr, query, nil)
defer ensureReaderClosed(response)
if err != nil {
return types.ContainerPathStat{}, wrapResponseError(err, response, "container:path", containerID+":"+path)
return types.ContainerPathStat{}, err
}
return getContainerPathStatFromHeader(response.header)
}
@ -47,7 +47,7 @@ func (cli *Client) CopyToContainer(ctx context.Context, containerID, dstPath str
response, err := cli.putRaw(ctx, apiPath, query, content, nil)
defer ensureReaderClosed(response)
if err != nil {
return wrapResponseError(err, response, "container:path", containerID+":"+dstPath)
return err
}
// TODO this code converts non-error status-codes (e.g., "204 No Content") into an error; verify if this is the desired behavior
@ -67,7 +67,7 @@ func (cli *Client) CopyFromContainer(ctx context.Context, containerID, srcPath s
apiPath := "/containers/" + containerID + "/archive"
response, err := cli.get(ctx, apiPath, query, nil)
if err != nil {
return nil, types.ContainerPathStat{}, wrapResponseError(err, response, "container:path", containerID+":"+srcPath)
return nil, types.ContainerPathStat{}, err
}
// TODO this code converts non-error status-codes (e.g., "204 No Content") into an error; verify if this is the desired behavior

4
client/container_inspect.go
View file

@ -18,7 +18,7 @@ func (cli *Client) ContainerInspect(ctx context.Context, containerID string) (ty
serverResp, err := cli.get(ctx, "/containers/"+containerID+"/json", nil, nil)
defer ensureReaderClosed(serverResp)
if err != nil {
return types.ContainerJSON{}, wrapResponseError(err, serverResp, "container", containerID)
return types.ContainerJSON{}, err
}
var response types.ContainerJSON
@ -38,7 +38,7 @@ func (cli *Client) ContainerInspectWithRaw(ctx context.Context, containerID stri
serverResp, err := cli.get(ctx, "/containers/"+containerID+"/json", query, nil)
defer ensureReaderClosed(serverResp)
if err != nil {
return types.ContainerJSON{}, nil, wrapResponseError(err, serverResp, "container", containerID)
return types.ContainerJSON{}, nil, err
}
body, err := io.ReadAll(serverResp.body)

2
client/container_logs.go
View file

@ -74,7 +74,7 @@ func (cli *Client) ContainerLogs(ctx context.Context, container string, options
resp, err := cli.get(ctx, "/containers/"+container+"/logs", query, nil)
if err != nil {
return nil, wrapResponseError(err, resp, "container", container)
return nil, err
}
return resp.body, nil
}

2
client/container_remove.go
View file

@ -23,5 +23,5 @@ func (cli *Client) ContainerRemove(ctx context.Context, containerID string, opti
resp, err := cli.delete(ctx, "/containers/"+containerID, query, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "container", containerID)
return err
}

5
client/container_remove_test.go
View file

@ -12,7 +12,6 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/errdefs"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
func TestContainerRemoveError(t *testing.T) {
@ -27,10 +26,10 @@ func TestContainerRemoveError(t *testing.T) {
func TestContainerRemoveNotFoundError(t *testing.T) {
client := &Client{
client: newMockClient(errorMock(http.StatusNotFound, "missing")),
client: newMockClient(errorMock(http.StatusNotFound, "no such container: container_id")),
}
err := client.ContainerRemove(context.Background(), "container_id", types.ContainerRemoveOptions{})
assert.Check(t, is.Error(err, "Error: No such container: container_id"))
assert.ErrorContains(t, err, "no such container: container_id")
assert.Check(t, IsErrNotFound(err))
}

14
client/errors.go
View file

@ -2,7 +2,6 @@ package client // import "github.com/docker/docker/client"
import (
"fmt"
"net/http"
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/errdefs"
@ -59,19 +58,6 @@ func (e objectNotFoundError) Error() string {
return fmt.Sprintf("Error: No such %s: %s", e.object, e.id)
}
func wrapResponseError(err error, resp serverResponse, object, id string) error {
switch {
case err == nil:
return nil
case resp.statusCode == http.StatusNotFound:
return objectNotFoundError{object: object, id: id}
case resp.statusCode == http.StatusNotImplemented:
return errdefs.NotImplemented(err)
default:
return err
}
}
// unauthorizedError represents an authorization error in a remote registry.
type unauthorizedError struct {
cause error

2
client/image_inspect.go
View file

@ -17,7 +17,7 @@ func (cli *Client) ImageInspectWithRaw(ctx context.Context, imageID string) (typ
serverResp, err := cli.get(ctx, "/images/"+imageID+"/json", nil, nil)
defer ensureReaderClosed(serverResp)
if err != nil {
return types.ImageInspect{}, nil, wrapResponseError(err, serverResp, "image", imageID)
return types.ImageInspect{}, nil, err
}
body, err := io.ReadAll(serverResp.body)

2
client/image_remove.go
View file

@ -23,7 +23,7 @@ func (cli *Client) ImageRemove(ctx context.Context, imageID string, options type
resp, err := cli.delete(ctx, "/images/"+imageID, query, nil)
defer ensureReaderClosed(resp)
if err != nil {
return dels, wrapResponseError(err, resp, "image", imageID)
return dels, err
}
err = json.NewDecoder(resp.body).Decode(&dels)

5
client/image_remove_test.go
View file

@ -13,7 +13,6 @@ import (
"github.com/docker/docker/api/types"
"github.com/docker/docker/errdefs"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
func TestImageRemoveError(t *testing.T) {
@ -29,11 +28,11 @@ func TestImageRemoveError(t *testing.T) {
func TestImageRemoveImageNotFound(t *testing.T) {
client := &Client{
client: newMockClient(errorMock(http.StatusNotFound, "missing")),
client: newMockClient(errorMock(http.StatusNotFound, "no such image: unknown")),
}
_, err := client.ImageRemove(context.Background(), "unknown", types.ImageRemoveOptions{})
assert.Check(t, is.Error(err, "Error: No such image: unknown"))
assert.ErrorContains(t, err, "no such image: unknown")
assert.Check(t, IsErrNotFound(err))
}

2
client/network_inspect.go
View file

@ -36,7 +36,7 @@ func (cli *Client) NetworkInspectWithRaw(ctx context.Context, networkID string,
resp, err = cli.get(ctx, "/networks/"+networkID, query, nil)
defer ensureReaderClosed(resp)
if err != nil {
return networkResource, nil, wrapResponseError(err, resp, "network", networkID)
return networkResource, nil, err
}
body, err := io.ReadAll(resp.body)

7
client/network_inspect_test.go
View file

@ -14,7 +14,6 @@ import (
"github.com/docker/docker/api/types/network"
"github.com/docker/docker/errdefs"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
func TestNetworkInspect(t *testing.T) {
@ -37,7 +36,7 @@ func TestNetworkInspect(t *testing.T) {
return nil, errors.New("expected URL '/networks/network_id', got " + req.URL.Path)
}
if strings.Contains(req.URL.RawQuery, "scope=global") {
return errorMock(http.StatusNotFound, "Error: No such network: unknown")(req)
return errorMock(http.StatusNotFound, "Error: No such network: network_id")(req)
}
var (
content []byte
@ -88,12 +87,12 @@ func TestNetworkInspect(t *testing.T) {
})
t.Run("global scope", func(t *testing.T) {
_, err := client.NetworkInspect(context.Background(), "network_id", types.NetworkInspectOptions{Scope: "global"})
assert.Check(t, is.Error(err, "Error: No such network: network_id"))
assert.ErrorContains(t, err, "Error: No such network: network_id")
assert.Check(t, IsErrNotFound(err))
})
t.Run("unknown network", func(t *testing.T) {
_, err := client.NetworkInspect(context.Background(), "unknown", types.NetworkInspectOptions{})
assert.Check(t, is.Error(err, "Error: No such network: unknown"))
assert.ErrorContains(t, err, "Error: No such network: unknown")
assert.Check(t, IsErrNotFound(err))
})
t.Run("server error", func(t *testing.T) {

2
client/network_remove.go
View file

@ -6,5 +6,5 @@ import "context"
func (cli *Client) NetworkRemove(ctx context.Context, networkID string) error {
resp, err := cli.delete(ctx, "/networks/"+networkID, nil, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "network", networkID)
return err
}

2
client/node_inspect.go
View file

@ -17,7 +17,7 @@ func (cli *Client) NodeInspectWithRaw(ctx context.Context, nodeID string) (swarm
serverResp, err := cli.get(ctx, "/nodes/"+nodeID, nil, nil)
defer ensureReaderClosed(serverResp)
if err != nil {
return swarm.Node{}, nil, wrapResponseError(err, serverResp, "node", nodeID)
return swarm.Node{}, nil, err
}
body, err := io.ReadAll(serverResp.body)

2
client/node_remove.go
View file

@ -16,5 +16,5 @@ func (cli *Client) NodeRemove(ctx context.Context, nodeID string, options types.
resp, err := cli.delete(ctx, "/nodes/"+nodeID, query, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "node", nodeID)
return err
}

2
client/plugin_inspect.go
View file

@ -17,7 +17,7 @@ func (cli *Client) PluginInspectWithRaw(ctx context.Context, name string) (*type
resp, err := cli.get(ctx, "/plugins/"+name+"/json", nil, nil)
defer ensureReaderClosed(resp)
if err != nil {
return nil, nil, wrapResponseError(err, resp, "plugin", name)
return nil, nil, err
}
body, err := io.ReadAll(resp.body)

2
client/plugin_list.go
View file

@ -25,7 +25,7 @@ func (cli *Client) PluginList(ctx context.Context, filter filters.Args) (types.P
resp, err := cli.get(ctx, "/plugins", query, nil)
defer ensureReaderClosed(resp)
if err != nil {
return plugins, wrapResponseError(err, resp, "plugin", "")
return plugins, err
}
err = json.NewDecoder(resp.body).Decode(&plugins)

2
client/plugin_remove.go
View file

@ -16,5 +16,5 @@ func (cli *Client) PluginRemove(ctx context.Context, name string, options types.
resp, err := cli.delete(ctx, "/plugins/"+name, query, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "plugin", name)
return err
}

2
client/secret_inspect.go
View file

@ -20,7 +20,7 @@ func (cli *Client) SecretInspectWithRaw(ctx context.Context, id string) (swarm.S
resp, err := cli.get(ctx, "/secrets/"+id, nil, nil)
defer ensureReaderClosed(resp)
if err != nil {
return swarm.Secret{}, nil, wrapResponseError(err, resp, "secret", id)
return swarm.Secret{}, nil, err
}
body, err := io.ReadAll(resp.body)

2
client/secret_remove.go
View file

@ -9,5 +9,5 @@ func (cli *Client) SecretRemove(ctx context.Context, id string) error {
}
resp, err := cli.delete(ctx, "/secrets/"+id, nil, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "secret", id)
return err
}

2
client/service_inspect.go
View file

@ -22,7 +22,7 @@ func (cli *Client) ServiceInspectWithRaw(ctx context.Context, serviceID string,
serverResp, err := cli.get(ctx, "/services/"+serviceID, query, nil)
defer ensureReaderClosed(serverResp)
if err != nil {
return swarm.Service{}, nil, wrapResponseError(err, serverResp, "service", serviceID)
return swarm.Service{}, nil, err
}
body, err := io.ReadAll(serverResp.body)

2
client/service_remove.go
View file

@ -6,5 +6,5 @@ import "context"
func (cli *Client) ServiceRemove(ctx context.Context, serviceID string) error {
resp, err := cli.delete(ctx, "/services/"+serviceID, nil, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "service", serviceID)
return err
}

5
client/service_remove_test.go
View file

@ -11,7 +11,6 @@ import (
"github.com/docker/docker/errdefs"
"gotest.tools/v3/assert"
is "gotest.tools/v3/assert/cmp"
)
func TestServiceRemoveError(t *testing.T) {
@ -27,11 +26,11 @@ func TestServiceRemoveError(t *testing.T) {
func TestServiceRemoveNotFoundError(t *testing.T) {
client := &Client{
client: newMockClient(errorMock(http.StatusNotFound, "missing")),
client: newMockClient(errorMock(http.StatusNotFound, "no such service: service_id")),
}
err := client.ServiceRemove(context.Background(), "service_id")
assert.Check(t, is.Error(err, "Error: No such service: service_id"))
assert.ErrorContains(t, err, "no such service: service_id")
assert.Check(t, IsErrNotFound(err))
}

2
client/task_inspect.go
View file

@ -17,7 +17,7 @@ func (cli *Client) TaskInspectWithRaw(ctx context.Context, taskID string) (swarm
serverResp, err := cli.get(ctx, "/tasks/"+taskID, nil, nil)
defer ensureReaderClosed(serverResp)
if err != nil {
return swarm.Task{}, nil, wrapResponseError(err, serverResp, "task", taskID)
return swarm.Task{}, nil, err
}
body, err := io.ReadAll(serverResp.body)

2
client/volume_inspect.go
View file

@ -25,7 +25,7 @@ func (cli *Client) VolumeInspectWithRaw(ctx context.Context, volumeID string) (t
resp, err := cli.get(ctx, "/volumes/"+volumeID, nil, nil)
defer ensureReaderClosed(resp)
if err != nil {
return volume, nil, wrapResponseError(err, resp, "volume", volumeID)
return volume, nil, err
}
body, err := io.ReadAll(resp.body)

2
client/volume_remove.go
View file

@ -17,5 +17,5 @@ func (cli *Client) VolumeRemove(ctx context.Context, volumeID string, force bool
}
resp, err := cli.delete(ctx, "/volumes/"+volumeID, query, nil)
defer ensureReaderClosed(resp)
return wrapResponseError(err, resp, "volume", volumeID)
return err
}

3
integration-cli/docker_api_containers_test.go
View file

@ -23,6 +23,7 @@ import (
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/client"
dconfig "github.com/docker/docker/daemon/config"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/integration-cli/cli"
"github.com/docker/docker/integration-cli/cli/build"
"github.com/docker/docker/pkg/ioutils"
@ -1590,7 +1591,7 @@ func (s *DockerSuite) TestContainerAPIDeleteWithEmptyName(c *testing.T) {
defer cli.Close()
err = cli.ContainerRemove(context.Background(), "", types.ContainerRemoveOptions{})
assert.ErrorContains(c, err, "No such container")
assert.Check(c, errdefs.IsNotFound(err))
}
func (s *DockerSuite) TestContainerAPIStatsWithNetworkDisabled(c *testing.T) {

7
integration/container/copy_test.go
View file

@ -4,7 +4,6 @@ import (
"archive/tar"
"context"
"encoding/json"
"fmt"
"io"
"os"
"testing"
@ -28,8 +27,7 @@ func TestCopyFromContainerPathDoesNotExist(t *testing.T) {
_, _, err := apiclient.CopyFromContainer(ctx, cid, "/dne")
assert.Check(t, client.IsErrNotFound(err))
expected := fmt.Sprintf("No such container:path: %s:%s", cid, "/dne")
assert.Check(t, is.ErrorContains(err, expected))
assert.ErrorContains(t, err, "Could not find the file /dne in container "+cid)
}
func TestCopyFromContainerPathIsNotDir(t *testing.T) {
@ -58,8 +56,7 @@ func TestCopyToContainerPathDoesNotExist(t *testing.T) {
err := apiclient.CopyToContainer(ctx, cid, "/dne", nil, types.CopyToContainerOptions{})
assert.Check(t, client.IsErrNotFound(err))
expected := fmt.Sprintf("No such container:path: %s:%s", cid, "/dne")
assert.Check(t, is.ErrorContains(err, expected))
assert.ErrorContains(t, err, "Could not find the file /dne in container "+cid)
}
func TestCopyToContainerPathIsNotDir(t *testing.T) {