Fix Bytes/AppendFile/WriteFile to return read errs

Calling any sink method (such as Bytes) should return a non-nil error if there is an error reading the pipe to completion. Due to a bug, Bytes, AppendFile, and WriteFile failed to do this. This commit fixes the bug, and also tests that the other sink methods behave correctly (which they do). Fixes #170.

Co-authored-by: Namho Kim <git@namho.kim>

Author: bitfield

script.go

@@ -221,7 +221,7 @@ func (p *Pipe) Bytes() ([]byte, error) {
 	if err != nil {
 		p.SetError(err)
 	}
-	return data, nil
+	return data, p.Error()
 }
 
 // Close closes the pipe's associated reader. This is a no-op if the reader is
@@ -894,9 +894,8 @@ func (p *Pipe) writeOrAppendFile(path string, mode int) (int64, error) {
 	wrote, err := io.Copy(out, p)
 	if err != nil {
 		p.SetError(err)
-		return 0, err
 	}
-	return wrote, nil
+	return wrote, p.Error()
 }
 
 // ReadAutoCloser wraps an [io.ReadCloser] so that it will be automatically

script_test.go

@@ -15,6 +15,7 @@ import (
 	"regexp"
 	"strings"
 	"testing"
+	"testing/iotest"
 
 	"github.com/bitfield/script"
 	"github.com/google/go-cmp/cmp"
@@ -1259,6 +1260,16 @@ func TestStdinReadsFromProgramStandardInput(t *testing.T) {
 	}
 }
 
+func TestStdoutReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	brokenReader := iotest.ErrReader(errors.New("oh no"))
+	_, err := script.NewPipe().WithStdout(io.Discard).
+		WithReader(brokenReader).Stdout()
+	if err == nil {
+		t.Fatal(nil)
+	}
+}
+
 func TestStdoutSendsPipeContentsToConfiguredStandardOutput(t *testing.T) {
 	t.Parallel()
 	buf := &bytes.Buffer{}
@@ -1307,6 +1318,18 @@ func TestAppendFile_AppendsAllItsInputToSpecifiedFile(t *testing.T) {
 	}
 }
 
+func TestAppendFile_ReturnsBytesWrittenAndErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	var want int64 = 1
+	got, err := script.NewPipe().WithReader(partialErrReader{}).AppendFile(t.TempDir() + "/tmp")
+	if err == nil {
+		t.Fatal("want error reading pipe with error status, got nil")
+	}
+	if !cmp.Equal(want, got) {
+		t.Error(cmp.Diff(want, got))
+	}
+}
+
 func TestBytesOutputsInputBytesUnchanged(t *testing.T) {
 	t.Parallel()
 	want := []byte{8, 0, 0, 16}
@@ -1320,6 +1343,15 @@ func TestBytesOutputsInputBytesUnchanged(t *testing.T) {
 	}
 }
 
+func TestBytesReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	brokenReader := iotest.ErrReader(errors.New("oh no"))
+	_, err := script.NewPipe().WithReader(brokenReader).Bytes()
+	if err == nil {
+		t.Fatal(nil)
+	}
+}
+
 func TestCountLines_CountsCorrectNumberOfLinesInInput(t *testing.T) {
 	t.Parallel()
 	want := 3
@@ -1344,6 +1376,15 @@ func TestCountLines_Counts0LinesInEmptyInput(t *testing.T) {
 	}
 }
 
+func TestCountLines_ReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	brokenReader := iotest.ErrReader(errors.New("oh no"))
+	_, err := script.NewPipe().WithReader(brokenReader).CountLines()
+	if err == nil {
+		t.Fatal(nil)
+	}
+}
+
 func TestSHA256Sum_OutputsCorrectHash(t *testing.T) {
 	t.Parallel()
 	tcs := []struct {
@@ -1379,6 +1420,24 @@ func TestSHA256Sum_OutputsCorrectHash(t *testing.T) {
 	}
 }
 
+func TestSHA256Sum_ReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	brokenReader := iotest.ErrReader(errors.New("oh no"))
+	_, err := script.NewPipe().WithReader(brokenReader).SHA256Sum()
+	if err == nil {
+		t.Fatal(nil)
+	}
+}
+
+func TestSliceSink_ReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	brokenReader := iotest.ErrReader(errors.New("oh no"))
+	_, err := script.NewPipe().WithReader(brokenReader).Slice()
+	if err == nil {
+		t.Fatal(nil)
+	}
+}
+
 func TestSliceSink_(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
@@ -1441,6 +1500,15 @@ func TestStringOutputsInputStringUnchanged(t *testing.T) {
 	}
 }
 
+func TestStringReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	brokenReader := iotest.ErrReader(errors.New("oh no"))
+	_, err := script.NewPipe().WithReader(brokenReader).String()
+	if err == nil {
+		t.Fatal(nil)
+	}
+}
+
 func TestWaitReadsPipeSourceToCompletion(t *testing.T) {
 	t.Parallel()
 	source := bytes.NewBufferString("hello")
@@ -1470,6 +1538,25 @@ func TestWriteFile_WritesInputToFileCreatingItIfNecessary(t *testing.T) {
 	}
 }
 
+// partialErrReader returns 1 and a non-EOF error on reading.
+type partialErrReader struct{}
+
+func (r partialErrReader) Read(p []byte) (int, error) {
+	return 1, errors.New("oh no")
+}
+
+func TestWriteFile_ReturnsBytesWrittenAndErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	var want int64 = 1
+	got, err := script.NewPipe().WithReader(partialErrReader{}).WriteFile(t.TempDir() + "/tmp")
+	if err == nil {
+		t.Fatal("want error reading pipe with error status, got nil")
+	}
+	if !cmp.Equal(want, got) {
+		t.Error(cmp.Diff(want, got))
+	}
+}
+
 func TestWriteFile_TruncatesExistingFile(t *testing.T) {
 	t.Parallel()
 	want := "Hello, world"
@@ -1629,6 +1716,16 @@ func TestReadReturnsEOFOnUninitialisedPipe(t *testing.T) {
 	}
 }
 
+func TestReadReturnsErrorGivenReadErrorOnPipe(t *testing.T) {
+	t.Parallel()
+	brokenReader := iotest.ErrReader(errors.New("oh no"))
+	buf := make([]byte, 0)
+	_, err := script.NewPipe().WithReader(brokenReader).Read(buf)
+	if err == nil {
+		t.Fatal(nil)
+	}
+}
+
 func ExampleArgs() {
 	script.Args().Stdout()
 	// prints command-line arguments