From 50690574da2a0c9be900be12db7c3244aa342d3d Mon Sep 17 00:00:00 2001 From: Samuel Lampa Date: Thu, 14 Oct 2021 10:50:18 +0200 Subject: [PATCH] Fix #146: Handle all errors --- misc_test.go | 6 ++++-- process.go | 5 ++++- settings_test.go | 3 ++- task.go | 45 +++++++++++++++++++++++++++------------------ task_test.go | 16 ++++++++++------ workflow.go | 5 ++++- workflow_test.go | 21 ++++++++++++--------- 7 files changed, 63 insertions(+), 38 deletions(-) diff --git a/misc_test.go b/misc_test.go index 46800ed..8fd0d05 100644 --- a/misc_test.go +++ b/misc_test.go @@ -26,11 +26,13 @@ func cleanFiles(fileNames ...string) { for _, fileName := range fileNames { auditFileName := fileName + ".audit.json" if _, err := os.Stat(fileName); err == nil { - os.Remove(fileName) + errRem := os.Remove(fileName) + Check(errRem) Debug.Println("Successfully removed file", fileName) } if _, err := os.Stat(auditFileName); err == nil { - os.Remove(auditFileName) + errRem := os.Remove(auditFileName) + Check(errRem) Debug.Println("Successfully removed audit.json file", auditFileName) } } diff --git a/process.go b/process.go index 4f42616..1ac01f0 100644 --- a/process.go +++ b/process.go @@ -346,7 +346,10 @@ func (p *Process) Run() { } // Remove any FIFO file if oip.doStream && oip.FifoFileExists() { - os.Remove(oip.FifoPath()) + err := os.Remove(oip.FifoPath()) + if err != nil { + p.Failf("Could not remove Fifo path %s", oip.FifoPath()) + } } } } diff --git a/settings_test.go b/settings_test.go index 67eac40..8130b7a 100644 --- a/settings_test.go +++ b/settings_test.go @@ -9,7 +9,8 @@ func TestGetBufsize(t *testing.T) { initTestLogs() wantBufSize := 1234 - os.Setenv("SCIPIPE_BUFSIZE", "1234") + err := os.Setenv("SCIPIPE_BUFSIZE", "1234") + Check(err) haveBufSize := getBufsize() if haveBufSize != wantBufSize { diff --git a/task.go b/task.go index 91e4647..7f3c1c5 100644 --- a/task.go +++ b/task.go @@ -67,7 +67,7 @@ func NewTask(workflow *Workflow, process *Process, name string, cmdPat string, i for oname, outPathFunc := range outPathFuncs { oip, err := NewFileIP(outPathFunc(t)) if err != nil { - process.Fail(err.Error()) + process.Failf("Could not create out IP %s: %v", oname, err) } if ptInfo, ok := portInfos[oname]; ok { if ptInfo.doStream { @@ -266,7 +266,7 @@ func (t *Task) Execute() { t.workflow.IncConcurrentTasks(t.cores) // Will block if max concurrent tasks is reached err := t.createDirs() // Create output directories needed for any outputs if err != nil { - t.Fail(err) + t.Failf("Could not create directories: %v", err) } startTime := time.Now() if t.CustomExecute != nil { @@ -325,7 +325,10 @@ func (t *Task) anyOutputsExist() (anyFileExists bool) { // createDirs creates directories for out-IPs of the task func (t *Task) createDirs() error { - os.MkdirAll(t.TempDir(), 0777) + err := os.MkdirAll(t.TempDir(), 0777) + if err != nil { + t.Failf("Could not create tempdir %s: %v", t.TempDir(), err) + } for _, oip := range t.OutIPs { oipDir := oip.TempDir() // This will create all out dirs, including the temp dir @@ -336,7 +339,7 @@ func (t *Task) createDirs() error { } err := os.MkdirAll(oipDir, 0777) if err != nil { - return errors.New(fmt.Sprintf("Could not create directory: %s: %s", oipDir, err)) + return errors.New(fmt.Sprintf("Could not create directory %s: %v", oipDir, err)) } } @@ -426,32 +429,38 @@ func FinalizePaths(tempExecDir string, ips ...*FileIP) error { // Move paths for ports, to final destinations if !oip.doStream { tempPath := tempExecDir + "/" + oip.TempPath() - newPath := oip.Path() - Debug.Println("Moving OutIP path: ", tempPath, " -> ", newPath) - renameErr := os.Rename(tempPath, newPath) + finPath := oip.Path() + Debug.Println("Moving OutIP path: ", tempPath, " -> ", finPath) + renameErr := os.Rename(tempPath, finPath) if renameErr != nil { - return errors.New(fmt.Sprintf("Could not rename out-IP file %s to %s: %s", tempPath, newPath, renameErr)) + return errors.New(fmt.Sprintf("Could not rename out-IP file %s to %s: %s", tempPath, finPath, renameErr)) } } } // For remaining paths in temporary execution dir, just move out of it - filepath.Walk(tempExecDir, func(tempPath string, fileInfo os.FileInfo, err error) error { + err := filepath.Walk(tempExecDir, func(tempPath string, fileInfo os.FileInfo, err error) error { if !fileInfo.IsDir() { - newPath := strings.Replace(tempPath, tempExecDir+"/", "", 1) - newPath = strings.Replace(newPath, FSRootPlaceHolder+"/", "/", 1) - newPath = replacePlaceholdersWithParentDirs(newPath) - newPathDir := filepath.Dir(newPath) - if _, err := os.Stat(newPathDir); os.IsNotExist(err) { - os.MkdirAll(newPathDir, 0777) + finPath := strings.Replace(tempPath, tempExecDir+"/", "", 1) + finPath = strings.Replace(finPath, FSRootPlaceHolder+"/", "/", 1) + finPath = replacePlaceholdersWithParentDirs(finPath) + finPathDir := filepath.Dir(finPath) + if _, err := os.Stat(finPathDir); os.IsNotExist(err) { + errMkdir := os.MkdirAll(finPathDir, 0777) + if errMkdir != nil { + Error.Printf("Failed to create directory for final path: %s\n", finPathDir) + } } - Debug.Println("Moving remaining file path: ", tempPath, " -> ", newPath) - renameErr := os.Rename(tempPath, newPath) + Debug.Println("Moving remaining file path: ", tempPath, " -> ", finPath) + renameErr := os.Rename(tempPath, finPath) if renameErr != nil { - return errors.New(fmt.Sprintf("Could not rename remaining file %s to %s: %s", tempPath, newPath, renameErr)) + return errors.New(fmt.Sprintf("Could not rename remaining file %s to %s: %s", tempPath, finPath, renameErr)) } } return err }) + if err != nil { + Error.Printf("Failed walking temporary execution directory %s: %v\n", tempExecDir, err) + } // Remove temporary execution dir (but not for absolute paths, or current dir) if tempExecDir != "" && tempExecDir != "." && tempExecDir[0] != '/' { remErr := os.RemoveAll(tempExecDir) diff --git a/task_test.go b/task_test.go index 17f285a..03c8374 100644 --- a/task_test.go +++ b/task_test.go @@ -154,13 +154,15 @@ func TestExtraFilesFinalizePaths(t *testing.T) { tsk := NewTask(nil, nil, "test_task", "echo foo", map[string]*FileIP{}, nil, nil, map[string]string{}, nil, "", nil, 4) // Create extra file tmpDir := tsk.TempDir() - os.MkdirAll(tmpDir, 0777) + err := os.MkdirAll(tmpDir, 0777) + Check(err) fName := filepath.Join(tmpDir, "letterfile_a.txt") - _, err := os.Create(fName) - if err != nil { + _, errCreate := os.Create(fName) + if errCreate != nil { t.Fatalf("File could not be created: %s\n", fName) } - tsk.finalizePaths() + errFin := tsk.finalizePaths() + Check(errFin) filePath := filepath.Join(".", "letterfile_a.txt") if _, err := os.Stat(filePath); os.IsNotExist(err) { t.Error("File did not exist: " + filePath) @@ -178,13 +180,15 @@ func TestExtraFilesFinalizePathsAbsolute(t *testing.T) { } absDir := filepath.Join(tmpDir, FSRootPlaceHolder, tmpDir) - os.MkdirAll(absDir, 0777) + errMkdir := os.MkdirAll(absDir, 0777) + Check(errMkdir) fName := filepath.Join(absDir, "letterfile_a.txt") _, err = os.Create(fName) if err != nil { t.Fatalf("File could not be created: %s\n", fName) } - FinalizePaths(tmpDir) + errFin := FinalizePaths(tmpDir) + Check(errFin) filePath := filepath.Join(tmpDir, "letterfile_a.txt") if _, err := os.Stat(filePath); os.IsNotExist(err) { t.Error("File did not exist: " + filePath) diff --git a/workflow.go b/workflow.go index e68e721..ddad30a 100644 --- a/workflow.go +++ b/workflow.go @@ -197,7 +197,10 @@ func (wf *Workflow) PlotGraph(filePath string) { createDirs(filePath) dotFile, err := os.Create(filePath) CheckWithMsg(err, "Could not create dot file "+filePath) - dotFile.WriteString(dot) + _, errDot := dotFile.WriteString(dot) + if errDot != nil { + wf.Failf("Could not write to DOT-file %s: %s", dotFile.Name(), errDot) + } } // PlotGraphPDF writes the workflow structure to a dot file, and also runs the diff --git a/workflow_test.go b/workflow_test.go index ee07ec9..d6211a4 100644 --- a/workflow_test.go +++ b/workflow_test.go @@ -365,9 +365,12 @@ func TestStreaming(t *testing.T) { func TestSubStreamJoinInPlaceHolder(t *testing.T) { initTestLogs() - exec.Command("bash", "-c", "echo 1 > .tmp/file1.txt").CombinedOutput() - exec.Command("bash", "-c", "echo 2 > .tmp/file2.txt").CombinedOutput() - exec.Command("bash", "-c", "echo 3 > .tmp/file3.txt").CombinedOutput() + _, err1 := exec.Command("bash", "-c", "echo 1 > .tmp/file1.txt").CombinedOutput() + Check(err1) + _, err2 := exec.Command("bash", "-c", "echo 2 > .tmp/file2.txt").CombinedOutput() + Check(err2) + _, err3 := exec.Command("bash", "-c", "echo 3 > .tmp/file3.txt").CombinedOutput() + Check(err3) wf := NewWorkflow("TestSubStreamJoinInPlaceHolderWf", 16) @@ -384,14 +387,14 @@ func TestSubStreamJoinInPlaceHolder(t *testing.T) { wf.Run() - _, err1 := os.Stat(".tmp/file1.txt") - assertNil(t, err1, "File missing!") + _, err1b := os.Stat(".tmp/file1.txt") + assertNil(t, err1b, "File missing!") - _, err2 := os.Stat(".tmp/file2.txt") - assertNil(t, err2, "File missing!") + _, err2b := os.Stat(".tmp/file2.txt") + assertNil(t, err2b, "File missing!") - _, err3 := os.Stat(".tmp/file3.txt") - assertNil(t, err3, "File missing!") + _, err3b := os.Stat(".tmp/file3.txt") + assertNil(t, err3b, "File missing!") _, err4 := os.Stat(".tmp/substream_merged.txt") assertNil(t, err4, "File missing!")