Repository: ceph/go-ceph
Pull request #344 (2020-08-10 15:09:52)

General changes to error types
Fixes: #343 

These changes change the function that was formerly called `Errno` to `ErrorCode`.
For consistency with the above we now use a common formatting function for the three package error types and it prints negative numbers when the error code is negative (as it should typically be).

See linked issue for more context.

## Checklist
- [x] Added tests for features and functional changes
- [x] Public functions and types are documented
- [x] Standard formatting is applied to Go code



Patch:
diff --git a/cephfs/errors.go b/cephfs/errors.go
index 1462fc4..4ac3c84 100644
--- a/cephfs/errors.go
+++ b/cephfs/errors.go
@@ -7,7 +7,6 @@ import "C"
 
 import (
 	"errors"
-	"fmt"
 
 	"github.com/ceph/go-ceph/internal/errutil"
 )
@@ -17,14 +16,10 @@ type cephFSError int
 
 // Error returns the error string for the cephFSError type.
 func (e cephFSError) Error() string {
-	errno, s := errutil.FormatErrno(int(e))
-	if s == "" {
-		return fmt.Sprintf("cephfs: ret=%d", errno)
-	}
-	return fmt.Sprintf("cephfs: ret=%d, %s", errno, s)
+	return errutil.FormatErrorCode("cephfs", int(e))
 }
 
-func (e cephFSError) Errno() int {
+func (e cephFSError) ErrorCode() int {
 	return int(e)
 }
 
diff --git a/internal/errutil/strerror.go b/internal/errutil/strerror.go
index 907c702..2fb24cb 100644
--- a/internal/errutil/strerror.go
+++ b/internal/errutil/strerror.go
@@ -14,6 +14,7 @@ package errutil
 import "C"
 
 import (
+	"fmt"
 	"unsafe"
 )
 
@@ -37,9 +38,15 @@ func FormatErrno(errno int) (int, string) {
 	return errno, C.GoString((*C.char)(unsafe.Pointer(&buf[0])))
 }
 
-// StrError returns a string describing the errno. The string will be empty if
-// the errno is not known.
-func StrError(errno int) string {
-	_, s := FormatErrno(errno)
-	return s
+// FormatErrorCode returns a string that describes the supplied error source
+// and error code as a string. Suitable to use in Error() methods.  If the
+// error code maps to an errno the string will contain a description of the
+// error. Otherwise the string will only indicate the source and value if the
+// value does not map to a known errno.
+func FormatErrorCode(source string, errValue int) string {
+	_, s := FormatErrno(errValue)
+	if s == "" {
+		return fmt.Sprintf("%s: ret=%d", source, errValue)
+	}
+	return fmt.Sprintf("%s: ret=%d, %s", source, errValue, s)
 }
diff --git a/rados/errors.go b/rados/errors.go
index 2f643f3..6039d12 100644
--- a/rados/errors.go
+++ b/rados/errors.go
@@ -7,7 +7,6 @@ import "C"
 
 import (
 	"errors"
-	"fmt"
 
 	"github.com/ceph/go-ceph/internal/errutil"
 )
@@ -17,14 +16,10 @@ type radosError int
 
 // Error returns the error string for the radosError type.
 func (e radosError) Error() string {
-	errno, s := errutil.FormatErrno(int(e))
-	if s == "" {
-		return fmt.Sprintf("rados: ret=%d", errno)
-	}
-	return fmt.Sprintf("rados: ret=%d, %s", errno, s)
+	return errutil.FormatErrorCode("rados", int(e))
 }
 
-func (e radosError) Errno() int {
+func (e radosError) ErrorCode() int {
 	return int(e)
 }
 
diff --git a/rbd/errors.go b/rbd/errors.go
index 87a43b2..5bfc092 100644
--- a/rbd/errors.go
+++ b/rbd/errors.go
@@ -7,7 +7,6 @@ import "C"
 
 import (
 	"errors"
-	"fmt"
 
 	"github.com/ceph/go-ceph/internal/errutil"
 )
@@ -16,14 +15,10 @@ import (
 type rbdError int
 
 func (e rbdError) Error() string {
-	errno, s := errutil.FormatErrno(int(e))
-	if s == "" {
-		return fmt.Sprintf("rbd: ret=%d", errno)
-	}
-	return fmt.Sprintf("rbd: ret=%d, %s", errno, s)
+	return errutil.FormatErrorCode("rbd", int(e))
 }
 
-func (e rbdError) Errno() int {
+func (e rbdError) ErrorCode() int {
 	return int(e)
 }
 


Test patch:
diff --git a/cephfs/errors_test.go b/cephfs/errors_test.go
index aa8ef72..ddfd91c 100644
--- a/cephfs/errors_test.go
+++ b/cephfs/errors_test.go
@@ -13,12 +13,12 @@ func TestCephFSError(t *testing.T) {
 
 	err = getError(-5) // IO error
 	assert.Error(t, err)
-	assert.Equal(t, err.Error(), "cephfs: ret=5, Input/output error")
+	assert.Equal(t, err.Error(), "cephfs: ret=-5, Input/output error")
 
-	errno, ok := err.(interface{ Errno() int })
+	errno, ok := err.(interface{ ErrorCode() int })
 	assert.True(t, ok)
 	require.NotNil(t, errno)
-	assert.Equal(t, errno.Errno(), -5)
+	assert.Equal(t, errno.ErrorCode(), -5)
 
 	err = getError(345) // no such errno
 	assert.Error(t, err)
diff --git a/internal/errutil/strerror_test.go b/internal/errutil/strerror_test.go
index b89e489..4d17b6c 100644
--- a/internal/errutil/strerror_test.go
+++ b/internal/errutil/strerror_test.go
@@ -20,13 +20,13 @@ func TestFormatError(t *testing.T) {
 	assert.Equal(t, msg, "")
 }
 
-func TestStrError(t *testing.T) {
-	msg := StrError(39)
-	assert.Equal(t, msg, "Directory not empty")
+func TestFormatErrorCode(t *testing.T) {
+	msg := FormatErrorCode("test", -39)
+	assert.Equal(t, msg, "test: ret=-39, Directory not empty")
 
-	msg = StrError(-5)
-	assert.Equal(t, msg, "Input/output error")
+	msg = FormatErrorCode("test", -5)
+	assert.Equal(t, msg, "test: ret=-5, Input/output error")
 
-	msg = StrError(345)
-	assert.Equal(t, msg, "")
+	msg = FormatErrorCode("boop", 345)
+	assert.Equal(t, msg, "boop: ret=345")
 }
diff --git a/rados/errors_test.go b/rados/errors_test.go
index 090f488..4564d81 100644
--- a/rados/errors_test.go
+++ b/rados/errors_test.go
@@ -13,12 +13,12 @@ func TestRadosError(t *testing.T) {
 
 	err = getError(-5) // IO error
 	assert.Error(t, err)
-	assert.Equal(t, err.Error(), "rados: ret=5, Input/output error")
+	assert.Equal(t, err.Error(), "rados: ret=-5, Input/output error")
 
-	errno, ok := err.(interface{ Errno() int })
+	errno, ok := err.(interface{ ErrorCode() int })
 	assert.True(t, ok)
 	require.NotNil(t, errno)
-	assert.Equal(t, errno.Errno(), -5)
+	assert.Equal(t, errno.ErrorCode(), -5)
 
 	err = getError(345) // no such errno
 	assert.Error(t, err)
diff --git a/rbd/diff_iterate_test.go b/rbd/diff_iterate_test.go
index 6a4019f..3b4ff46 100644
--- a/rbd/diff_iterate_test.go
+++ b/rbd/diff_iterate_test.go
@@ -335,8 +335,8 @@ func testDiffIterateEarlyExit(t *testing.T, ioctx *rados.IOContext) {
 			},
 		})
 	assert.Error(t, err)
-	if errno, ok := err.(interface{ Errno() int }); assert.True(t, ok) {
-		assert.EqualValues(t, -5, errno.Errno())
+	if errno, ok := err.(interface{ ErrorCode() int }); assert.True(t, ok) {
+		assert.EqualValues(t, -5, errno.ErrorCode())
 	}
 	if assert.Len(t, calls, 1) {
 		assert.EqualValues(t, 0, calls[0].offset)
diff --git a/rbd/errors_test.go b/rbd/errors_test.go
index 1fc1065..987fa88 100644
--- a/rbd/errors_test.go
+++ b/rbd/errors_test.go
@@ -13,12 +13,12 @@ func TestRBDError(t *testing.T) {
 
 	err = getError(-39) // NOTEMPTY (image still has a snapshot)
 	assert.Error(t, err)
-	assert.Equal(t, err.Error(), "rbd: ret=39, Directory not empty")
+	assert.Equal(t, err.Error(), "rbd: ret=-39, Directory not empty")
 
-	errno, ok := err.(interface{ Errno() int })
+	errno, ok := err.(interface{ ErrorCode() int })
 	assert.True(t, ok)
 	require.NotNil(t, errno)
-	assert.Equal(t, errno.Errno(), -39)
+	assert.Equal(t, errno.ErrorCode(), -39)
 
 	err = getError(345) // no such errno
 	assert.Error(t, err)