Skip to content

Apply timeout to persistent connections - #36

Open
camfilho wants to merge 2 commits into
serpapi:masterfrom
camfilho:fix-persistent-timeout
Open

camfilho wants to merge 2 commits into
serpapi:masterfrom
camfilho:fix-persistent-timeout

Conversation

@camfilho

@camfilho camfilho commented Oct 12, 2026 •

Copy link
Copy Markdown

The persistent socket is built with HTTP.persistent(...) alone, so the timeout option only reaches the non-persistent path in execute_request. Since persistent: true is the default, most clients make requests with no client-side timeout:

client = SerpApi::Client.new(api_key: 'x', timeout: 5)
client.socket.default_options.timeout_options # => {}

A request on a half-open connection then waits on the OS TCP retry limit, and a server that accepts but never responds can block the caller indefinitely.

Change: build the socket with HTTP.timeout(timeout).persistent(...). http.rb resets the global timeout counter after each response on a kept-alive connection, so long-lived clients are not cut off after timeout seconds in total.

The README options table describes timeout as applying to non-persistent requests only, while its examples pass timeout: with persistent: true. I've updated the table to match; if the current behavior is intended, happy to close this.

Tested:

  • New spec applies the timeout to the persistent socket fails before the change (got: {}) and passes after. It makes no network call.
  • rubocop lib spec: no offenses.
  • Two live searches on one persistent client with timeout: 30: both succeeded over the same socket, and close returned :clean.

This fix was also part of #22, which was closed by its author along with several unrelated changes. This PR only carries the timeout fix.

The persistent socket was built with HTTP.persistent alone, so the
timeout option only reached the non-persistent path. With persistent
mode on by default, requests ran with no client-side timeout.
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