Browse Source

Better error message when setting values on directories

Without this commit etcd returns the following error message when
setting values on directories:

    {
      "errorCode":102,
      "message":"Not a file",
      "cause":"/postgres",
      "index":2
    }

While the above error message is accurate it's not very descriptive.
This commit adds a new error code/message which better describes why the
write operation failed. etcd now returns the following:

    {
      "errorCode":109,
      "message":"Cannot set value on directory",
      "cause":"/postgres",
      "index":2
    }
Kelsey Hightower 12 years ago
parent
commit
d13dd50d51
5 changed files with 10 additions and 6 deletions
  1. 2 0
      Documentation/errorcode.md
  2. 3 1
      error/error.go
  3. 1 1
      store/node.go
  4. 2 2
      store/store.go
  5. 2 2
      store/store_test.go

+ 2 - 0
Documentation/errorcode.md

@@ -21,6 +21,7 @@ Error code corresponding strerror
         EcodeNotDir         = 104
         EcodeNotDir         = 104
         EcodeNodeExist      = 105
         EcodeNodeExist      = 105
         EcodeKeyIsPreserved = 106
         EcodeKeyIsPreserved = 106
+        EcodeNoValueOnDir   = 109
 
 
         EcodeValueRequired     = 200
         EcodeValueRequired     = 200
         EcodePrevValueRequired = 201
         EcodePrevValueRequired = 201
@@ -42,6 +43,7 @@ Error code corresponding strerror
     errors[104] = "Not A Directory"
     errors[104] = "Not A Directory"
     errors[105] = "Already exists" // create
     errors[105] = "Already exists" // create
     errors[106] = "The prefix of given key is a keyword in etcd"
     errors[106] = "The prefix of given key is a keyword in etcd"
+    errors[109] = "Cannot set value on directory"
 
 
     // Post form related errors
     // Post form related errors
     errors[200] = "Value is Required in POST form"
     errors[200] = "Value is Required in POST form"

+ 3 - 1
error/error.go

@@ -34,6 +34,7 @@ const (
 	EcodeKeyIsPreserved = 106
 	EcodeKeyIsPreserved = 106
 	EcodeRootROnly      = 107
 	EcodeRootROnly      = 107
 	EcodeDirNotEmpty    = 108
 	EcodeDirNotEmpty    = 108
+	EcodeNoValueOnDir   = 109
 
 
 	EcodeValueRequired        = 200
 	EcodeValueRequired        = 200
 	EcodePrevValueRequired    = 201
 	EcodePrevValueRequired    = 201
@@ -66,6 +67,7 @@ func init() {
 	errors[EcodeRootROnly] = "Root is read only"
 	errors[EcodeRootROnly] = "Root is read only"
 	errors[EcodeKeyIsPreserved] = "The prefix of given key is a keyword in etcd"
 	errors[EcodeKeyIsPreserved] = "The prefix of given key is a keyword in etcd"
 	errors[EcodeDirNotEmpty] = "Directory not empty"
 	errors[EcodeDirNotEmpty] = "Directory not empty"
+	errors[EcodeNoValueOnDir] = "Cannot set value on directory"
 
 
 	// Post form related errors
 	// Post form related errors
 	errors[EcodeValueRequired] = "Value is Required in POST form"
 	errors[EcodeValueRequired] = "Value is Required in POST form"
@@ -126,7 +128,7 @@ func (e Error) Write(w http.ResponseWriter) {
 	switch e.ErrorCode {
 	switch e.ErrorCode {
 	case EcodeKeyNotFound:
 	case EcodeKeyNotFound:
 		status = http.StatusNotFound
 		status = http.StatusNotFound
-	case EcodeNotFile, EcodeDirNotEmpty:
+	case EcodeNotFile, EcodeDirNotEmpty, EcodeNoValueOnDir:
 		status = http.StatusForbidden
 		status = http.StatusForbidden
 	case EcodeTestFailed, EcodeNodeExist:
 	case EcodeTestFailed, EcodeNodeExist:
 		status = http.StatusPreconditionFailed
 		status = http.StatusPreconditionFailed

+ 1 - 1
store/node.go

@@ -102,7 +102,7 @@ func (n *node) Read() (string, *etcdErr.Error) {
 // If the receiver node is a directory, a "Not A File" error will be returned.
 // If the receiver node is a directory, a "Not A File" error will be returned.
 func (n *node) Write(value string, index uint64) *etcdErr.Error {
 func (n *node) Write(value string, index uint64) *etcdErr.Error {
 	if n.IsDir() {
 	if n.IsDir() {
-		return etcdErr.NewError(etcdErr.EcodeNotFile, "", n.store.Index())
+		return etcdErr.NewError(etcdErr.EcodeNoValueOnDir, "", n.store.Index())
 	}
 	}
 
 
 	n.Value = value
 	n.Value = value

+ 2 - 2
store/store.go

@@ -419,7 +419,7 @@ func (s *store) Update(nodePath string, newValue string, expireTime time.Time) (
 	if n.IsDir() && len(newValue) != 0 {
 	if n.IsDir() && len(newValue) != 0 {
 		// if the node is a directory, we cannot update value to non-empty
 		// if the node is a directory, we cannot update value to non-empty
 		s.Stats.Inc(UpdateFail)
 		s.Stats.Inc(UpdateFail)
-		return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, currIndex)
+		return nil, etcdErr.NewError(etcdErr.EcodeNoValueOnDir, nodePath, currIndex)
 	}
 	}
 
 
 	n.Write(newValue, nextIndex)
 	n.Write(newValue, nextIndex)
@@ -481,7 +481,7 @@ func (s *store) internalCreate(nodePath string, dir bool, value string, unique,
 	if n != nil {
 	if n != nil {
 		if replace {
 		if replace {
 			if n.IsDir() {
 			if n.IsDir() {
-				return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, currIndex)
+				return nil, etcdErr.NewError(etcdErr.EcodeNoValueOnDir, nodePath, currIndex)
 			}
 			}
 			e.PrevNode = n.Repr(false, false)
 			e.PrevNode = n.Repr(false, false)
 
 

+ 2 - 2
store/store_test.go

@@ -232,8 +232,8 @@ func TestStoreUpdateFailsIfDirectory(t *testing.T) {
 	s.Create("/foo", true, "", false, Permanent)
 	s.Create("/foo", true, "", false, Permanent)
 	e, _err := s.Update("/foo", "baz", Permanent)
 	e, _err := s.Update("/foo", "baz", Permanent)
 	err := _err.(*etcdErr.Error)
 	err := _err.(*etcdErr.Error)
-	assert.Equal(t, err.ErrorCode, etcdErr.EcodeNotFile, "")
-	assert.Equal(t, err.Message, "Not a file", "")
+	assert.Equal(t, err.ErrorCode, etcdErr.EcodeNoValueOnDir, "")
+	assert.Equal(t, err.Message, "Cannot set value on directory", "")
 	assert.Equal(t, err.Cause, "/foo", "")
 	assert.Equal(t, err.Cause, "/foo", "")
 	assert.Nil(t, e, "")
 	assert.Nil(t, e, "")
 }
 }