From c8ec4559ba79b9ca1eba7d2798f55bc86dcf697c Mon Sep 17 00:00:00 2001 From: Steven Hartland Date: Fri, 16 Aug 2024 06:40:33 +0100 Subject: [PATCH] chore: improve error wrapping (#2720) Improve error wrapping to help detect failures in compose and wait tests. --- docker.go | 12 ++++++------ generic.go | 4 ++-- modules/compose/compose.go | 6 +++--- modules/compose/compose_api.go | 20 ++++++++++---------- modules/compose/compose_local.go | 25 +++++++++++++------------ 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/docker.go b/docker.go index 6bd80c39c4..5650f88612 100644 --- a/docker.go +++ b/docker.go @@ -508,12 +508,12 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string, options ...tce response, err := cli.ContainerExecCreate(ctx, c.ID, processOptions.ExecConfig) if err != nil { - return 0, nil, err + return 0, nil, fmt.Errorf("container exec create: %w", err) } hijack, err := cli.ContainerExecAttach(ctx, response.ID, container.ExecAttachOptions{}) if err != nil { - return 0, nil, err + return 0, nil, fmt.Errorf("container exec attach: %w", err) } processOptions.Reader = hijack.Reader @@ -528,7 +528,7 @@ func (c *DockerContainer) Exec(ctx context.Context, cmd []string, options ...tce for { execResp, err := cli.ContainerExecInspect(ctx, response.ID) if err != nil { - return 0, nil, err + return 0, nil, fmt.Errorf("container exec inspect: %w", err) } if !execResp.Running { @@ -1129,7 +1129,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque resp, err := p.client.ContainerCreate(ctx, dockerInput, hostConfig, networkingConfig, platform, req.Name) if err != nil { - return nil, err + return nil, fmt.Errorf("container create: %w", err) } // #248: If there is more than one network specified in the request attach newly created container to them one by one @@ -1144,7 +1144,7 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque } err = p.client.NetworkConnect(ctx, nw.ID, resp.ID, &endpointSetting) if err != nil { - return nil, err + return nil, fmt.Errorf("network connect: %w", err) } } } @@ -1245,7 +1245,7 @@ func (p *DockerProvider) ReuseOrCreateContainer(ctx context.Context, req Contain if !p.config.RyukDisabled { r, err := reuseOrCreateReaper(context.WithValue(ctx, core.DockerHostContextKey, p.host), sessionID, p) if err != nil { - return nil, fmt.Errorf("%w: creating reaper failed", err) + return nil, fmt.Errorf("reaper: %w", err) } termSignal, err = r.Connect() if err != nil { diff --git a/generic.go b/generic.go index f0cda13407..4c214744e7 100644 --- a/generic.go +++ b/generic.go @@ -57,7 +57,7 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain } provider, err := req.ProviderType.GetProvider(WithLogger(logging)) if err != nil { - return nil, err + return nil, fmt.Errorf("get provider: %w", err) } defer provider.Close() @@ -79,7 +79,7 @@ func GenericContainer(ctx context.Context, req GenericContainerRequest) (Contain if req.Started && !c.IsRunning() { if err := c.Start(ctx); err != nil { - return c, fmt.Errorf("failed to start container: %w", err) + return c, fmt.Errorf("start container: %w", err) } } return c, nil diff --git a/modules/compose/compose.go b/modules/compose/compose.go index 38adfafd2f..b0f26370cb 100644 --- a/modules/compose/compose.go +++ b/modules/compose/compose.go @@ -129,7 +129,7 @@ func NewDockerComposeWith(opts ...ComposeStackOption) (*dockerCompose, error) { for i := range opts { if err := opts[i].applyToComposeStack(&composeOptions); err != nil { - return nil, err + return nil, fmt.Errorf("apply compose stack option: %w", err) } } @@ -139,11 +139,11 @@ func NewDockerComposeWith(opts ...ComposeStackOption) (*dockerCompose, error) { dockerCli, err := command.NewDockerCli() if err != nil { - return nil, err + return nil, fmt.Errorf("new docker client: %w", err) } if err = dockerCli.Initialize(flags.NewClientOptions(), command.WithInitializeClient(makeClient)); err != nil { - return nil, err + return nil, fmt.Errorf("initialize docker client: %w", err) } reaperProvider, err := testcontainers.NewDockerProvider() diff --git a/modules/compose/compose_api.go b/modules/compose/compose_api.go index 426715f0d5..129f55897e 100644 --- a/modules/compose/compose_api.go +++ b/modules/compose/compose_api.go @@ -120,19 +120,19 @@ func (r ComposeStackReaders) applyToComposeStack(o *composeStackOptions) error { tmp = filepath.Join(tmp, strconv.FormatInt(time.Now().UnixNano(), 10)) err := os.MkdirAll(tmp, 0o755) if err != nil { - return fmt.Errorf("failed to create temporary directory: %w", err) + return fmt.Errorf("create temporary directory: %w", err) } name := fmt.Sprintf(baseName, i) bs, err := io.ReadAll(reader) if err != nil { - return fmt.Errorf("failed to read from reader: %w", err) + return fmt.Errorf("read from reader: %w", err) } err = os.WriteFile(filepath.Join(tmp, name), bs, 0o644) if err != nil { - return fmt.Errorf("failed to write to temporary file: %w", err) + return fmt.Errorf("write to temporary file: %w", err) } f[i] = filepath.Join(tmp, name) @@ -311,7 +311,7 @@ func (d *dockerCompose) Up(ctx context.Context, opts ...StackUpOption) error { }, }) if err != nil { - return err + return fmt.Errorf("compose up: %w", err) } err = d.lookupNetworks(ctx) @@ -442,7 +442,7 @@ func (d *dockerCompose) lookupContainer(ctx context.Context, svcName string) (*t ), }) if err != nil { - return nil, err + return nil, fmt.Errorf("container list: %w", err) } if len(containers) == 0 { @@ -458,7 +458,7 @@ func (d *dockerCompose) lookupContainer(ctx context.Context, svcName string) (*t dockerProvider, err := testcontainers.NewDockerProvider(testcontainers.WithLogger(d.logger)) if err != nil { - return nil, err + return nil, fmt.Errorf("new docker provider: %w", err) } dockerProvider.SetClient(d.dockerClient) @@ -479,7 +479,7 @@ func (d *dockerCompose) lookupNetworks(ctx context.Context) error { ), }) if err != nil { - return err + return fmt.Errorf("network list: %w", err) } for _, n := range networks { @@ -504,12 +504,12 @@ func (d *dockerCompose) compileProject(ctx context.Context) (*types.Project, err compiledOptions, err := cli.NewProjectOptions(d.configs, projectOptions...) if err != nil { - return nil, err + return nil, fmt.Errorf("new project options: %w", err) } proj, err := compiledOptions.LoadProject(ctx) if err != nil { - return nil, err + return nil, fmt.Errorf("load project: %w", err) } for i, s := range proj.Services { @@ -568,7 +568,7 @@ func withEnv(env map[string]string) func(*cli.ProjectOptions) error { func makeClient(*command.DockerCli) (client.APIClient, error) { dockerClient, err := testcontainers.NewDockerClientWithOpts(context.Background()) if err != nil { - return nil, err + return nil, fmt.Errorf("new docker client: %w", err) } return dockerClient, nil } diff --git a/modules/compose/compose_local.go b/modules/compose/compose_local.go index cb818dbc94..964547bae7 100644 --- a/modules/compose/compose_local.go +++ b/modules/compose/compose_local.go @@ -136,7 +136,7 @@ func (dc *LocalDockerCompose) containerNameFromServiceName(service, separator st func (dc *LocalDockerCompose) applyStrategyToRunningContainer() error { cli, err := testcontainers.NewDockerClientWithOpts(context.Background()) if err != nil { - return err + return fmt.Errorf("new docker client: %w", err) } defer cli.Close() @@ -150,22 +150,22 @@ func (dc *LocalDockerCompose) applyStrategyToRunningContainer() error { containerListOptions := container.ListOptions{Filters: f, All: true} containers, err := cli.ContainerList(context.Background(), containerListOptions) if err != nil { - return fmt.Errorf("error %w occurred while filtering the service %s: %d by name and published port", err, k.service, k.publishedPort) + return fmt.Errorf("container list service %q: %w", k.service, err) } if len(containers) == 0 { - return fmt.Errorf("service with name %s not found in list of running containers", k.service) + return fmt.Errorf("service with name %q not found in list of running containers", k.service) } // The length should always be a list of 1, since we are matching one service name at a time if l := len(containers); l > 1 { - return fmt.Errorf("expecting only one running container for %s but got %d", k.service, l) + return fmt.Errorf("expecting only one running container for %q but got %d", k.service, l) } container := containers[0] strategy := dc.WaitStrategyMap[k] dockerProvider, err := testcontainers.NewDockerProvider(testcontainers.WithLogger(dc.Logger)) if err != nil { - return fmt.Errorf("unable to create new Docker Provider: %w", err) + return fmt.Errorf("new docker provider: %w", err) } defer dockerProvider.Close() @@ -175,7 +175,7 @@ func (dc *LocalDockerCompose) applyStrategyToRunningContainer() error { err = strategy.WaitUntilReady(context.Background(), dockercontainer) if err != nil { - return fmt.Errorf("unable to apply wait strategy %v to service %s due to %w", strategy, k.service, err) + return fmt.Errorf("wait until ready %v to service %q due: %w", strategy, k.service, err) } } return nil @@ -223,7 +223,6 @@ func (dc *LocalDockerCompose) WithExposedService(service string, port int, strat // depending on the version services names are composed in a different way func (dc *LocalDockerCompose) determineVersion() error { execErr := executeCompose(dc, []string{"version", "--short"}) - if err := execErr.Error; err != nil { return err } @@ -235,7 +234,7 @@ func (dc *LocalDockerCompose) determineVersion() error { majorVersion, err := strconv.ParseInt(string(components[0]), 10, 8) if err != nil { - return err + return fmt.Errorf("parsing major version: %w", err) } switch majorVersion { @@ -263,11 +262,11 @@ func (dc *LocalDockerCompose) validate() error { yamlFile, err := os.ReadFile(abs) if err != nil { - return err + return fmt.Errorf("read compose file %q: %w", abs, err) } err = yaml.Unmarshal(yamlFile, &c) if err != nil { - return err + return fmt.Errorf("unmarshalling file %q: %w", abs, err) } if dc.Services == nil { @@ -448,7 +447,9 @@ func (w *capturingPassThroughWriter) Bytes() []byte { // Which checks if a binary is present in PATH func which(binary string) error { - _, err := exec.LookPath(binary) + if _, err := exec.LookPath(binary); err != nil { + return fmt.Errorf("lookup: %w", err) + } - return err + return nil }