From 65e416f662f3fdd404e6e9357f60b89faa6e8e91 Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kylem@metrumrg.com>
Date: Mon, 7 Oct 2024 16:33:18 -0400
Subject: [PATCH] runModelCommand: write output file even on failure

The new runModelCommand was extracted from executeLocalJob and
executeSGEJob and behaves as they did in terms of handling the output.
The output is collected with CombinedOutput.  If there's an error, the
output is logged before returning a ConcurrentError.  The output is
written to _disk_ only if there was not an error.

Capturing the output on disk is helpful for troubleshooting.  Rework
runModelCommand to send standard output and standard error directly to
a file.
---
 cmd/run.go      | 56 ++++++++++++++++++++++++++++++++++---------------
 cmd/run_test.go | 10 +++++++++
 2 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/cmd/run.go b/cmd/run.go
index 211e8a13..887a306a 100644
--- a/cmd/run.go
+++ b/cmd/run.go
@@ -15,7 +15,6 @@ import (
 
 	"github.com/metrumresearchgroup/turnstile"
 	log "github.com/sirupsen/logrus"
-	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 )
@@ -339,7 +338,7 @@ func onlyBbiVariables(provided []string) []string {
 
 // runModelCommand runs command, writing the combined standard output and
 // standard error to {model.OutputDir}/{model.Model}.out.  command should be
-// primed to run, but Stdout and Stderr must not be set.
+// primed to run; existing values for Stdout and Stderr are ignored.
 //
 // ignoreError is a function called if running the command fails.  It takes the
 // error and combined standard output and standard error as arguments.  A return
@@ -349,29 +348,52 @@ func runModelCommand(
 	model *NonMemModel, command *exec.Cmd, ignoreError func(error, string) bool,
 ) turnstile.ConcurrentError {
 
-	output, err := command.CombinedOutput()
-	if err != nil && !ignoreError(err, string(output)) {
-		log.Debug(err)
-
-		var exitError *exec.ExitError
-		if errors.As(err, &exitError) {
-			code := exitError.ExitCode()
-			log.Errorf("%s exit code: %d, output:\n%s", model.LogIdentifier(), code, string(output))
+	outfile := filepath.Join(model.OutputDir, model.Model+".out")
+	outfh, err := os.OpenFile(outfile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o640)
+	if err != nil {
+		if errClose := outfh.Close(); errClose != nil {
+			log.Errorf("error closing output file: %v", errClose)
 		}
 
 		return turnstile.ConcurrentError{
 			RunIdentifier: model.Model,
-			Notes:         fmt.Sprintf("error running %q", command.String()),
+			Notes:         "unable to create model output file",
 			Error:         err,
 		}
 	}
 
-	fs := afero.NewOsFs()
-	if err = afero.WriteFile(fs, filepath.Join(model.OutputDir, model.Model+".out"), output, 0640); err != nil {
-		return turnstile.ConcurrentError{
-			RunIdentifier: model.Model,
-			Notes:         "failed to write model output",
-			Error:         err,
+	command.Stdout = outfh
+	command.Stderr = outfh
+
+	if err = command.Run(); err != nil {
+		if errClose := outfh.Close(); errClose != nil {
+			log.Errorf("error closing output file: %v", errClose)
+		}
+
+		ignore := false
+		output, errRead := os.ReadFile(outfile)
+		if errRead == nil {
+			ignore = ignoreError(err, string(output))
+		} else {
+			log.Errorf("error reading output file: %v", errRead)
+		}
+
+		if !ignore {
+			log.Debug(err)
+
+			var exitError *exec.ExitError
+			if errors.As(err, &exitError) {
+				code := exitError.ExitCode()
+				log.Errorf("%s exit code: %d, output:\n%s",
+					model.LogIdentifier(), code, string(output))
+			}
+
+			return turnstile.ConcurrentError{
+				RunIdentifier: model.Model,
+				Notes: fmt.Sprintf("error running %q; command output written to %q",
+					command.String(), outfile),
+				Error: err,
+			}
 		}
 	}
 
diff --git a/cmd/run_test.go b/cmd/run_test.go
index 1ce4b720..34832703 100644
--- a/cmd/run_test.go
+++ b/cmd/run_test.go
@@ -59,6 +59,16 @@ func TestRunModelCommandError(tt *testing.T) {
 
 	cerr := runModelCommand(mod, cmd, never)
 	t.A.Error(cerr.Error)
+	t.A.Contains(cerr.Notes, mod.Model+".out")
+
+	outfile := filepath.Join(mod.OutputDir, mod.Model+".out")
+	t.R.FileExists(outfile)
+	bs, err := os.ReadFile(outfile)
+	t.R.NoError(err)
+	output = string(bs)
+
+	t.A.Contains(output, "stdout")
+	t.A.Contains(output, "stderr")
 }
 
 func TestRunModelCommandIgnoreError(tt *testing.T) {