Fix stale errno handling when recv returns 0

clickhouse/base/socket.cpp

@@ -1,6 +1,7 @@
 #include "socket.h"
 #include "singleton.h"
 #include "../client.h"
+#include "../exceptions.h"
 
 #include <assert.h>
 #include <stdexcept>
@@ -436,7 +437,12 @@ size_t SocketInput::DoRead(void* buf, size_t len) {
     }
 
     if (ret == 0) {
-        throw std::system_error(getSocketErrorCode(), getErrorCategory(), "closed");
+        // Clean peer close (EOF) before the requested number of bytes
+        // arrived: the underlying `recv()` succeeded, so this is not a
+        // syscall error. The decoder expected more protocol data and the
+        // connection ended instead, so surface this as a truncated-data
+        // / protocol decoding failure rather than a system_error.
+        throw ProtocolError("connection closed by peer while reading");
     }
 
     throw std::system_error(getSocketErrorCode(), getErrorCategory(), "can't receive string data");

ut/socket_ut.cpp

@@ -129,3 +129,49 @@ TEST(Socketcase, connecttimeout) {
 //    auto input = socket.makeInputStream();
 //    input->Read(buffer, sizeof(buffer));
 //}
+
+#if !defined(_win_)
+#   include <cerrno>
+#   include <sys/socket.h>
+#   include <unistd.h>
+
+// Regression test for issue #487.
+//
+// On a clean peer close, `recv()` returns 0, which is EOF, not a syscall
+// error. POSIX does NOT require `errno` to be set when `recv()` returns 0,
+// so reading `errno` at that point would yield a stale value from a previous
+// syscall. The proper representation is a protocol-level / truncated-data
+// failure, not a `std::system_error`: the underlying `recv()` succeeded;
+// the decoder expected more protocol bytes and the connection ended instead.
+//
+// The fix throws `clickhouse::ProtocolError` on `recv() == 0`. The other
+// `recv() < 0` path still uses `std::system_error` because that is an
+// actual syscall failure. This test drives a clean close via `socketpair(2)`
+// and asserts the resulting exception is exactly `ProtocolError` with a
+// message indicating a peer-closed connection.
+TEST(Socketcase, recvReturnsZeroReportsProtocolErrorNotStaleErrno) {
+    int sv[2];
+    ASSERT_EQ(0, ::socketpair(AF_UNIX, SOCK_STREAM, 0, sv));
+
+    SocketInput input(sv[0]);
+    // Close the peer side: the next `recv()` on sv[0] returns 0 (EOF).
+    ::close(sv[1]);
+
+    char buf[16];
+    try {
+        input.Read(buf, sizeof(buf));
+        ::close(sv[0]);
+        FAIL() << "expected ProtocolError on clean peer close";
+    } catch (const ProtocolError& e) {
+        ::close(sv[0]);
+        const std::string what = e.what();
+        EXPECT_NE(what.find("closed"), std::string::npos)
+            << "expected message to mention 'closed', got: " << what;
+    } catch (const std::system_error& e) {
+        ::close(sv[0]);
+        FAIL() << "recv()==0 must be reported as ProtocolError, not "
+                  "std::system_error (got errno-style code "
+               << e.code().value() << "); stale errno regression";
+    }
+}
+#endif  // !defined(_win_)