Probe idle HTTP/1 connections before reuse - #230
Merged
Conversation
Use IO::Stream::Readable#peek_partial to process pending EOF and TLS close notifications before the connection pool reuses an idle HTTP/1 connection. Reject unexpected application bytes on an idle connection and treat transport failures during the probe as non-viable. Raise the io-stream dependency to 0.14 and cover clean TLS shutdown, abrupt disconnects, pending application data, inactive connections, missing streams, and an end-to-end non-idempotent POST after the server closes the pooled connection. Closes #223
samuel-williams-shopify
marked this pull request as ready for review
July 27, 2026 05:53
ioquatix
reviewed
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR
Probe idle HTTP/1 connections before the pool reuses them, so a peer shutdown that is already observable does not turn a non-idempotent request into an
EOFError.Context
An underlying socket can be readable because EOF or a TLS
close_notifyis pending. The existingReadable#readable?check intentionally does not consume the layered transport, so it cannot distinguish that state from a connection on which another read may eventually succeed.io-stream0.14 introducedReadable#peek_partial, which can perform one non-blocking read through TLS while preserving any application bytes in the stream buffer. HTTP/1 can safely use that primitive while idle because it has no concurrent reader.Closes #223
Changes
io-stream ~> 0.14peek_partial(1)in the HTTP/1 connection viability checkclose_notify, abrupt TLS disconnect, pending buffered application data, inactive connections, and absent streamsTophatting
The end-to-end regression was also run against the previous
viable?implementation and reproduced the reportedEOFErrorfromProtocol::HTTP1::Connection#read_response_line.Validation on Ruby 3.4.4:
bundle exec sus test/async/http/ssl.rb test/async/http/protocol/http1/connection.rb— 8 tests, 12 assertionsbundle exec bake test— 236 passed, 3 skipped, 34,728 assertionsbundle exec rubocop— 127 files, no offensesbundle exec bake decode:index:coverage lib— 292/292 public definitions documentedviable?