Skip to content

fix: add default HTTP request timeout to prevent indefinite hangs #938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Alfredo Garcia (agarctfi) wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: add default HTTP request timeout to prevent indefinite hangs #938

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ddff77
fix: add default HTTP request timeout to prevent indefinite hangs
devin-ai-integration[bot] Mar 6, 2026
34cdcb1
fix: address review comments - fix type safety and add unit tests
devin-ai-integration[bot] Mar 6, 2026
d2233c5
style: apply ruff format to new code
devin-ai-integration[bot] Mar 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions airbyte_cdk/sources/streams/http/http_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ def monkey_patched_get_item(self, key): # type: ignore # this interface is a co
class HttpClient:
_DEFAULT_MAX_RETRY: int = 5
_DEFAULT_MAX_TIME: int = 60 * 10
_DEFAULT_CONNECT_TIMEOUT: int = 30
_DEFAULT_READ_TIMEOUT: int = 300
_ACTIONS_TO_RETRY_ON = {
ResponseAction.RETRY,
ResponseAction.RATE_LIMITED,
Expand Down Expand Up @@ -588,6 +590,9 @@ def send_request(
)
request_kwargs = {**request_kwargs, **env_settings}

if "timeout" not in request_kwargs:
request_kwargs["timeout"] = (self._DEFAULT_CONNECT_TIMEOUT, self._DEFAULT_READ_TIMEOUT)

Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces new default-timeout behavior but there isn't a unit test asserting (a) the default timeout is applied when request_kwargs has no timeout, and (b) an explicit timeout is respected. Adding a focused test would help prevent regressions (especially since this behavior is safety-critical for avoiding indefinite hangs).

Copilot uses AI. Check for mistakes.
response: requests.Response = self._send_with_retry(
request=request,
request_kwargs=request_kwargs,
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request_kwargs is typed as Mapping[str, Any], but this code mutates it via request_kwargs["timeout"] = .... With the repo's strict mypy settings, this will raise a type error because Mapping is read-only. Consider copying into a Dict[str, Any]/MutableMapping[str, Any] local (or changing the parameter type) before setting defaults so mypy passes.

Suggested change
request_kwargs = {**request_kwargs, **env_settings}
if "timeout" not in request_kwargs:
request_kwargs["timeout"] = (self._DEFAULT_CONNECT_TIMEOUT, self._DEFAULT_READ_TIMEOUT)
response: requests.Response = self._send_with_retry(
request=request,
request_kwargs=request_kwargs,
mutable_request_kwargs: Dict[str, Any] = {**request_kwargs, **env_settings}
if "timeout" not in mutable_request_kwargs:
mutable_request_kwargs["timeout"] = (self._DEFAULT_CONNECT_TIMEOUT, self._DEFAULT_READ_TIMEOUT)
response: requests.Response = self._send_with_retry(
request=request,
request_kwargs=mutable_request_kwargs,

Copilot uses AI. Check for mistakes.
Expand Down
Loading