Pgx: stdlib Rows.Next() propagates io.EOF errors from pgx Rows.Err()

[nick-jones]

I'm not sure if this issue (assuming it is one) lies here or in pgconn.

The Go standard library expects an io.EOF be returned by drivers when there are no more rows to be read (under non-error conditions). The stdlib package correctly returns io.EOF when no error has occured during reading. If an error is returned during reading, this is then returned to the standard library verbatim.

We are experiencing circumstances where pgx Rows.Err() is returning io.EOF but the reading of rows _did not complete_. The error is being returned to the standard library at https://github.com/jackc/pgx/blob/69048c281c7cc134b9dd4ed3883e1c639c211d75/stdlib/sql.go#L473
The consequence of this is that from the clients perspective, the row reading _did_ complete, but actually it only got part way through the result set.. this can be quite disasterous.

The circumstances are reproducable, although perhaps a little off-piste: we are sending a SIGKILL to a CockroachDB instance (which uses pg protocol).. that is running inside a kubernetes cluster (so dockerised). If a client is busy reading rows whilst this occurs, and happens to be connected to that particular instance, the reading stops short and no error is returned. Unfortunately I don't have an isolated reproduction case for this, I could possibly try and piece something together if wanted.

So, to trace this through a bit further...

• pgx connRows.Next() calls pgconn ResultReader.NextRow() (https://github.com/jackc/pgx/blob/69048c281c7cc134b9dd4ed3883e1c639c211d75/rows.go#L176)
• pgconn ResultReader.NextRow() calls ResultReader.receiveMessage() (https://github.com/jackc/pgconn/blob/e7dd01e064b5caf31bd290db23fadd13e60f8cd8/pgconn.go#L1297). If an error is returned by this function false is returned by NextRow(), stopping any further iteration
• pgconn ResultReader.receiveMessage() calls PgConn.receiveMessage() (https://github.com/jackc/pgconn/blob/e7dd01e064b5caf31bd290db23fadd13e60f8cd8/pgconn.go#L1364). If this returns an error, this is set into ResultReader.err (and this is the error value that is returned to the standard library) via ResultReader.concludeCommand (https://github.com/jackc/pgconn/blob/e7dd01e064b5caf31bd290db23fadd13e60f8cd8/pgconn.go#L1370 / https://github.com/jackc/pgconn/blob/e7dd01e064b5caf31bd290db23fadd13e60f8cd8/pgconn.go#L1398)
• Assuming bufferingReceive is false, pgconn PgConn.receiveMessage() calls Frontend.Receive() - this is implemented by pgproto3 Frontend. The error from this call is returned verbatim.
• pgproto3 Frontend.Receive() calls ChunkReader.Next() (https://github.com/jackc/pgproto3/blob/master/frontend.go#L63-L77)
• The implementation of ChunkReader.Next() calls ChunkReader.appendAtLeast() (https://github.com/jackc/chunkreader/blob/2c463c0e7d0d0876517f087ce2cce66a46182141/chunkreader.go#L83-L87). That then calls io.ReadAtLeast(), which we are seeing return io.EOF as an error in circumstances where not all rows have been received. I believe the underlying reader for what we are dealing with in our example is standard library net TCPConn wrapped with crypto/tls Client.

My understanding of io.EOF is that it should be returned on graceful end of input - which perhaps seems a bit odd given we are sending a SIGKILL to the server. tcpdump shows a FIN / ACK is being sent by the server, which maybe suggests the kernel is slamming to door on the client, the client sees this as graceful? Quite unsure.

Either way, should the libraries not protect against this occurring? There are a couple of things that seemingly could help here..

1. I'm not well versed in postgres protocol, but it seems you'd expect a CommandComplete to come through, which it clearly won't be in this case. pgconn could check for io.EOF and return io.UnexpectedEOF if the command did not complete?
2. Alternatively could the stdlib package do something similar? If io.EOF is returned, return io.UnexpectedEOF the the caller.

[tsingson]

i had same issue when i use conn.Conn().WaitForNotification(ctx).

[jackc]

Thanks for the detailed report. As I understand the PostgreSQL wire protocol, there are only two cases where an EOF is expected.

1. After receiving a FATAL ErrorResponse from the server
2. After sending a Terminate message to the server

So it does seem fair to translate any io.EOF into an io.UnexpectedEOF. The question is where to make that transformation. I think pgproto3.Frontend is the right layer. I can't think of any time in the protocol when io.EOF would be a valid expectation to be returned from pgproto3.Frontend.Receive().

I've just pushed a change jackc/pgproto3@595780be0f9f581451a23a5151b77f782202ad72 that I believe should resolve this issue. Update to this commit and let me know if it works. If so I can tag a release.

go get github.com/jackc/pgproto3/v2@595780be0f9f581451a23a5151b77f782202ad72

[nick-jones]

Thanks @jackc. I'm unable to reproduce the issue after pulling in that commit, so from my perspective that suitably resolves it.

[jackc]

This fix is included in the just released v4.3.0.