Skip to content

Don't wait for ReadyForQuery after FATAL errors#765

Open
v0idpwn wants to merge 1 commit intomasterfrom
fix/handle-fatal-errors
Open

Don't wait for ReadyForQuery after FATAL errors#765
v0idpwn wants to merge 1 commit intomasterfrom
fix/handle-fatal-errors

Conversation

@v0idpwn
Copy link
Member

@v0idpwn v0idpwn commented Mar 18, 2026

When PostgreSQL sends a FATAL or PANIC ErrorResponse, it closes the connection immediately without sending ReadyForQuery.

Postgrex unconditionally waited for ReadyForQuery, which would hit tcp_closed and return a generic disconnect error, discarding the original error message.

I've included in the PR a test that would fail with the previous implementation. The query would return a

{:error,
   %DBConnection.ConnectionError{
     message: "tcp recv: closed",
     severity: :error,
     reason: :closed
   }}

Comment on lines +3071 to 3077
defp error_ready(s, _status, %Postgrex.Error{postgres: %{severity: severity}} = err, buffer)
when severity in ["FATAL", "PANIC"] do
%{connection_id: connection_id} = s
{:disconnect, %{err | connection_id: connection_id}, %{s | buffer: buffer}}
end

defp error_ready(s, status, %Postgrex.Error{} = err, buffer) do
Copy link
Member Author

@v0idpwn v0idpwn Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love that this is called error_ready but couldn't come up with a better name for the new implementation.

Copy link
Member Author

@v0idpwn v0idpwn Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe instead of returning {:disconnect, ...} immediately, it should wait for ReadyForQuery or for the socket closed to be closed (which is what libpq does, if I'm not mistaken), then return the error.

When PostgreSQL sends a FATAL or PANIC ErrorResponse, it closes the
connection immediately without sending ReadyForQuery.

Postgrex unconditionally waited for ReadyForQuery, which would hit
tcp_closed and return a generic disconnect error, discarding the
original FATAL error.
@v0idpwn v0idpwn force-pushed the fix/handle-fatal-errors branch from be2740a to b3eb658 Compare March 18, 2026 00:23
Copy link
Member Author

@v0idpwn v0idpwn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SimpleConnection may need some error handling tweak to handle this too. I'll look into that tomorrow 🙏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant