From 8da3c3092289407932a0441555ed9051fd9585b6 Mon Sep 17 00:00:00 2001 From: "hiroto.toyoda" Date: Thu, 1 Jan 2026 01:02:31 +0900 Subject: [PATCH 1/4] Improve error handling in attach.go Signed-off-by: hiroto.toyoda --- pkg/compose/attach.go | 67 ++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/pkg/compose/attach.go b/pkg/compose/attach.go index 47b5888ff5..639e0ea53f 100644 --- a/pkg/compose/attach.go +++ b/pkg/compose/attach.go @@ -28,6 +28,7 @@ import ( containerType "github.com/docker/docker/api/types/container" "github.com/docker/docker/pkg/stdcopy" "github.com/moby/term" + "github.com/sirupsen/logrus" "github.com/docker/compose/v5/pkg/api" "github.com/docker/compose/v5/pkg/utils" @@ -49,7 +50,11 @@ func (s *composeService) attach(ctx context.Context, project *types.Project, lis names = append(names, getContainerNameWithoutProject(c)) } - _, _ = fmt.Fprintf(s.stdout(), "Attaching to %s\n", strings.Join(names, ", ")) + _, err = fmt.Fprintf(s.stdout(), "Attaching to %s\n", strings.Join(names, ", ")) + if err != nil { + logrus.Debugf("failed to write attach message: %v", err) + return nil, err + } for _, ctr := range containers { err := s.attachContainer(ctx, ctr, listener) @@ -57,7 +62,7 @@ func (s *composeService) attach(ctx context.Context, project *types.Project, lis return nil, err } } - return containers, err + return containers, nil } func (s *composeService) attachContainer(ctx context.Context, container containerType.Summary, listener api.ContainerEventListener) error { @@ -91,10 +96,21 @@ func (s *composeService) doAttachContainer(ctx context.Context, service, id, nam }) }) - _, _, err = s.attachContainerStreams(ctx, id, inspect.Config.Tty, nil, wOut, wErr) - return err + restore, detached, err := s.attachContainerStreams(ctx, id, inspect.Config.Tty, nil, wOut, wErr) + if err != nil { + return err + } + defer restore() + + go func() { + <-detached + logrus.Debugf("detached from container %s", name) + }() + + return nil } +//nolint:gocyclo func (s *composeService) attachContainerStreams(ctx context.Context, container string, tty bool, stdin io.ReadCloser, stdout, stderr io.WriteCloser) (func(), chan bool, error) { detached := make(chan bool) restore := func() { /* noop */ } @@ -106,7 +122,10 @@ func (s *composeService) attachContainerStreams(ctx context.Context, container s return restore, detached, err } restore = func() { - term.RestoreTerminal(in.FD(), state) //nolint:errcheck + err := term.RestoreTerminal(in.FD(), state) + if err != nil { + logrus.Warnf("failed to restore terminal: %v", err) + } } } } @@ -119,7 +138,10 @@ func (s *composeService) attachContainerStreams(ctx context.Context, container s go func() { <-ctx.Done() if stdin != nil { - stdin.Close() //nolint:errcheck + err := stdin.Close() + if err != nil { + logrus.Debugf("failed to close stdin: %v", err) + } } }() @@ -129,19 +151,34 @@ func (s *composeService) attachContainerStreams(ctx context.Context, container s var escapeErr term.EscapeError if errors.As(err, &escapeErr) { close(detached) + } else if err != nil && !errors.Is(err, io.EOF) { + logrus.Debugf("stdin copy error for container %s: %v", container, err) } }() } if stdout != nil { go func() { - defer stdout.Close() //nolint:errcheck - defer stderr.Close() //nolint:errcheck - defer streamOut.Close() //nolint:errcheck + defer func() { + if err := stdout.Close(); err != nil { + logrus.Debugf("failed to close stdout: %v", err) + } + if err := stderr.Close(); err != nil { + logrus.Debugf("failed to close stderr: %v", err) + } + if err := streamOut.Close(); err != nil { + logrus.Debugf("failed to close stream output: %v", err) + } + }() + + var err error if tty { - io.Copy(stdout, streamOut) //nolint:errcheck + _, err = io.Copy(stdout, streamOut) } else { - stdcopy.StdCopy(stdout, stderr, streamOut) //nolint:errcheck + _, err = stdcopy.StdCopy(stdout, stderr, streamOut) + } + if err != nil && !errors.Is(err, io.EOF) { + logrus.Debugf("stream copy error for container %s: %v", container, err) } }() } @@ -149,8 +186,6 @@ func (s *composeService) attachContainerStreams(ctx context.Context, container s } func (s *composeService) getContainerStreams(ctx context.Context, container string) (io.WriteCloser, io.ReadCloser, error) { - var stdout io.ReadCloser - var stdin io.WriteCloser cnx, err := s.apiClient().ContainerAttach(ctx, container, containerType.AttachOptions{ Stream: true, Stdin: true, @@ -160,8 +195,8 @@ func (s *composeService) getContainerStreams(ctx context.Context, container stri DetachKeys: s.configFile().DetachKeys, }) if err == nil { - stdout = ContainerStdout{HijackedResponse: cnx} - stdin = ContainerStdin{HijackedResponse: cnx} + stdout := ContainerStdout{HijackedResponse: cnx} + stdin := ContainerStdin{HijackedResponse: cnx} return stdin, stdout, nil } @@ -174,5 +209,5 @@ func (s *composeService) getContainerStreams(ctx context.Context, container stri if err != nil { return nil, nil, err } - return stdin, logs, nil + return nil, logs, nil } From 5825bcb292b9e695213e50abd8e37efeaa924a67 Mon Sep 17 00:00:00 2001 From: "hiroto.toyoda" Date: Thu, 1 Jan 2026 01:22:59 +0900 Subject: [PATCH 2/4] refactor(attach): remove unused detach watcher and keep attach behavior Signed-off-by: hiroto.toyoda --- pkg/compose/attach.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pkg/compose/attach.go b/pkg/compose/attach.go index 639e0ea53f..7e7460e258 100644 --- a/pkg/compose/attach.go +++ b/pkg/compose/attach.go @@ -53,7 +53,6 @@ func (s *composeService) attach(ctx context.Context, project *types.Project, lis _, err = fmt.Fprintf(s.stdout(), "Attaching to %s\n", strings.Join(names, ", ")) if err != nil { logrus.Debugf("failed to write attach message: %v", err) - return nil, err } for _, ctr := range containers { @@ -96,17 +95,12 @@ func (s *composeService) doAttachContainer(ctx context.Context, service, id, nam }) }) - restore, detached, err := s.attachContainerStreams(ctx, id, inspect.Config.Tty, nil, wOut, wErr) + restore, _, err := s.attachContainerStreams(ctx, id, inspect.Config.Tty, nil, wOut, wErr) if err != nil { return err } defer restore() - go func() { - <-detached - logrus.Debugf("detached from container %s", name) - }() - return nil } From aad4071032a0cc1e255a34979d1ccf001e264c95 Mon Sep 17 00:00:00 2001 From: "hiroto.toyoda" Date: Mon, 5 Jan 2026 23:59:19 +0900 Subject: [PATCH 3/4] refactor(attach): simplify attachContainerStreams signature Signed-off-by: hiroto.toyoda --- pkg/compose/attach.go | 54 ++++--------------------------------------- 1 file changed, 5 insertions(+), 49 deletions(-) diff --git a/pkg/compose/attach.go b/pkg/compose/attach.go index 7e7460e258..41b666bb43 100644 --- a/pkg/compose/attach.go +++ b/pkg/compose/attach.go @@ -24,10 +24,8 @@ import ( "strings" "github.com/compose-spec/compose-go/v2/types" - "github.com/docker/cli/cli/streams" containerType "github.com/docker/docker/api/types/container" "github.com/docker/docker/pkg/stdcopy" - "github.com/moby/term" "github.com/sirupsen/logrus" "github.com/docker/compose/v5/pkg/api" @@ -95,60 +93,18 @@ func (s *composeService) doAttachContainer(ctx context.Context, service, id, nam }) }) - restore, _, err := s.attachContainerStreams(ctx, id, inspect.Config.Tty, nil, wOut, wErr) + err = s.attachContainerStreams(ctx, id, inspect.Config.Tty, wOut, wErr) if err != nil { return err } - defer restore() return nil } -//nolint:gocyclo -func (s *composeService) attachContainerStreams(ctx context.Context, container string, tty bool, stdin io.ReadCloser, stdout, stderr io.WriteCloser) (func(), chan bool, error) { - detached := make(chan bool) - restore := func() { /* noop */ } - if stdin != nil { - in := streams.NewIn(stdin) - if in.IsTerminal() { - state, err := term.SetRawTerminal(in.FD()) - if err != nil { - return restore, detached, err - } - restore = func() { - err := term.RestoreTerminal(in.FD(), state) - if err != nil { - logrus.Warnf("failed to restore terminal: %v", err) - } - } - } - } - - streamIn, streamOut, err := s.getContainerStreams(ctx, container) +func (s *composeService) attachContainerStreams(ctx context.Context, container string, tty bool, stdout, stderr io.WriteCloser) error { + _, streamOut, err := s.getContainerStreams(ctx, container) if err != nil { - return restore, detached, err - } - - go func() { - <-ctx.Done() - if stdin != nil { - err := stdin.Close() - if err != nil { - logrus.Debugf("failed to close stdin: %v", err) - } - } - }() - - if streamIn != nil && stdin != nil { - go func() { - _, err := io.Copy(streamIn, stdin) - var escapeErr term.EscapeError - if errors.As(err, &escapeErr) { - close(detached) - } else if err != nil && !errors.Is(err, io.EOF) { - logrus.Debugf("stdin copy error for container %s: %v", container, err) - } - }() + return err } if stdout != nil { @@ -176,7 +132,7 @@ func (s *composeService) attachContainerStreams(ctx context.Context, container s } }() } - return restore, detached, nil + return nil } func (s *composeService) getContainerStreams(ctx context.Context, container string) (io.WriteCloser, io.ReadCloser, error) { From 1036fbbed6bf0d07364acb411590a058bc92f1d9 Mon Sep 17 00:00:00 2001 From: "hiroto.toyoda" Date: Tue, 6 Jan 2026 21:05:08 +0900 Subject: [PATCH 4/4] refactor(attach): remove unused stdin from getContainerStream Signed-off-by: hiroto.toyoda --- pkg/compose/attach.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pkg/compose/attach.go b/pkg/compose/attach.go index 41b666bb43..2ab07b1e99 100644 --- a/pkg/compose/attach.go +++ b/pkg/compose/attach.go @@ -102,7 +102,7 @@ func (s *composeService) doAttachContainer(ctx context.Context, service, id, nam } func (s *composeService) attachContainerStreams(ctx context.Context, container string, tty bool, stdout, stderr io.WriteCloser) error { - _, streamOut, err := s.getContainerStreams(ctx, container) + streamOut, err := s.getContainerStreams(ctx, container) if err != nil { return err } @@ -135,19 +135,17 @@ func (s *composeService) attachContainerStreams(ctx context.Context, container s return nil } -func (s *composeService) getContainerStreams(ctx context.Context, container string) (io.WriteCloser, io.ReadCloser, error) { +func (s *composeService) getContainerStreams(ctx context.Context, container string) (io.ReadCloser, error) { cnx, err := s.apiClient().ContainerAttach(ctx, container, containerType.AttachOptions{ - Stream: true, - Stdin: true, - Stdout: true, - Stderr: true, - Logs: false, - DetachKeys: s.configFile().DetachKeys, + Stream: true, + Stdin: false, + Stdout: true, + Stderr: true, + Logs: false, }) if err == nil { stdout := ContainerStdout{HijackedResponse: cnx} - stdin := ContainerStdin{HijackedResponse: cnx} - return stdin, stdout, nil + return stdout, nil } // Fallback to logs API @@ -157,7 +155,7 @@ func (s *composeService) getContainerStreams(ctx context.Context, container stri Follow: true, }) if err != nil { - return nil, nil, err + return nil, err } - return nil, logs, nil + return logs, nil }