Don't close connections on clean protocol-level SERVER_ERROR responses

[mapno]

Summary

A SERVER_ERROR <msg>\r\n reply is a complete, well-formed protocol line: the server keeps the connection open and in sync (it even swallows the data block on storage errors). Closing our side causes needless reconnect churn, e.g. on object too large for cache or OOM bursts — the worst possible time to add TCP handshake load, since the server is already under memory pressure.

Why this is safe: memcached keeps the connection open on these errors

Verified against memcached 1.6.31 source. For the ASCII protocol (the only one this client speaks), every common SERVER_ERROR is written with out_string(), which transitions to conn_new_cmd — it does not close the connection. On storage errors the server additionally swallows the in-flight data block, so both sides stay in sync:

• SERVER_ERROR object too large for cache / SERVER_ERROR out of memory storing object — replies, then swallows the data block and continues:
  proto_text.c#L2045-L2063

  out_of_memory(c, "SERVER_ERROR out of memory storing object");
  ...
  /* swallow the data line */
  conn_set_state(c, conn_swallow);
  c->sbytes = vlen;

• SERVER_ERROR Out of memory during read (chunk alloc failure mid-read of a set payload) — same swallow pattern:
  memcached.c#L3240-L3250
• SERVER_ERROR out of memory writing get response — get requests carry no data block, so the connection is already in sync; the server just replies and moves on:
  proto_text.c#L702-L713
• out_of_memory() for ASCII clients is just out_string() (memcached.c#L1300-L1313); the conn_closing behavior lives only in the binary-protocol branch, which this client never uses.

Known exception: SERVER_ERROR out of memory reading request (failed read-buffer realloc on a huge multiget) sets close_after_write = true (memcached.c#L2494-L2496). If such a connection is pooled, the next request fails with EOF and the connection is then discarded as non-resumable — the same bounded, one-request cost as the pre-existing stale-pooled-connection races (server restarts, server-side idle timeouts) that the client already tolerates. Severe OOMs where memcached can't even allocate a response object close the connection without writing any error line, so the client sees a read error, not SERVER_ERROR, and the connection is dropped as before.

Note: error text for these responses changes from memcache: unexpected response line... to memcache: server error: <msg>.

[narqo]

Works for me 🔥

[56quarters]

Can you create a default: case for this switch like you did for another case above for consistency?

[mapno]

Good point, addressed.

[56quarters]

Doesn't seem like you updated it everywhere?

[mapno]

Fixed now, sorry!