Skip to content

Apply timeout to persistent connections - #36

Merged
strzibny merged 3 commits into
serpapi:masterfrom
camfilho:fix-persistent-timeout
Oct 16, 2026
Merged

strzibny merged 3 commits into
serpapi:masterfrom
camfilho:fix-persistent-timeout

Conversation

@camfilho

@camfilho camfilho commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

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.

@strzibny
strzibny self-requested a review October 14, 2026 09:30
@strzibny

Copy link
Copy Markdown
Contributor

@camfilho Looks good to me, thanks for sending the fix!

One small thing for tests to run green for me:

diff --git a/spec/serpapi/client/image_api_spec.rb b/spec/serpapi/client/image_api_spec.rb
index 0fc4f5d..3503479 100644
--- a/spec/serpapi/client/image_api_spec.rb
+++ b/spec/serpapi/client/image_api_spec.rb
@@ -13,7 +13,7 @@ describe 'Image API' do
   let(:client) { SerpApi::Client.new(api_key: api_key) }

   before do
-    allow(HTTP).to receive(:persistent).and_return(socket)
+    allow(HTTP).to receive(:timeout).and_return(double(persistent: socket))
     allow(response).to receive(:flush)
   end

Please update.

@camfilho

Copy link
Copy Markdown
Contributor Author

@strzibny Thanks for the review! Applied your suggested change to image_api_spec.rb in 1c6edbf. The image API specs pass again along with the new timeout spec, and rubocop is clean.

@strzibny

Copy link
Copy Markdown
Contributor

@camfilho can you please rebase, we don't have README.md.erb any longer.

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.
@camfilho
camfilho force-pushed the fix-persistent-timeout branch from 1c6edbf to 34bbfca Compare October 15, 2026 16:00
@camfilho

Copy link
Copy Markdown
Contributor Author

Done strzibny

@strzibny
strzibny merged commit ba78dd8 into serpapi:master Oct 16, 2026
1 check failed
@strzibny

Copy link
Copy Markdown
Contributor

Thank you @camfilho!

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.

2 participants