runutil: Simplified CloseWithErrCapture.

pkg/block/index.go

@@ -317,7 +317,7 @@ func GatherIndexIssueStats(logger log.Logger, fn string, minTime int64, maxTime
 	if err != nil {
 		return stats, errors.Wrap(err, "open index file")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, r, "gather index issue file reader")
+	defer runutil.CloseWithErrCapture(&err, r, "gather index issue file reader")
 
 	p, err := r.Postings(index.AllPostingsKey())
 	if err != nil {
@@ -432,33 +432,33 @@ func Repair(logger log.Logger, dir string, id ulid.ULID, source metadata.SourceT
 	if err != nil {
 		return resid, errors.Wrap(err, "open block")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, b, "repair block reader")
+	defer runutil.CloseWithErrCapture(&err, b, "repair block reader")
 
 	indexr, err := b.Index()
 	if err != nil {
 		return resid, errors.Wrap(err, "open index")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, indexr, "repair index reader")
+	defer runutil.CloseWithErrCapture(&err, indexr, "repair index reader")
 
 	chunkr, err := b.Chunks()
 	if err != nil {
 		return resid, errors.Wrap(err, "open chunks")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, chunkr, "repair chunk reader")
+	defer runutil.CloseWithErrCapture(&err, chunkr, "repair chunk reader")
 
 	resdir := filepath.Join(dir, resid.String())
 
 	chunkw, err := chunks.NewWriter(filepath.Join(resdir, ChunksDirname))
 	if err != nil {
 		return resid, errors.Wrap(err, "open chunk writer")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, chunkw, "repair chunk writer")
+	defer runutil.CloseWithErrCapture(&err, chunkw, "repair chunk writer")
 
 	indexw, err := index.NewWriter(filepath.Join(resdir, IndexFilename))
 	if err != nil {
 		return resid, errors.Wrap(err, "open index writer")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, indexw, "repair index writer")
+	defer runutil.CloseWithErrCapture(&err, indexw, "repair index writer")
 
 	// TODO(fabxc): adapt so we properly handle the version once we update to an upstream
 	// that has multiple.

pkg/compact/downsample/downsample.go

@@ -45,13 +45,13 @@ func Downsample(
 	if err != nil {
 		return id, errors.Wrap(err, "open index reader")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, indexr, "downsample index reader")
+	defer runutil.CloseWithErrCapture(&err, indexr, "downsample index reader")
 
 	chunkr, err := b.Chunks()
 	if err != nil {
 		return id, errors.Wrap(err, "open chunk reader")
 	}
-	defer runutil.CloseWithErrCapture(logger, &err, chunkr, "downsample chunk reader")
+	defer runutil.CloseWithErrCapture(&err, chunkr, "downsample chunk reader")
 
 	rng := origMeta.MaxTime - origMeta.MinTime
 

pkg/runutil/runutil.go

@@ -33,7 +33,7 @@
 // For capturing error, use CloseWithErrCapture:
 //
 // 	var err error
-// 	defer runutil.CloseWithErrCapture(logger, &err, closer, "log format message")
+// 	defer runutil.CloseWithErrCapture(&err, closer, "log format message")
 //
 // 	// ...
 //
@@ -49,6 +49,7 @@ import (
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
 	"github.com/pkg/errors"
+	"github.com/prometheus/tsdb"
 )
 
 // Repeat executes f every interval seconds until stopc is closed.
@@ -107,26 +108,12 @@ func CloseWithLogOnErr(logger log.Logger, closer io.Closer, format string, a ...
 	level.Warn(logger).Log("msg", "detected close error", "err", errors.Wrap(err, fmt.Sprintf(format, a...)))
 }
 
-// CloseWithErrCapture runs function and on error tries to return error by argument.
-// If error is already there we assume that error has higher priority and we just log the function error.
-func CloseWithErrCapture(logger log.Logger, err *error, closer io.Closer, format string, a ...interface{}) {
-	closeErr := closer.Close()
-	if closeErr == nil {
-		return
-	}
-
-	if *err == nil {
-		err = &closeErr
-		return
-	}
-
-	// There is already an error, let's log this one.
-	if logger == nil {
-		logger = log.NewLogfmtLogger(os.Stderr)
-	}
+// CloseWithErrCapture runs function and on error return error by argument including the given error (usually
+// from caller function)
+func CloseWithErrCapture(err *error, closer io.Closer, format string, a ...interface{}) {
+	var merr tsdb.MultiError
 
-	level.Warn(logger).Log(
-		"msg", "detected best effort close error that was preempted from the more important one",
-		"err", errors.Wrap(closeErr, fmt.Sprintf(format, a...)),
-	)
+	merr.Add(*err)
+	merr.Add(errors.Wrapf(closer.Close(), format, a...))
+	*err = merr.Err()
 }

pkg/runutil/runutil_test.go

@@ -0,0 +1,70 @@
+package runutil
+
+import (
+	"github.com/pkg/errors"
+	"io"
+	"testing"
+)
+
+type testCloser struct {
+	err error
+}
+
+func (c testCloser) Close() error {
+	return c.err
+}
+
+func TestCloseWithErrCapture(t *testing.T) {
+	for _, tcase := range []struct{
+	   err error
+	   closer io.Closer
+
+	   expectedErrStr string
+	}{
+	    {
+	    	err: nil,
+	    	closer: testCloser{err:nil},
+	    	expectedErrStr: "",
+	    },
+		{
+			err: errors.New("test"),
+			closer: testCloser{err:nil},
+			expectedErrStr: "test",
+		},
+		{
+			err: nil,
+			closer: testCloser{err:errors.New("test")},
+			expectedErrStr: "close: test",
+		},
+		{
+			err: errors.New("test"),
+			closer: testCloser{err:errors.New("test")},
+			expectedErrStr: "2 errors: test; close: test",
+		},
+	}{
+	  if ok := t.Run("", func(t *testing.T) {
+	  	ret := tcase.err
+	  	CloseWithErrCapture(&ret, tcase.closer, "close")
+
+	  	if tcase.expectedErrStr == "" {
+	  		if ret != nil {
+	  			t.Error("Expected error to be nil")
+	  			t.Fail()
+			}
+		} else {
+			if ret == nil {
+				t.Error("Expected error to be not nil")
+				t.Fail()
+			}
+
+			if tcase.expectedErrStr != ret.Error() {
+				t.Errorf("%s != %s", tcase.expectedErrStr, ret.Error())
+				t.Fail()
+			}
+		}
+
+	  }); !ok {
+	    return
+	  }
+	}
+}