A fix for Bug#769040

A fix for
http://bugs.launchpad.net/bugs/769040
"Incomplete error output in Memcached protocol"

Append an error description to a memcached error answer.

core/iproto.c

@@ -35,6 +35,7 @@
 
 const uint32_t msg_ping = 0xff00;
 STRS(error_codes, ERROR_CODES);
+DESC_STRS(error_codes, ERROR_CODES);
 
 static struct tbuf *
 iproto_parse(struct tbuf *in)

include/iproto.h

@@ -38,39 +38,40 @@ struct iproto_interactor
 
 void iproto_interact(void *);
 
-#define ERROR_CODES(_)							\
-	_(ERR_CODE_OK,                    0x00000000) /* OK */		\
-	_(ERR_CODE_NONMASTER,             0x00000102) /* Non master connection, but it should be */ \
-	_(ERR_CODE_ILLEGAL_PARAMS,        0x00000202) /* Illegal parametrs */ \
-	_(ERR_CODE_BAD_UID,               0x00000302) /* Uid not from this storage range */ \
-	_(ERR_CODE_NODE_IS_RO,            0x00000401) /* Node is marked as read-only */ \
-	_(ERR_CODE_NODE_IS_NOT_LOCKED,    0x00000501) /* Node isn't locked */ \
-	_(ERR_CODE_NODE_IS_LOCKED,        0x00000601) /* Node is locked */ \
-	_(ERR_CODE_MEMORY_ISSUE,          0x00000701) /* Some memory issues */ \
-	_(ERR_CODE_BAD_INTEGRITY,         0x00000802) /* Bad graph integrity */ \
-	_(ERR_CODE_UNSUPPORTED_COMMAND,   0x00000a02) /* Unsupported command */ \
+#define ERROR_CODES(_)					    \
+	_(ERR_CODE_OK,                    0x00000000, "ok") \
+	_(ERR_CODE_NONMASTER,             0x00000102, "non master connection, but it should be") \
+	_(ERR_CODE_ILLEGAL_PARAMS,        0x00000202, "illegal parametrs") \
+	_(ERR_CODE_BAD_UID,               0x00000302, "uid not from this storage range") \
+	_(ERR_CODE_NODE_IS_RO,            0x00000401, "node is marked as read-only") \
+	_(ERR_CODE_NODE_IS_NOT_LOCKED,    0x00000501, "node isn't locked") \
+	_(ERR_CODE_NODE_IS_LOCKED,        0x00000601, "node is locked") \
+	_(ERR_CODE_MEMORY_ISSUE,          0x00000701, "some memory issues") \
+	_(ERR_CODE_BAD_INTEGRITY,         0x00000802, "bad graph integrity") \
+	_(ERR_CODE_UNSUPPORTED_COMMAND,   0x00000a02, "unsupported command") \
 	/* gap due to silverproxy */					\
-	_(ERR_CODE_CANNOT_REGISTER,       0x00001801) /* Can't register new user */ \
-	_(ERR_CODE_CANNOT_INIT_ALERT_ID,  0x00001a01) /* Can't generate alert id */ \
-	_(ERR_CODE_CANNOT_DEL,            0x00001b02) /* Can't del node */ \
-	_(ERR_CODE_USER_NOT_REGISTERED,   0x00001c02) /* User isn't registered */ \
+	_(ERR_CODE_CANNOT_REGISTER,       0x00001801, "can't register new user") \
+	_(ERR_CODE_CANNOT_INIT_ALERT_ID,  0x00001a01, "can't generate alert id") \
+	_(ERR_CODE_CANNOT_DEL,            0x00001b02, "can't del node") \
+	_(ERR_CODE_USER_NOT_REGISTERED,   0x00001c02, "user isn't registered") \
 	/* silversearch error codes */					\
-	_(ERR_CODE_SYNTAX_ERROR,          0x00001d02) /* Syntax error in query */ \
-	_(ERR_CODE_WRONG_FIELD,           0x00001e02) /* Unknown field */ \
-	_(ERR_CODE_WRONG_NUMBER,          0x00001f02) /* Number value is out of range */ \
-	_(ERR_CODE_DUPLICATE,             0x00002002) /* Insert already existing object */ \
-	_(ERR_CODE_UNSUPPORTED_ORDER,     0x00002202) /* Can not order result */ \
-	_(ERR_CODE_MULTIWRITE,            0x00002302) /* Multiple to update/delete */ \
-	_(ERR_CODE_NOTHING,               0x00002400) /* nothing to do (not an error) */ \
-	_(ERR_CODE_UPDATE_ID,             0x00002502) /* id's update */ \
-	_(ERR_CODE_WRONG_VERSION,         0x00002602)	/* unsupported version of protocol */ \
+	_(ERR_CODE_SYNTAX_ERROR,          0x00001d02, "syntax error in query") \
+	_(ERR_CODE_WRONG_FIELD,           0x00001e02, "unknown field") \
+	_(ERR_CODE_WRONG_NUMBER,          0x00001f02, "number value is out of range") \
+	_(ERR_CODE_DUPLICATE,             0x00002002, "insert already existing object") \
+	_(ERR_CODE_UNSUPPORTED_ORDER,     0x00002202, "can not order result") \
+	_(ERR_CODE_MULTIWRITE,            0x00002302, "multiple to update/delete") \
+	_(ERR_CODE_NOTHING,               0x00002400, "nothing to do (not an error)") \
+	_(ERR_CODE_UPDATE_ID,             0x00002502, "id's update") \
+	_(ERR_CODE_WRONG_VERSION,         0x00002602, "unsupported version of protocol") \
 	/* other generic error codes */					\
-	_(ERR_CODE_UNKNOWN_ERROR,         0x00002702) \
-        _(ERR_CODE_NODE_NOT_FOUND,	  0x00003102) \
-	_(ERR_CODE_NODE_FOUND,		  0x00003702) \
-	_(ERR_CODE_INDEX_VIOLATION,	  0x00003802) \
-	_(ERR_CODE_NO_SUCH_NAMESPACE,	  0x00003902)
+	_(ERR_CODE_UNKNOWN_ERROR,         0x00002702, "unknown error") \
+        _(ERR_CODE_NODE_NOT_FOUND,	  0x00003102, "node isn't found") \
+	_(ERR_CODE_NODE_FOUND,		  0x00003702, "node is found") \
+	_(ERR_CODE_INDEX_VIOLATION,	  0x00003802, "some index violation occur") \
+	_(ERR_CODE_NO_SUCH_NAMESPACE,	  0x00003902, "there is no such namespace")
 
 ENUM(error_codes, ERROR_CODES);
 extern char *error_codes_strs[];
+extern char *error_codes_desc_strs[];
 #endif

include/util.h

@@ -35,11 +35,14 @@
 #endif
 
 /* Macros to define enum and corresponding strings. */
-#define ENUM_MEMBER(s, v) s = v,
-#define ENUM_STRS_MEMBER(s, v) [s] = #s,
+#define ENUM_MEMBER(s, v, d...) s = v,
+#define ENUM_STRS_MEMBER(s, v, d...) [s] = #s,
+#define ENUM_DESC_STRS_MEMBER(s, v, d...) [s] = d,
 #define ENUM(enum_name, enum_members) enum enum_name {enum_members(ENUM_MEMBER) enum_name##_MAX}
 #define STRS(enum_name, enum_members) \
 	char *enum_name##_strs[enum_name##_MAX + 1] = {enum_members(ENUM_STRS_MEMBER) '\0'}
+#define DESC_STRS(enum_name, enum_members) \
+	char *enum_name##_desc_strs[enum_name##_MAX + 1] = {enum_members(ENUM_DESC_STRS_MEMBER) '\0'}
 
 // Macros for printf functions
 #include <inttypes.h>

mod/silverbox/memcached.rl

@@ -30,6 +30,7 @@
 #include <unistd.h>
 #include <stdlib.h>
 
+#include <iproto.h>
 #include <salloc.h>
 #include <palloc.h>
 #include <fiber.h>
@@ -239,18 +240,22 @@ memcached_dispatch(struct box_txn *txn)
 
 	say_debug("memcached_dispatch '%.*s'", MIN((int)(pe - p), 40) , p);
 
-#define STORE ({							\
-	stats.cmd_set++;						\
-	if (bytes > (1<<20)) {						\
-		add_iov("SERVER_ERROR object too large for cache\r\n", 41); \
-	} else {							\
-		if (store(txn, key, exptime, flags, bytes, data) == 0) { \
-			stats.total_items++;				\
-			add_iov("STORED\r\n", 8);			\
-		} else {						\
-			add_iov("SERVER_ERROR\r\n", 14);		\
-		}							\
-	}								\
+#define STORE ({									\
+	stats.cmd_set++;								\
+	if (bytes > (1<<20)) {								\
+		add_iov("SERVER_ERROR object too large for cache\r\n", 41);		\
+	} else {									\
+		u32 ret_code;								\
+		if ((ret_code = store(txn, key, exptime, flags, bytes, data)) == 0) {	\
+			stats.total_items++;						\
+			add_iov("STORED\r\n", 8);					\
+		} else {								\
+			add_iov("SERVER_ERROR ", 13);					\
+			add_iov(error_codes_desc_strs[ret_code],			\
+				strlen(error_codes_desc_strs[ret_code]));		\
+			add_iov("\r\n", 2);						\
+		}									\
+	}										\
 })
 
 	%%{
@@ -372,10 +377,15 @@ memcached_dispatch(struct box_txn *txn)
 			if (tuple == NULL || tuple->flags & GHOST || expired(tuple)) {
 				add_iov("NOT_FOUND\r\n", 11);
 			} else {
-				if (delete(txn, key) == 0)
+				u32 ret_code;
+				if ((ret_code = delete(txn, key)) == 0)
 					add_iov("DELETED\r\n", 9);
-				else
-					add_iov("SERVER_ERROR\r\n", 14);
+				else {
+					add_iov("SERVER_ERROR ", 13);
+					add_iov(error_codes_desc_strs[ret_code],
+						strlen(error_codes_desc_strs[ret_code]));
+					add_iov("\r\n", 2);
+				}
 			}
 		}