diff --git a/clickhouse/base/socket.cpp b/clickhouse/base/socket.cpp index dad6e4d3..3ece9487 100644 --- a/clickhouse/base/socket.cpp +++ b/clickhouse/base/socket.cpp @@ -1,6 +1,7 @@ #include "socket.h" #include "singleton.h" #include "../client.h" +#include "../exceptions.h" #include #include @@ -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"); diff --git a/ut/socket_ut.cpp b/ut/socket_ut.cpp index ee531544..c275a07e 100644 --- a/ut/socket_ut.cpp +++ b/ut/socket_ut.cpp @@ -129,3 +129,49 @@ TEST(Socketcase, connecttimeout) { // auto input = socket.makeInputStream(); // input->Read(buffer, sizeof(buffer)); //} + +#if !defined(_win_) +# include +# include +# include + +// 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_)