diff --git a/src/pkg/cli/compose/loader.go b/src/pkg/cli/compose/loader.go index 3102d3d94..312e5f9c8 100644 --- a/src/pkg/cli/compose/loader.go +++ b/src/pkg/cli/compose/loader.go @@ -175,7 +175,7 @@ func (l *Loader) loadProject(ctx context.Context, suppressWarn bool) (*Project, if term.DoDebug() { b, _ := yaml.Marshal(project) - term.Println(string(b)) + term.Println(string(b)) // term.Println routes to stderr in JSON mode, so this never corrupts --json stdout } l.cached = project diff --git a/src/pkg/cli/compose/loader_test.go b/src/pkg/cli/compose/loader_test.go index 83b18342b..d682878c5 100644 --- a/src/pkg/cli/compose/loader_test.go +++ b/src/pkg/cli/compose/loader_test.go @@ -38,6 +38,44 @@ func TestResolveProjectWorkingDirDoesNotLoadOrCacheProject(t *testing.T) { assert.Equal(t, dir, workingDir) } +// TestLoadProjectDebugDumpNeverHitsStdoutInJSONMode is a regression test: +// `defang services --json` with DEFANG_DEBUG=1 used to write the project's +// YAML dump to stdout ahead of the JSON payload, corrupting it for callers +// like `defang-github-action`'s deployment summary (jq: invalid numeric +// literal). term.Println now routes to stderr in JSON mode, so the dump +// must never appear on stdout, whether or not JSON mode is on. +func TestLoadProjectDebugDumpNeverHitsStdoutInJSONMode(t *testing.T) { + dir := t.TempDir() + composePath := filepath.Join(dir, "compose.yaml") + require.NoError(t, os.WriteFile(composePath, []byte("services:\n web:\n image: alpine\n"), 0o644)) + + oldTerm := term.DefaultTerm + t.Cleanup(func() { term.DefaultTerm = oldTerm }) + + t.Run("debug alone dumps the project to stdout", func(t *testing.T) { + var stdout, stderr bytes.Buffer + term.DefaultTerm = term.NewTerm(os.Stdin, &stdout, &stderr) + term.DefaultTerm.SetDebug(true) + + _, err := NewLoader(WithPath(composePath)).LoadProject(t.Context()) + require.NoError(t, err) + assert.Contains(t, stdout.String(), "services:") + assert.Empty(t, stderr.String()) + }) + + t.Run("debug plus json moves the dump to stderr", func(t *testing.T) { + var stdout, stderr bytes.Buffer + term.DefaultTerm = term.NewTerm(os.Stdin, &stdout, &stderr) + term.DefaultTerm.SetDebug(true) + term.DefaultTerm.SetJSON(true) + + _, err := NewLoader(WithPath(composePath)).LoadProject(t.Context()) + require.NoError(t, err) + assert.Empty(t, stdout.String()) + assert.Contains(t, stderr.String(), "services:") + }) +} + func TestResolveProjectWorkingDirDoesNotSuppressProjectWarnings(t *testing.T) { dir := t.TempDir() composePath := filepath.Join(dir, "compose.yaml") diff --git a/src/pkg/term/colorizer.go b/src/pkg/term/colorizer.go index b3bea0775..90ee4c965 100644 --- a/src/pkg/term/colorizer.go +++ b/src/pkg/term/colorizer.go @@ -195,20 +195,24 @@ func ensurePrefix(prefix prefixChars, s string) string { return string(prefix) + s } +// Printc, Print, Println, and Printf are for human-readable text; they write +// to stderr instead of stdout when JSON mode is on, so they never corrupt a +// command's --json payload. The only thing that belongs on stdout in JSON +// mode is the JSON payload itself (see jsonTable in table.go). func (t *Term) Printc(c Color, v ...any) (int, error) { - return output(t.out, c, fmt.Sprint(v...)) + return output(t.outOrErr(), c, fmt.Sprint(v...)) } func (t *Term) Print(v ...any) (int, error) { - return fmt.Fprint(t.out, v...) + return fmt.Fprint(t.outOrErr(), v...) } func (t *Term) Println(v ...any) (int, error) { - return fmt.Fprintln(t.out, v...) + return fmt.Fprintln(t.outOrErr(), v...) } func (t *Term) Printf(format string, v ...any) (int, error) { - return fmt.Fprintf(t.out, format, v...) + return fmt.Fprintf(t.outOrErr(), format, v...) } func (t *Term) Debug(v ...any) (int, error) { diff --git a/src/pkg/term/colorizer_test.go b/src/pkg/term/colorizer_test.go index 295adfca6..36d5caad0 100644 --- a/src/pkg/term/colorizer_test.go +++ b/src/pkg/term/colorizer_test.go @@ -162,6 +162,35 @@ func TestIsTerminal(t *testing.T) { t.Error("Expected IsTerminal() to return false") } } + +// TestPrintRoutingInJSONMode is a regression test: Print/Println/Printf/Printc +// used to always write to stdout, so any of them reachable from a command +// that also emits --json output (e.g. via a debug dump) would corrupt that +// JSON payload. They must move to stderr in JSON mode, like Info/Warn do. +func TestPrintRoutingInJSONMode(t *testing.T) { + var stdout, stderr bytes.Buffer + defaultTerm := NewTerm(os.Stdin, &stdout, &stderr) + + defaultTerm.Print("a") + defaultTerm.Println("b") + defaultTerm.Printf("%s", "c") + defaultTerm.Printc(InfoColor, "d") + if stdout.String() == "" || stderr.String() != "" { + t.Errorf("expected Print* to write to stdout when JSON mode is off; stdout=%q stderr=%q", stdout.String(), stderr.String()) + } + + stdout.Reset() + stderr.Reset() + defaultTerm.SetJSON(true) + + defaultTerm.Print("a") + defaultTerm.Println("b") + defaultTerm.Printf("%s", "c") + defaultTerm.Printc(InfoColor, "d") + if stdout.String() != "" || stderr.String() == "" { + t.Errorf("expected Print* to write to stderr when JSON mode is on; stdout=%q stderr=%q", stdout.String(), stderr.String()) + } +} func TestWarn(t *testing.T) { tests := []struct { msgs []string