Merge pull request #504 from kelseyhightower/master

Better error message when setting values on directories

Documentation/errorcode.md

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

error/error.go

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

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.
 func (n *node) Write(value string, index uint64) *etcdErr.Error {
 	if n.IsDir() {
-		return etcdErr.NewError(etcdErr.EcodeNotFile, "", n.store.Index())
+		return etcdErr.NewError(etcdErr.EcodeNoValueOnDir, "", n.store.Index())
 	}
 
 	n.Value = value

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 the node is a directory, we cannot update value to non-empty
 		s.Stats.Inc(UpdateFail)
-		return nil, etcdErr.NewError(etcdErr.EcodeNotFile, nodePath, currIndex)
+		return nil, etcdErr.NewError(etcdErr.EcodeNoValueOnDir, nodePath, currIndex)
 	}
 
 	n.Write(newValue, nextIndex)
@@ -481,7 +481,7 @@ func (s *store) internalCreate(nodePath string, dir bool, value string, unique,
 	if n != nil {
 		if replace {
 			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)
 

store/store_test.go

@@ -232,8 +232,8 @@ func TestStoreUpdateFailsIfDirectory(t *testing.T) {
 	s.Create("/foo", true, "", false, Permanent)
 	e, _err := s.Update("/foo", "baz", Permanent)
 	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.Nil(t, e, "")
 }