Revert "Ignore io.EOF errors in plugin.Read"

This reverts commit fe59c2b. We preserve EOF
because it matches people's expectations of block-readable function behavior.

Signed-off-by: Enis Inan <[email protected]>

plugin/entryContent.go

@@ -2,6 +2,7 @@ package plugin
 
 import (
 	"context"
+	"io"
 )
 
 // entryContent is the cached result of a Read invocation
@@ -26,11 +27,13 @@ func (c *entryContentImpl) read(_ context.Context, size int64, offset int64) (da
 	data = []byte{}
 	contentSize := int64(len(c.content))
 	if offset >= contentSize {
+		err = io.EOF
 		return
 	}
 	endIx := offset + size
 	if contentSize < endIx {
 		endIx = contentSize
+		err = io.EOF
 	}
 	data = c.content[offset:endIx]
 	return

plugin/entryContent_test.go

@@ -3,6 +3,7 @@ package plugin
 import (
 	"context"
 	"fmt"
+	"io"
 	"testing"
 
 	"github.com/stretchr/testify/mock"
@@ -28,23 +29,29 @@ func (s *EntryContentTestSuite) TestEntryContentImpl() {
 		size     int64
 		offset   int64
 		expected []byte
+		err      error
 	}
 	testCases := []testCase{
 		// Test offset >= contentSize
-		testCase{size: 0, offset: contentSize, expected: []byte("")},
-		testCase{size: 0, offset: contentSize + 1, expected: []byte("")},
+		testCase{size: 0, offset: contentSize, expected: []byte(""), err: io.EOF},
+		testCase{size: 0, offset: contentSize + 1, expected: []byte(""), err: io.EOF},
 		// Test happy-cases
 		testCase{size: 0, offset: 0, expected: []byte("")},
 		testCase{size: 0, offset: 1, expected: []byte("")},
 		testCase{size: 4, offset: 2, expected: []byte("me r")},
 		testCase{size: contentSize, offset: 0, expected: rawContent},
 		// Test out-of-bounds sizes
-		testCase{size: contentSize + 1, offset: 0, expected: rawContent},
-		testCase{size: contentSize, offset: 1, expected: rawContent[1:]},
+		testCase{size: contentSize + 1, offset: 0, expected: rawContent, err: io.EOF},
+		testCase{size: contentSize, offset: 1, expected: rawContent[1:], err: io.EOF},
 	}
 	for _, testCase := range testCases {
 		actual, err := content.read(context.Background(), testCase.size, testCase.offset)
-		if s.NoError(err) {
+		if testCase.err != nil {
+			s.Equal(testCase.err, err)
+		} else {
+			s.NoError(err)
+		}
+		if testCase.expected != nil {
 			s.Equal(testCase.expected, actual)
 		}
 	}

plugin/methodWrappers.go

@@ -134,7 +134,7 @@ func List(ctx context.Context, p Parent) (*EntryMap, error) {
 // len(data) to check the amount of data that was actually read.
 //
 // If the offset is >= to the available content size, then data != nil, len(data) == 0,
-// and err == nil. Otherwise if len(data) < size, then err == nil.
+// and err == io.EOF. Otherwise if len(data) < size, then err == io.EOF.
 //
 // Note that Read is thread-safe.
 func Read(ctx context.Context, e Entry, size int64, offset int64) (data []byte, err error) {
@@ -145,11 +145,13 @@ func Read(ctx context.Context, e Entry, size int64, offset int64) (data []byte,
 	if attr := e.attributes(); attr.HasSize() {
 		contentSize := int64(attr.Size())
 		if offset >= contentSize {
+			err = io.EOF
 			return
 		}
 		minSize := size + offset
 		if contentSize < minSize {
 			size = contentSize
+			err = io.EOF
 		}
 	}
 	content, contentErr := cachedRead(ctx, e)
@@ -158,7 +160,7 @@ func Read(ctx context.Context, e Entry, size int64, offset int64) (data []byte,
 		return
 	}
 	data, readErr := content.read(ctx, size, offset)
-	if readErr != nil && readErr != io.EOF {
+	if readErr != nil {
 		err = readErr
 	}
 	return

plugin/methodWrappers_test.go

@@ -178,23 +178,6 @@ func (suite *MethodWrappersTestSuite) TestRead_ReadsTheEntryContent() {
 	}
 }
 
-func (suite *MethodWrappersTestSuite) TestRead_IgnoresIOEOFErrors() {
-	// This test-case only applies to BlockReadable entries
-	e := &methodWrappersTestsMockBlockReadableEntry{
-		newMethodWrappersTestsMockEntry("mockEntry"),
-	}
-	e.DisableDefaultCaching()
-	e.SetTestID("/foo")
-
-	ctx := context.Background()
-	e.On("Read", ctx, int64(10), int64(0)).Return([]byte("foo"), io.EOF)
-
-	data, err := Read(ctx, e, 10, 0)
-	if suite.NoError(err) {
-		suite.Equal([]byte("foo"), data)
-	}
-}
-
 func (suite *MethodWrappersTestSuite) TestRead_EntryHasSizeAttribute() {
 	rawContent := []byte("some raw content")
 	contentSize := int64(len(rawContent))
@@ -210,13 +193,11 @@ func (suite *MethodWrappersTestSuite) TestRead_EntryHasSizeAttribute() {
 
 	// Test that out-of-bounds offset does the right thing.
 	data, err := Read(ctx, e, 0, contentSize)
-	if suite.NoError(err) {
-		suite.Equal([]byte{}, data)
-	}
+	suite.Equal(io.EOF, err)
+	suite.Equal([]byte{}, data)
 	data, err = Read(ctx, e, 0, contentSize+1)
-	if suite.NoError(err) {
-		suite.Equal([]byte{}, data)
-	}
+	suite.Equal(io.EOF, err)
+	suite.Equal([]byte{}, data)
 
 	// Now test that the right "size" parameter is passed in to
 	// entryContent#read
@@ -234,9 +215,12 @@ func (suite *MethodWrappersTestSuite) TestRead_EntryHasSizeAttribute() {
 	for _, testCase := range testCases {
 		e.On("Read", ctx, testCase.expectedSize, testCase.offset).Return([]byte("success"), nil).Once()
 		actual, err := Read(context.Background(), e, testCase.size, testCase.offset)
-		if suite.NoError(err) {
-			suite.Equal([]byte("success"), actual)
+		if testCase.expectedSize != testCase.size {
+			suite.Equal(io.EOF, err)
+		} else {
+			suite.NoError(err)
 		}
+		suite.Equal([]byte("success"), actual)
 	}
 }