Add request timeout - #24
Conversation
Sorry, something went wrong.
…pecific arguments to API calls, along with updating documentation and removing the README template.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
DetailsYou are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Sorry, something went wrong.
| except requests.exceptions.ConnectionError as e: | ||
| raise HTTPConnectionError(e) | ||
| except requests.exceptions.Timeout as e: | ||
| raise TimeoutError(e) |
There was a problem hiding this comment.
Timeout handler unreachable for connection timeouts
High Severity
The except requests.exceptions.ConnectionError clause is placed before except requests.exceptions.Timeout. Since requests.exceptions.ConnectTimeout is a subclass of both ConnectionError and Timeout, any connection timeout will be caught by the first handler and raised as HTTPConnectionError instead of TimeoutError. The Timeout except clause needs to come first so that timeouts are properly distinguished from other connection errors.
Sorry, something went wrong.
|
|
||
| # Use the default timeout if one was provided to the client. | ||
| if self.timeout and "timeout" not in kwargs: | ||
| kwargs["timeout"] = self.timeout |
There was a problem hiding this comment.
Truthiness check ignores falsy timeout values
Low Severity
The condition if self.timeout uses a truthiness check, which means a timeout value of 0 (or 0.0) would be treated as falsy and not applied. The check needs to be if self.timeout is not None to correctly distinguish between "no timeout set" and "timeout explicitly set to zero."
Sorry, something went wrong.
| request_kwargs = {} | ||
| for key in ["timeout", "proxies", "verify", "stream", "cert"]: | ||
| if key in kwargs: | ||
| request_kwargs[key] = kwargs.pop(key) |
There was a problem hiding this comment.
Duplicated request kwargs extraction across four methods
Low Severity
The same request_kwargs extraction loop (iterating over ["timeout", "proxies", "verify", "stream", "cert"] and popping from kwargs) is copy-pasted identically in search, search_archive, locations, and account. Extracting this into a small helper method would reduce duplication and ensure the list of forwarded keys stays consistent if it ever changes.
Additional Locations (2)
Sorry, something went wrong.
implement request #19
Note
Medium Risk
Changes how all HTTP requests are issued by introducing default/overridable timeouts and a new TimeoutError, which can affect runtime behavior and error handling across consumers.
Overview
Adds first-class request timeout support by introducing a timeout option on Client/HTTPClient, defaulting it into outbound requests, and allowing per-call overrides via search, search_archive, locations, and account.
Introduces a new serpapi.TimeoutError mapped from requests.exceptions.Timeout, updates docs/changelog and version to 0.1.6, removes the old README.md.erb, and adds tests validating default vs overridden timeout behavior.
Written by Cursor Bugbot for commit a0bb5ca. This will update automatically on new commits. Configure here.