0ct0pu5/moby
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge pull request #43398 from thaJeztah/client_error_handling

Browse source 
client: remove wrapResponseError()
This commit is contained in:
Brian Goff 2022-03-23 22:26:59 -07:00 • committed by GitHub
commit 8236be1207
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
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) {