Merge pull request #5143 from vvoland/cli-err-wrapstream

cli: Wrap Err stream

cli/command/cli.go

@@ -44,7 +44,7 @@ const defaultInitTimeout = 2 * time.Second
 type Streams interface {
 	In() *streams.In
 	Out() *streams.Out
-	Err() io.Writer
+	Err() *streams.Out
 }
 
 // Cli represents the docker command line client.
@@ -75,7 +75,7 @@ type DockerCli struct {
 	options            *cliflags.ClientOptions
 	in                 *streams.In
 	out                *streams.Out
-	err                io.Writer
+	err                *streams.Out
 	client             client.APIClient
 	serverInfo         ServerInfo
 	contentTrust       bool
@@ -124,7 +124,7 @@ func (cli *DockerCli) Out() *streams.Out {
 }
 
 // Err returns the writer used for stderr
-func (cli *DockerCli) Err() io.Writer {
+func (cli *DockerCli) Err() *streams.Out {
 	return cli.err
 }
 

cli/command/cli_options.go

@@ -23,7 +23,7 @@ func WithStandardStreams() CLIOption {
 		stdin, stdout, stderr := term.StdStreams()
 		cli.in = streams.NewIn(stdin)
 		cli.out = streams.NewOut(stdout)
-		cli.err = stderr
+		cli.err = streams.NewOut(stderr)
 		return nil
 	}
 }
@@ -40,8 +40,9 @@ func WithBaseContext(ctx context.Context) CLIOption {
 // WithCombinedStreams uses the same stream for the output and error streams.
 func WithCombinedStreams(combined io.Writer) CLIOption {
 	return func(cli *DockerCli) error {
-		cli.out = streams.NewOut(combined)
-		cli.err = combined
+		s := streams.NewOut(combined)
+		cli.out = s
+		cli.err = s
 		return nil
 	}
 }
@@ -65,7 +66,7 @@ func WithOutputStream(out io.Writer) CLIOption {
 // WithErrorStream sets a cli error stream.
 func WithErrorStream(err io.Writer) CLIOption {
 	return func(cli *DockerCli) error {
-		cli.err = err
+		cli.err = streams.NewOut(err)
 		return nil
 	}
 }

cli/command/cli_test.go

@@ -18,6 +18,7 @@ import (
 	"github.com/docker/cli/cli/config"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/flags"
+	"github.com/docker/cli/cli/streams"
 	"github.com/docker/docker/api"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
@@ -253,7 +254,7 @@ func TestExperimentalCLI(t *testing.T) {
 				},
 			}
 
-			cli := &DockerCli{client: apiclient, err: os.Stderr}
+			cli := &DockerCli{client: apiclient, err: streams.NewOut(os.Stderr)}
 			config.SetDir(dir.Path())
 			err := cli.Initialize(flags.NewClientOptions())
 			assert.NilError(t, err)

cli/command/container/create.go

@@ -130,9 +130,9 @@ func pullImage(ctx context.Context, dockerCli command.Cli, img string, options *
 
 	out := dockerCli.Err()
 	if options.quiet {
-		out = io.Discard
+		out = streams.NewOut(io.Discard)
 	}
-	return jsonmessage.DisplayJSONMessagesToStream(responseBody, streams.NewOut(out), nil)
+	return jsonmessage.DisplayJSONMessagesToStream(responseBody, out, nil)
 }
 
 type cidFile struct {

cli/command/image/push.go

@@ -18,7 +18,6 @@ import (
 	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/registry"
-	"github.com/moby/term"
 	"github.com/morikuni/aec"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
@@ -176,7 +175,7 @@ func handleAux(dockerCli command.Cli) func(jm jsonmessage.JSONMessage) {
 }
 
 func printNote(dockerCli command.Cli, format string, args ...any) {
-	if _, isTTY := term.GetFdInfo(dockerCli.Err()); isTTY {
+	if dockerCli.Err().IsTerminal() {
 		_, _ = fmt.Fprint(dockerCli.Err(), aec.WhiteF.Apply(aec.CyanB.Apply("[ NOTE ]"))+" ")
 	} else {
 		_, _ = fmt.Fprint(dockerCli.Err(), "[ NOTE ] ")

cli/command/registry/login_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	configtypes "github.com/docker/cli/cli/config/types"
+	"github.com/docker/cli/cli/streams"
 	"github.com/docker/cli/internal/test"
 	registrytypes "github.com/docker/docker/api/types/registry"
 	"github.com/docker/docker/api/types/system"
@@ -69,7 +70,7 @@ func TestLoginWithCredStoreCreds(t *testing.T) {
 	for _, tc := range testCases {
 		cli := test.NewFakeCli(&fakeClient{})
 		errBuf := new(bytes.Buffer)
-		cli.SetErr(errBuf)
+		cli.SetErr(streams.NewOut(errBuf))
 		loginWithCredStoreCreds(ctx, cli, &tc.inputAuthConfig)
 		outputString := cli.OutBuffer().String()
 		assert.Check(t, is.Equal(tc.expectedMsg, outputString))

cli/command/telemetry_utils.go

@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/docker/cli/cli/version"
-	"github.com/moby/term"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"go.opentelemetry.io/otel/attribute"
@@ -101,12 +100,10 @@ func startCobraCommandTimer(mp metric.MeterProvider, attrs []attribute.KeyValue)
 }
 
 func stdioAttributes(streams Streams) []attribute.KeyValue {
-	// we don't wrap stderr, but we do wrap in/out
-	_, stderrTty := term.GetFdInfo(streams.Err())
 	return []attribute.KeyValue{
 		attribute.Bool("command.stdin.isatty", streams.In().IsTerminal()),
 		attribute.Bool("command.stdout.isatty", streams.Out().IsTerminal()),
-		attribute.Bool("command.stderr.isatty", stderrTty),
+		attribute.Bool("command.stderr.isatty", streams.Err().IsTerminal()),
 	}
 }
 

cli/command/telemetry_utils_test.go

@@ -136,7 +136,7 @@ func TestStdioAttributes(t *testing.T) {
 			cli := &DockerCli{
 				in:  streams.NewIn(io.NopCloser(strings.NewReader(""))),
 				out: streams.NewOut(outBuffer),
-				err: errBuffer,
+				err: streams.NewOut(errBuffer),
 			}
 			cli.In().SetIsTerminal(tc.stdinTty)
 			cli.Out().SetIsTerminal(tc.stdoutTty)

internal/test/cli.go

@@ -28,7 +28,8 @@ type FakeCli struct {
 	configfile       *configfile.ConfigFile
 	out              *streams.Out
 	outBuffer        *bytes.Buffer
-	err              *bytes.Buffer
+	err              *streams.Out
+	errBuffer        *bytes.Buffer
 	in               *streams.In
 	server           command.ServerInfo
 	notaryClientFunc NotaryClientFuncType
@@ -48,7 +49,8 @@ func NewFakeCli(apiClient client.APIClient, opts ...func(*FakeCli)) *FakeCli {
 		client:    apiClient,
 		out:       streams.NewOut(outBuffer),
 		outBuffer: outBuffer,
-		err:       errBuffer,
+		err:       streams.NewOut(errBuffer),
+		errBuffer: errBuffer,
 		in:        streams.NewIn(io.NopCloser(strings.NewReader(""))),
 		// Use an empty string for filename so that tests don't create configfiles
 		// Set cli.ConfigFile().Filename to a tempfile to support Save.
@@ -67,7 +69,7 @@ func (c *FakeCli) SetIn(in *streams.In) {
 }
 
 // SetErr sets the stderr stream for the cli to the specified io.Writer
-func (c *FakeCli) SetErr(err *bytes.Buffer) {
+func (c *FakeCli) SetErr(err *streams.Out) {
 	c.err = err
 }
 
@@ -112,7 +114,7 @@ func (c *FakeCli) Out() *streams.Out {
 }
 
 // Err returns the output stream (stderr) the cli should write on
-func (c *FakeCli) Err() io.Writer {
+func (c *FakeCli) Err() *streams.Out {
 	return c.err
 }
 
@@ -153,13 +155,13 @@ func (c *FakeCli) OutBuffer() *bytes.Buffer {
 
 // ErrBuffer Buffer returns the stderr buffer
 func (c *FakeCli) ErrBuffer() *bytes.Buffer {
-	return c.err
+	return c.errBuffer
 }
 
 // ResetOutputBuffers resets the .OutBuffer() and.ErrBuffer() back to empty
 func (c *FakeCli) ResetOutputBuffers() {
 	c.outBuffer.Reset()
-	c.err.Reset()
+	c.errBuffer.Reset()
 }
 
 // SetNotaryClient sets the internal getter for retrieving a NotaryClient