`hackney_conn:request/5` can return `{error, invalid_state}` for a connection that was successfully checked out, if the peer has already closed it

[takakiNG]

Version

4.7.4 (also present on 4.1.0, the first tagged release after #852 was merged; likely present on any version with the current hackney_conn.erl state machine)

Summary

hackney_pool's checkout-time race between is_ready/find_available and set_owner (#850) was fixed in #852: the pool now detects a connection that closed between those two calls and transparently falls back to a fresh one, so callers never see an error from that specific path.

However, the same generic, catch-all {error, invalid_state} reply is still directly observable by callers through the request-call path itself: if a caller holds a connection Pid obtained from the pool and the peer closes it before the caller's request call reaches the connection process, hackney_conn:request/5,6,7 (and any other call routed to handle_common) returns {error, invalid_state}, with no distinction from any other unhandled call.

{error, invalid_state} is the generic reply for a call not handled in the connection's current state. It therefore conflates a caller using an operation at an invalid point in the connection lifecycle with a valid request that races a peer-initiated close after checkout. Client libraries built on top of hackney (in our case ex_aws/ex_aws_s3) cannot reasonably distinguish the two from the reply alone, and end up surfacing it straight to the application (in our case as a failed ExAws.S3.upload/3 multipart-upload initiate against S3, whose front end closes idle keep-alive connections aggressively).

Root cause

In hackney_conn.erl, a connection that observes the peer close (tcp_closed/ssl_closed/tcp_error/ssl_error) while connected transitions to a closed state:

connected(info, {tcp_closed, Socket}, #conn_data{socket = Socket} = Data) ->
    {next_state, closed, Data#conn_data{socket = undefined}};

closed/3's enter clause deliberately keeps the process alive for a short grace period instead of stopping immediately, specifically so that a caller who already obtained this Pid from the pool still gets some reply instead of the process simply being gone (see the comment referencing issue #836):

closed(enter, _OldState, #conn_data{socket = Socket, transport = Transport, pool_pid = PoolPid} = Data) ->
    case Socket of
        undefined -> ok;
        _ -> Transport:close(Socket)
    end,
    %% Pooled connections used to stop immediately here, but that made
    %% late-arriving {call, From, {request, _}} messages from workers that
    %% raced the pool checkout race a terminating gen_statem — which
    %% surfaces as `exit:{normal, _}` in the caller (issue #836). Stay
    %% alive briefly so those late calls get a proper `{error, {closed, _}}`
    %% reply via handle_common's invalid_state fallback, then stop.
    case PoolPid of
        undefined ->
            {keep_state, Data#conn_data{socket = undefined}};
        _ ->
            {keep_state, Data#conn_data{socket = undefined},
             [{state_timeout, ?CLOSED_GRACE_MS, closed_grace_expired}]}
    end;

The comment says a late {request, ...} call during this grace period gets "a proper {error, {closed, _}} reply via handle_common's invalid_state fallback" — but closed/3 has no dedicated clause for {request, ...} (or {request_async, ...}, {send_headers, ...}, etc.), so those calls fall through to closed(EventType, Event, Data) -> handle_common(EventType, Event, closed, Data), which ultimately hits the fully generic catch-all:

handle_common({call, From}, _, _State, _Data) ->
    {keep_state_and_data, [{reply, From, {error, invalid_state}}]};

This returns the bare atom {error, invalid_state}, not {error, {closed, _}} as the comment describes. So there is both:

1. A behavioral gap: a request call received while the connection is already in the closed state gets {error, invalid_state} instead of a closed/connection-terminated error.
2. A documentation/implementation mismatch: the comment's stated intent ({error, {closed, _}}) does not match the actual reply ({error, invalid_state}).

We hit this in production via ExAws.S3.upload/3 (multipart upload) against S3, which surfaced as a hard {error, invalid_state} on the underlying HTTP request.