proxy: send CLIENT_ERROR when necessarystaging

A few code paths were returning SERVER_ERROR (a retryable error)
when it should have been CLIENT_ERROR (bad protocol syntax).

4 files changed, 24 insertions, 18 deletions

diff --git a/proto_proxy.c b/proto_proxy.c
index e99f893..e604b40 100644
--- a/proto_proxy.c
+++ b/proto_proxy.c
@@ -499,10 +499,9 @@ void proxy_lua_ferror(lua_State *L, const char *fmt, ...) {
 }
 
 // Need a custom function so we can prefix lua strings easily.
-void proxy_out_errstring(mc_resp *resp, const char *str) {
+void proxy_out_errstring(mc_resp *resp, char *type, const char *str) {
     size_t len;
-    const static char error_prefix[] = "SERVER_ERROR ";
-    const static int error_prefix_len = sizeof(error_prefix) - 1;
+    size_t prefix_len = strlen(type);
 
     assert(resp != NULL);
 
@@ -511,21 +510,21 @@ void proxy_out_errstring(mc_resp *resp, const char *str) {
 
     // Fill response object with static string.
     len = strlen(str);
-    if ((len + error_prefix_len + 2) > WRITE_BUFFER_SIZE) {
+    if ((len + prefix_len + 2) > WRITE_BUFFER_SIZE) {
         /* ought to be always enough. just fail for simplicity */
         str = "SERVER_ERROR output line too long";
         len = strlen(str);
     }
 
     char *w = resp->wbuf;
-    memcpy(w, error_prefix, error_prefix_len);
-    w += error_prefix_len;
+    memcpy(w, type, prefix_len);
+    w += prefix_len;
 
     memcpy(w, str, len);
     w += len;
 
     memcpy(w, "\r\n", 2);
-    resp_add_iov(resp, resp->wbuf, len + error_prefix_len + 2);
+    resp_add_iov(resp, resp->wbuf, len + prefix_len + 2);
     return;
 }
 
@@ -583,7 +582,7 @@ int proxy_run_coroutine(lua_State *Lc, mc_resp *resp, io_pending_proxy_t *p, con
             mcp_resp_t *r = luaL_checkudata(Lc, 1, "mcp.response");
             _set_noreply_mode(resp, r);
             if (r->status != MCMC_OK && r->resp.type != MCMC_RESP_ERRMSG) {
-                proxy_out_errstring(resp, "backend failure");
+                proxy_out_errstring(resp, PROXY_SERVER_ERROR, "backend failure");
             } else if (r->cresp) {
                 mc_resp *tresp = r->cresp;
                 // The internal cache handler has created a resp we want to swap in
@@ -635,7 +634,7 @@ int proxy_run_coroutine(lua_State *Lc, mc_resp *resp, io_pending_proxy_t *p, con
             resp_add_iov(resp, resp->wbuf, l);
             lua_pop(Lc, 1);
         } else {
-            proxy_out_errstring(resp, "bad response");
+            proxy_out_errstring(resp, PROXY_SERVER_ERROR, "bad response");
         }
 
     } else if (cores == LUA_YIELD) {
@@ -697,7 +696,7 @@ int proxy_run_coroutine(lua_State *Lc, mc_resp *resp, io_pending_proxy_t *p, con
                     // internal run queued for extstore.
                 } else {
                     assert(res < 0);
-                    proxy_out_errstring(resp, "bad request");
+                    proxy_out_errstring(resp, PROXY_SERVER_ERROR, "bad request");
                 }
                 break;
             default:
@@ -708,7 +707,7 @@ int proxy_run_coroutine(lua_State *Lc, mc_resp *resp, io_pending_proxy_t *p, con
         WSTAT_DECR(c->thread, proxy_req_active, 1);
         P_DEBUG("%s: Failed to run coroutine: %s\n", __func__, lua_tostring(Lc, -1));
         LOGGER_LOG(NULL, LOG_PROXYEVENTS, LOGGER_PROXY_ERROR, NULL, lua_tostring(Lc, -1));
-        proxy_out_errstring(resp, "lua failure");
+        proxy_out_errstring(resp, PROXY_SERVER_ERROR, "lua failure");
     }
 
     return 0;
@@ -734,7 +733,7 @@ static void proxy_process_command(conn *c, char *command, size_t cmdlen, bool mu
             conn_set_state(c, conn_closing);
             return;
         }
-        proxy_out_errstring(c->resp, "parsing request");
+        proxy_out_errstring(c->resp, PROXY_CLIENT_ERROR, "parsing request");
         if (ret == -2) {
             // Kill connection on more critical parse failure.
             conn_set_state(c, conn_closing);
@@ -796,7 +795,7 @@ static void proxy_process_command(conn *c, char *command, size_t cmdlen, bool mu
                     conn_set_state(c, conn_closing);
                     return;
                 }
-                proxy_out_errstring(c->resp, "key too long");
+                proxy_out_errstring(c->resp, PROXY_CLIENT_ERROR, "key too long");
             } else {
                 // copy original request up until the original key token.
                 memcpy(cur, pr.request, pr.tokens[pr.keytoken]);
@@ -841,7 +840,7 @@ static void proxy_process_command(conn *c, char *command, size_t cmdlen, bool mu
             conn_set_state(c, conn_closing);
             return;
         }
-        proxy_out_errstring(c->resp, "request too long");
+        proxy_out_errstring(c->resp, PROXY_CLIENT_ERROR, "request too long");
         conn_set_state(c, conn_closing);
         return;
     }
@@ -864,7 +863,7 @@ static void proxy_process_command(conn *c, char *command, size_t cmdlen, bool mu
     WSTAT_UL(c->thread);
 
     if (active_reqs > ctx->active_req_limit) {
-        proxy_out_errstring(c->resp, "active request limit reached");
+        proxy_out_errstring(c->resp, PROXY_SERVER_ERROR, "active request limit reached");
         WSTAT_DECR(c->thread, proxy_req_active, 1);
         if (pr.vlen != 0) {
             c->sbytes = pr.vlen;
@@ -901,7 +900,7 @@ static void proxy_process_command(conn *c, char *command, size_t cmdlen, bool mu
         }
         if (c->item == NULL) {
             lua_settop(L, 0);
-            proxy_out_errstring(c->resp, "out of memory");
+            proxy_out_errstring(c->resp, PROXY_SERVER_ERROR, "out of memory");
             WSTAT_DECR(c->thread, proxy_req_active, 1);
             c->sbytes = rq->pr.vlen;
             conn_set_state(c, conn_swallow);
diff --git a/proxy.h b/proxy.h
index 87f2b7e..8b5cd37 100644
--- a/proxy.h
+++ b/proxy.h
@@ -554,7 +554,9 @@ void mcp_request_attach(lua_State *L, mcp_request_t *rq, io_pending_proxy_t *p);
 int mcp_request_render(mcp_request_t *rq, int idx, const char *tok, size_t len);
 void proxy_lua_error(lua_State *L, const char *s);
 void proxy_lua_ferror(lua_State *L, const char *fmt, ...);
-void proxy_out_errstring(mc_resp *resp, const char *str);
+#define PROXY_SERVER_ERROR "SERVER_ERROR "
+#define PROXY_CLIENT_ERROR "CLIENT_ERROR "
+void proxy_out_errstring(mc_resp *resp, char *type, const char *str);
 int _start_proxy_config_threads(proxy_ctx_t *ctx);
 int proxy_thread_loadconf(proxy_ctx_t *ctx, LIBEVENT_THREAD *thr);
 
diff --git a/proxy_internal.c b/proxy_internal.c
index 5484dc7..4da1c4a 100644
--- a/proxy_internal.c
+++ b/proxy_internal.c
@@ -250,7 +250,7 @@ static void process_get_cmd(LIBEVENT_THREAD *t, mcp_parser_t *pr, mc_resp *resp,
               pthread_mutex_unlock(&t->stats.mutex);
 
               item_remove(it);
-              proxy_out_errstring(resp, "out of memory writing get response");
+              proxy_out_errstring(resp, PROXY_SERVER_ERROR, "out of memory writing get response");
               return;
           }
       } else if ((it->it_flags & ITEM_CHUNKED) == 0) {
diff --git a/t/proxyunits.t b/t/proxyunits.t
index e76c11d..49594bf 100644
--- a/t/proxyunits.t
+++ b/t/proxyunits.t
@@ -66,6 +66,11 @@ for my $be (@mbe) {
 }
 
 {
+    print $ps "set with the wrong number of tokens\n";
+    is(scalar <$ps>, "CLIENT_ERROR parsing request\r\n", "got CLIENT_ERROR for bad syntax");
+}
+
+{
     # Test a fix for passing through partial read data if END ends up missing.
     print $ps "get /b/a\r\n";
     my $be = $mbe[0];