diff --git a/CHANGELOG.md b/CHANGELOG.md index 946b6ae7c..6cb6790f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,3 +15,4 @@ * [ENHANCEMENT] Replace go-kit/kit/log with go-kit/log. #52 * [ENHANCEMENT] Add spanlogger package. #42 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59 +* [ENHANCEMENT] Add runutil.CloseWithLogOnErr function. #58 diff --git a/go.mod b/go.mod index 29381a69e..b2daf3722 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/gogo/protobuf v1.3.2 github.com/gogo/status v1.1.0 github.com/golang/snappy v0.0.4 + github.com/google/go-cmp v0.5.5 github.com/gorilla/mux v1.8.0 // indirect github.com/grpc-ecosystem/go-grpc-middleware v1.1.0 github.com/hashicorp/consul/api v1.9.1 diff --git a/runutil/capture.go b/runutil/capture.go deleted file mode 100644 index 4e17b9c06..000000000 --- a/runutil/capture.go +++ /dev/null @@ -1,15 +0,0 @@ -package runutil - -import ( - "io" - - "github.com/pkg/errors" - - "github.com/grafana/dskit/multierror" -) - -// CloseWithErrCapture closes closer and wraps any error with the provided message and assigns it to err. -func CloseWithErrCapture(err *error, closer io.Closer, msg string) { - merr := multierror.New(*err, errors.Wrap(closer.Close(), msg)) - *err = merr.Err() -} diff --git a/runutil/runutil.go b/runutil/runutil.go new file mode 100644 index 000000000..bea0e3e4b --- /dev/null +++ b/runutil/runutil.go @@ -0,0 +1,31 @@ +package runutil + +import ( + "fmt" + "io" + "os" + + "github.com/go-kit/log" + "github.com/go-kit/log/level" + "github.com/pkg/errors" + + "github.com/grafana/dskit/multierror" +) + +// CloseWithErrCapture closes closer and wraps any error with the provided message and assigns it to err. +func CloseWithErrCapture(err *error, closer io.Closer, msg string) { + merr := multierror.New(*err, errors.Wrap(closer.Close(), msg)) + *err = merr.Err() +} + +// CloseWithLogOnErr closes an io.Closer and logs any relevant error from it wrapped with the provided format string and +// args. +func CloseWithLogOnErr(logger log.Logger, closer io.Closer, format string, args ...interface{}) { + err := closer.Close() + if err == nil || errors.Is(err, os.ErrClosed) { + return + } + + msg := fmt.Sprintf(format, args...) + level.Warn(logger).Log("msg", "detected close error", "err", fmt.Sprintf("%s: %s", msg, err.Error())) +} diff --git a/runutil/runutil_test.go b/runutil/runutil_test.go new file mode 100644 index 000000000..b0d35ac00 --- /dev/null +++ b/runutil/runutil_test.go @@ -0,0 +1,62 @@ +package runutil + +import ( + "fmt" + "os" + "testing" + + "github.com/go-kit/log/level" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" +) + +func TestCloseWithLogOnErr(t *testing.T) { + t.Run("With non-close error", func(t *testing.T) { + closer := fakeCloser{err: fmt.Errorf("an error")} + logger := fakeLogger{} + + CloseWithLogOnErr(&logger, closer, "closing failed") + + assert.Empty(t, cmp.Diff([]interface{}{ + "level", level.WarnValue(), "msg", "detected close error", "err", "closing failed: an error", + }, logger.keyvals, cmp.Comparer(func(a, b fmt.Stringer) bool { + t.Logf("Comparing %q and %q", a.String(), b.String()) + return a.String() == b.String() + }))) + }) + + t.Run("With no error", func(t *testing.T) { + closer := fakeCloser{} + logger := fakeLogger{} + + CloseWithLogOnErr(&logger, closer, "closing failed") + + assert.Empty(t, logger.keyvals) + }) + + t.Run("With closed error", func(t *testing.T) { + closer := fakeCloser{err: os.ErrClosed} + logger := fakeLogger{} + + CloseWithLogOnErr(&logger, closer, "closing failed") + + assert.Empty(t, logger.keyvals) + }) +} + +type fakeCloser struct { + err error +} + +func (c fakeCloser) Close() error { + return c.err +} + +type fakeLogger struct { + keyvals []interface{} +} + +func (l *fakeLogger) Log(keyvals ...interface{}) error { + l.keyvals = keyvals + return nil +}