From 3e7ebe81c4401baab0fbed2879e02d719497b178 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Wed, 5 Feb 2025 11:10:52 +0100
Subject: [PATCH] progress: fix missing build log output on errors in progress

This commit fixes a silly mistake from PR#810. The issue is that
in #810 we started to collect the osbuild stdout/stderr so that
we can show crashes from osbuild or other unexpected output.

However when a stage fails this is reported via the json progress
and not directly on stdout/stderr - this was missed when #810
was done.

This commit does a short term fix by collecting the buildlog again
and showing it in the error and also updates the test to be more
realistic. However we really need a test that actually tests
the real behavior, ideally a real osbuild run with a stage error
so that we can be sure we capture this (criticial!) functionality.
---
 bib/pkg/progress/progress.go      | 11 ++++++++++-
 bib/pkg/progress/progress_test.go | 11 ++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/bib/pkg/progress/progress.go b/bib/pkg/progress/progress.go
index 44f7da7..c7c79bb 100644
--- a/bib/pkg/progress/progress.go
+++ b/bib/pkg/progress/progress.go
@@ -368,6 +368,7 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 	}
 	wp.Close()
 
+	var tracesMsgs []string
 	var statusErrs []error
 	for {
 		st, err := osbuildStatus.Status()
@@ -389,10 +390,18 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 		if st.Message != "" {
 			pb.SetMessagef(st.Message)
 		}
+
+		// keep all messages/traces for better error reporting
+		if st.Message != "" {
+			tracesMsgs = append(tracesMsgs, st.Message)
+		}
+		if st.Trace != "" {
+			tracesMsgs = append(tracesMsgs, st.Trace)
+		}
 	}
 
 	if err := cmd.Wait(); err != nil {
-		return fmt.Errorf("error running osbuild: %w\nOutput:\n%s", err, stdio.String())
+		return fmt.Errorf("error running osbuild: %w\nBuildLog:\n%s\nOutput:\n%s", err, strings.Join(tracesMsgs, "\n"), stdio.String())
 	}
 	if len(statusErrs) > 0 {
 		return fmt.Errorf("errors parsing osbuild status:\n%w", errors.Join(statusErrs...))
diff --git a/bib/pkg/progress/progress_test.go b/bib/pkg/progress/progress_test.go
index 8bd2f93..4343c98 100644
--- a/bib/pkg/progress/progress_test.go
+++ b/bib/pkg/progress/progress_test.go
@@ -3,6 +3,7 @@ package progress_test
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -141,7 +142,13 @@ func makeFakeOsbuild(t *testing.T, content string) string {
 }
 
 func TestRunOSBuildWithProgressErrorReporting(t *testing.T) {
-	restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `echo osbuild-stdout-output
+	restore := progress.MockOsStderr(io.Discard)
+	defer restore()
+
+	restore = progress.MockOsbuildCmd(makeFakeOsbuild(t, `
+>&3 echo '{"message": "osbuild-stage-message"}'
+
+echo osbuild-stdout-output
 >&2 echo osbuild-stderr-output
 exit 112
 `))
@@ -151,6 +158,8 @@ exit 112
 	assert.NoError(t, err)
 	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), "", "", nil, nil)
 	assert.EqualError(t, err, `error running osbuild: exit status 112
+BuildLog:
+osbuild-stage-message
 Output:
 osbuild-stdout-output
 osbuild-stderr-output