Skip to content

fix: apply the client timeout to reading, not just connecting - #8

Merged
jvmvik merged 2 commits into
masterfrom
fix/homedepot-timeout
Aug 16, 2026
Merged

fix: apply the client timeout to reading, not just connecting#8
jvmvik merged 2 commits into
masterfrom
fix/homedepot-timeout

Conversation

@jvmvik

@jvmvik jvmvik commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Why CI keeps failing

HomeDepotTest has failed 4 of the last 7 runs, always identically:

serpapi.SerpApiException: java.net.http.HttpTimeoutException: request timed out
	at serpapi.SerpApiHttp.get(SerpApiHttp.java:93)

Not flake in the usual sense, and nothing to do with the recent error-handling or markdown work — the home_depot engine intermittently takes longer than a minute, and the request is cut off at 60s.

Root cause

SerpApi.timeout was applied to one timeout only:

public Integer timeout = 60000;
...
this.client.setHttpConnectionTimeout(this.timeout);   // read timeout never set

So the read timeout — the one actually expiring — was never configured and kept SerpApiHttp's own 60s default. That is backwards: a 60s connection timeout is meaningless, since connecting takes milliseconds, while 60s to read is tight for engines that scrape.

Change

Apply the timeout to both, and raise the default to 120s.

This is a library fix, not a test fix. Any caller searching a slow engine hit the same wall, and setHttpReadTimeout existed but nothing wired it up. The only behavioral change for users is that a hung request now waits 120s instead of 60s before failing; anyone wanting the old bound can still set timeout themselves.

TimeoutTest covers both halves and runs offline.

Also: the README generator was broken

snippet in README.md.erb sliced examples at a hardcoded lines[23..stop-1], assuming every example body starts at line 24. Quarantining GoogleEventsTest shifted that file, so regenerating pulled the @Ignore and its explanation into the docs as if they were usage code:

  @Ignore("google_events engine returns 'Fully empty' for all queries; not a client bug")
  @Test
  public void search() throws SerpApiException {
// skip test if no api_key provided
if("your_api_key" == null)

Now anchored on the // setup serpapi client marker, which every one of the 21 example tests has exactly once. README regenerated: the google_events example is correct again, and a stray blank line in the google play example is gone.

Verification

Offline tests pass locally. The real check is whether HomeDepotTest stays green across CI runs — one green run does not prove much given it has passed 3 of 7 on its own.

🤖 Generated with Claude Code

jvmvik and others added 2 commits August 16, 2026 07:08
HomeDepotTest failed 4 of the last 7 CI runs, always the same way:

  SerpApiException: java.net.http.HttpTimeoutException: request timed out

SerpApi.timeout was wired only to setHttpConnectionTimeout, so the read
timeout was never configured and kept SerpApiHttp's 60s default. That is
backwards: a 60s connection timeout is meaningless, since connecting
takes milliseconds, while 60s to read is tight for engines that scrape.
Apply the timeout to both, and raise the default to 120s, which the
home_depot engine regularly needs.

This is a library fix rather than a test fix: any caller searching a slow
engine hit the same timeout.

Also repair the README snippet helper, which sliced examples at a
hardcoded lines[23..], assuming every example body starts at line 24.
Quarantining GoogleEventsTest shifted that file and pulled the @ignore
and its comment into the docs as if they were usage code. Anchor on the
"// setup serpapi client" marker instead, which every example test has
exactly once, and regenerate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Created accidentally by a shell redirect run from the wrong directory.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jvmvik
jvmvik merged commit 02fb8d9 into master Aug 16, 2026
1 check passed
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