Builder diagnostics go to stderr; stdout is the result alone
The logging added a line to stdout, and the genesis path parses the builder's stdout as JSON — so the first log line broke the parse with "invalid character 'c'", the c from "[clone]". A build that had worked stopped working because of a print statement. The installer's runner captures stdout alone (cmd.Output), and the contract was already stdout=result, stderr=everything else. The fix is to honour it: every builder diagnostic — the step log, the per-command echo, the module path's own lines — goes to stderr. Stdout carries only once.go's result JSON. And a unit test now fails if any fmt.Print to stdout appears in the two builder command files, except the three that belong there: the result, --version, and --help. A guard, because this was invisible until a 20-minute run hit it, and the same class of mistake should fail in milliseconds next time. Claude-Session: https://claude.ai/code/session_01D6qtiYU3P9jk3pnAXyAFyx
This commit is contained in:
@@ -139,7 +139,7 @@ func run() error {
|
||||
return err
|
||||
}
|
||||
|
||||
fmt.Printf("building for the mesh, publishing to %s\n", registry)
|
||||
fmt.Fprintf(os.Stderr, "building for the mesh, publishing to %s\n", registry)
|
||||
publisher := builder.Registry{Address: registry, Run: builder.Command}
|
||||
|
||||
for {
|
||||
@@ -164,14 +164,14 @@ func answer(ctx context.Context, channel *amqp.Channel, publisher builder.Publis
|
||||
// until it either finishes or fails is indistinguishable from one that never arrived — which
|
||||
// cost a long diagnosis against a running mesh, chasing "the handler never fired" when the
|
||||
// truth was only that the handler said nothing until the end.
|
||||
fmt.Printf("a build request arrived (%d bytes)\n", len(delivery.Body))
|
||||
fmt.Fprintf(os.Stderr, "a build request arrived (%d bytes)\n", len(delivery.Body))
|
||||
|
||||
var request link.BuildRequest
|
||||
if err := json.Unmarshal(delivery.Body, &request); err != nil {
|
||||
// Unreadable. Acknowledged and dropped rather than requeued: a message this builder
|
||||
// cannot parse will not become parseable by being delivered again, and requeueing it
|
||||
// would put it in front of every real request for ever.
|
||||
fmt.Printf("a request could not be read and was dropped: %v\n", err)
|
||||
fmt.Fprintf(os.Stderr, "a request could not be read and was dropped: %v\n", err)
|
||||
_ = delivery.Ack(false)
|
||||
return
|
||||
}
|
||||
@@ -180,19 +180,19 @@ func answer(ctx context.Context, channel *amqp.Channel, publisher builder.Publis
|
||||
ID: request.ID, Repository: request.Repository, Path: request.Path,
|
||||
Ref: request.Ref, On: on,
|
||||
}
|
||||
fmt.Printf("building %s", request.Repository)
|
||||
fmt.Fprintf(os.Stderr, "building %s", request.Repository)
|
||||
if request.Path != "" {
|
||||
fmt.Printf(" at %s", request.Path)
|
||||
fmt.Fprintf(os.Stderr, " at %s", request.Path)
|
||||
}
|
||||
if request.Ref != "" {
|
||||
fmt.Printf(" at %s", request.Ref)
|
||||
fmt.Fprintf(os.Stderr, " at %s", request.Ref)
|
||||
}
|
||||
fmt.Println()
|
||||
fmt.Fprintln(os.Stderr)
|
||||
|
||||
built, err := builder.Build(ctx, builder.Command, publisher,
|
||||
request.Repository, request.Path, request.Ref, workspace, request.Held,
|
||||
func(step, message string) {
|
||||
fmt.Printf(" [%s] %s\n", step, message)
|
||||
fmt.Fprintf(os.Stderr, " [%s] %s\n", step, message)
|
||||
})
|
||||
if err != nil {
|
||||
// A failure is a result. A build that fails and says nothing is indistinguishable from a
|
||||
@@ -212,7 +212,7 @@ func answer(ctx context.Context, channel *amqp.Channel, publisher builder.Publis
|
||||
})
|
||||
}
|
||||
result.Against = built.Against
|
||||
fmt.Printf(" built %s from %s\n", built.Manifest.Module, short(built.Commit))
|
||||
fmt.Fprintf(os.Stderr, " built %s from %s\n", built.Manifest.Module, short(built.Commit))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -85,7 +85,7 @@ func buildOnce(ctx context.Context, args []string) error {
|
||||
fmt.Fprintln(os.Stderr)
|
||||
|
||||
built, buildErr := builder.Build(ctx, builder.Command, publisher, repository, *path, *ref, where, bases,
|
||||
func(step, message string) { fmt.Printf(" [%s] %s\n", step, message) })
|
||||
func(step, message string) { fmt.Fprintf(os.Stderr, " [%s] %s\n", step, message) })
|
||||
if buildErr != nil {
|
||||
return buildErr
|
||||
}
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// **The genesis path writes its result to stdout and nothing else there.** The installer captures
|
||||
// stdout alone and parses it as JSON; one stray log line makes that fail with "invalid character"
|
||||
// — exactly what a builder fmt.Printf caused, breaking a build that had worked. This keeps
|
||||
// diagnostics on stderr so a future print cannot repeat it silently.
|
||||
func TestBuilderDiagnosticsStayOffStdout(t *testing.T) {
|
||||
allowed := map[string]bool{
|
||||
"string(body)": true, // once.go: the result JSON, which IS stdout
|
||||
"version)": true, // --version
|
||||
`"stopping")`: true, // the loop.s shutdown line
|
||||
"usage)": true, // --help text, for a human
|
||||
}
|
||||
for _, file := range []string{"once.go", "main.go"} {
|
||||
src, err := os.ReadFile(file)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for i, raw := range strings.Split(string(src), "\n") {
|
||||
line := strings.TrimSpace(raw)
|
||||
if !strings.HasPrefix(line, "fmt.Printf(") &&
|
||||
!strings.HasPrefix(line, "fmt.Println(") &&
|
||||
!strings.HasPrefix(line, "fmt.Print(") {
|
||||
continue
|
||||
}
|
||||
ok := false
|
||||
for token := range allowed {
|
||||
if strings.Contains(line, token) {
|
||||
ok = true
|
||||
}
|
||||
}
|
||||
if !ok {
|
||||
t.Errorf("%s:%d writes a diagnostic to stdout, which the installer parses as JSON:\n %s",
|
||||
file, i+1, line)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user