feat(low-code): opt-in TCP keepalive and request timeouts on HttpRequester

[ZaneHyattAB]

Summary

Nothing changes for connectors that don't set these fields. A default HttpClient / HttpRequester still mounts main's plain HTTPAdapter on https:// only, sends no timeout kwarg, and opens sockets without SO_KEEPALIVE. Regression tests pin all three.

This PR adds three optional HttpRequester fields, all off by default:

requester:
  type: HttpRequester
  connect_timeout_in_seconds: 30   # unset = no connect timeout
  read_timeout_in_seconds: 300     # unset = no read timeout (max silence between bytes, not total duration)
  use_tcp_keepalive: true          # default false; 60 s idle / 10 s interval / 6 probes

Data path: model_to_component_factory → HttpRequester → HttpClient(request_timeout=(connect, read) | None, use_tcp_keepalive=bool).

• use_tcp_keepalive=True mounts TcpKeepaliveHTTPAdapter (new, streams/http/tcp_keepalive.py) on both https:// and http://, for cached and uncached sessions. It sets the socket options on the pool manager and on proxy_manager_for, because requests doesn't forward pool kwargs to proxy managers.
• Options are probed once per OS (TCP_KEEPIDLE on Linux/Windows, TCP_KEEPALIVE on macOS). Unsupported ones are dropped, never raised. The result is cached as an immutable tuple.
• Injected session= objects are never modified.
• request_timeout is added to request_kwargs only when set, and never overrides a caller-supplied timeout.

Why opt-in. The previous version of this PR shipped fleet-wide default timeouts. Review pointed out that this breaks manifest-only connectors with slow synchronous endpoints, and that they had no way to override it. This version adds the knobs only. A fleet-wide default can be decided separately later.

Evidence (local network lab: client → NAT router with a 1200 s conntrack idle timeout → server).

Scenario	Result
Server stays silent for 21 min mid-response	On main: hangs past 30 min with no exception. With keepalive: succeeds.
Connection blackholed mid-response	With keepalive: fails in about 125 s.
Slow trickle (1 byte / 600 s for 30 min)	With keepalive: unaffected.

GCP Cloud NAT's default TCP established idle timeout is 1200 s, and it applies to connections idle while an endpoint takes long to respond (Cloud NAT docs). AWS NAT Gateway drops idle connections after 350 s and recommends keepalive below that (AWS docs).

What it does not cover:

• A server that is alive but silent needs read_timeout_in_seconds; keepalive won't fail it.
• A stalled TLS handshake is bounded only by connect_timeout_in_seconds.
• A dead upstream behind a forward proxy: keepalive only covers the client→proxy leg.
• A connection that dies while a streamed body (stream=True) is being decoded raises outside HttpClient._send's retry loop.
• The raw requests calls in OAuth token refresh and the declarative session-token authenticator don't go through HttpClient, so they get neither knob.

No error-mapping change. Timeouts keep today's classification: they retry via the unexpected-exception fallback (RETRY + system_error).

Generated model. poe build (dagger) ran. Its output also rewrites ~420 unrelated lines of declarative_component_schema.py (conint(...)/confloat(...) annotations, class reordering) and then fails mypy airbyte_cdk with 10 valid-type errors. That happens on plain main with no schema change too, so it's pre-existing generator drift. The three fields were therefore added by hand to the committed file in its existing Field(..., gt=0) style, with the same content the generator emits.

Dependency. urllib3 is now declared directly (urllib3 = "*", no new constraint; it already came in through requests), because tcp_keepalive.py imports it and deptry flags transitive imports (DEP003).

Follow-ups (not in this PR):

• Classify timeouts as transient_error.
• Keepalive and timeouts for the raw auth calls.
• A possible fleet-wide default, as a separate decision, once source-stripe has run with it.

First consumer: source-stripe will opt in (connect_timeout_in_seconds: 30, read_timeout_in_seconds: 300, use_tcp_keepalive: true) in a separate airbyte PR after release.

Suggested reviewers: Daryna Ishchenko (@darynaishchenko), Maxime Carbonneau-Leclerc (@maxi297) (not assigned).

Link to Devin session: https://app.devin.ai/sessions/56ec2115e89a41e99baa6ae024af93b2
Open in Devin Desktop: https://app.devin.ai/desktop/session/56ec2115e89a41e99baa6ae024af93b2?variant=devin
Requested by: ZaneHyattAB

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[github-actions]

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1789662799-http-client-timeouts#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1789662799-http-client-timeouts

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

• /autofix - Fixes most formatting and linting issues
• /poetry-lock - Updates poetry.lock file
• /test - Runs connector tests with the updated CDK
• /prerelease - Triggers a prerelease publish with default arguments
• /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
• /poe <command> - Runs any poe command in the CDK environment

📚 Show Repo Guidance

Helpful Resources

• CDK API Reference

📝 Edit this welcome message.

[Copilot]

🟡 Changes recommended

Body-read and streamed-response timeouts are not consistently normalized or retried.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds configurable HTTP connect/read timeouts and maps timeout failures to transient retries.

Changes:

• Adds default, environment, client, and request-level timeout configuration.
• Adds timeout error mappings and retry behavior.
• Adds unit and socket-based timeout tests.

File summaries

File	Summary
unit_tests/sources/streams/http/test_http_client.py	Tests timeout defaults, overrides, and retry scenarios.
unit_tests/sources/streams/http/error_handlers/test_http_status_error_handler.py	Tests timeout error resolution.
unit_tests/sources/declarative/requesters/error_handlers/test_default_error_handler.py	Tests declarative timeout handling.
airbyte_cdk/sources/streams/http/http_client.py	Applies configured request timeouts.
airbyte_cdk/sources/streams/http/error_handlers/default_error_mapping.py	Maps timeout exceptions to transient retries.

Review details

Suppressed comments (1)

airbyte_cdk/sources/streams/http/error_handlers/default_error_mapping.py:41

• These exact-class mappings do not cover a read timeout that occurs after response headers. With stream=False, requests eagerly reads response.content inside Session.send and converts urllib3's ReadTimeoutError during that read into requests.exceptions.ConnectionError; the handler then misses this key and returns the generic system_error resolution. A server that sends headers and then stalls therefore does not get the promised transient_error classification. The timeout cause needs to be normalized/inspected before exact-class lookup, with a body-stall regression test.

    ReadTimeout: ErrorResolution(
        response_action=ResponseAction.RETRY,
        failure_type=FailureType.transient_error,
        error_message="The request timed out while waiting for the server to respond. Exception: requests.exceptions.ReadTimeout",

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

PyTest Results (Fast)

5 063 tests  +38   5 051 ✅ +38   10m 5s ⏱️ + 1m 2s
    1 suites ± 0      12 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit a8e017c. ± Comparison against base commit c43181d.

♻️ This comment has been updated with latest results.

[github-actions]

PyTest Results (Full)

5 066 tests   5 054 ✅  12m 1s ⏱️
    1 suites     12 💤
    1 files        0 ❌

Results for commit a8e017c.

♻️ This comment has been updated with latest results.

[Daryna Ishchenko (darynaishchenko)]

The change and the defaults look right to me. The fleet already uses the same 300 s read timeout independently (source-iterable (60, 300), source-marketo (30, 300)), and both halves of the change are load-bearing: dropping the mapping entries fails 5 tests.

🟠 medium: the token endpoint can still hang forever (not in the diff, so it's here instead of inline)

The PR bounds HttpClient._send(), but these call sites bypass it and still pass no timeout:

• abstract_oauth.py:411: OAuth token refresh, the first request of every sync for every OAuth connector
• declarative/auth/token.py:163 / :250: SessionTokenAuthenticator login and validation
• unstructured_parser.py:340: API-based file parsing

A token endpoint that accepts the connection and never answers parks the thread exactly as before, so the TL;DR ("one server that stopped answering could park a connector thread forever") still holds for auth. I'd fix this in this PR, because it's one argument per site:

timeout=default_request_timeout(),

The OAuth refresh already retries on ConnectionError / ConnectTimeout / ReadTimeout (abstract_oauth.py:379-386). That retry can't fire today because a silent endpoint never raises; with a timeout it plugs straight in. The session-token calls have no retry, so there it turns a hang into a hard error, which is still strictly better.

[devin-ai-integration]

Done in 64c851b: timeout=default_request_timeout() on all four sites (abstract_oauth.py refresh request, token.py get_new_session_token + is_valid_session_token, unstructured_parser.py remote parse), each with a test asserting the kwarg. To avoid the import cycle (http_client → requests_native_auth.protocols → package __init__ → oauth → abstract_oauth), the constants and default_request_timeout() moved to a new stdlib-only airbyte_cdk/sources/streams/http/request_timeout.py and are re-exported from http_client.py, so existing imports keep working. Left cli/_secrets.py and utils/docker.py alone (dev tooling, not connector runtime). PR body updated.

[ZaneHyattAB]

Both points are in as of 0c915f3b, and the parser one turned up something I'd rather raise myself than have you find.

The four requests.* calls on the sync path that bypassed HttpClient now take the default, with tests. Being precise about scope: the CLI and docker helpers still have no timeout, nltk.download at import time is untouched, and the storage clients used by file-based connectors live in the connectors, so they're out of scope here. The constants live in a small stdlib-only request_timeout.py so the auth and file-based modules don't have to import HttpClient.

The parser call is where a plain default would have been a regression, so it is now connect-bounded and read-unbounded: _read_file_remotely posts with timeout=connect_only_request_timeout() = (connect, None), where connect is the same 30 s default and still honours AIRBYTE_HTTP_CONNECT_TIMEOUT_SECONDS. The hosted partition endpoint is synchronous, so a read timeout there is really a cap on document processing time. hi_res runs at roughly seconds per page, a 39-page PDF on the hosted API has been measured at about 5m30s, and Unstructured documents no processing cap, no size limit and no recommended client timeout. On top of that, ReadTimeout is a RequestException, so the max_tries=5 backoff wrapper would have re-uploaded the whole document up to five times before failing a file that used to succeed. So an unreachable endpoint now fails at connect in 30 s, while a stall after the connection is accepted is still unbounded on this one call. That residual is stated in the body, with a per-format read-timeout field on APIProcessingConfigModel as the follow-up. Tests cover the (30.0, None) shape, the env override, and a hanging connect failing fast.

Kept the three explicit mapping entries, since you're right that the lookup matches the exact class. One nuance worth recording: before this PR timeouts weren't falling through to the RequestException row either, they hit the unexpected-exception fallback and already retried, as system_error. So this changes the failure type and the message, not the retry counts. Making the lookup MRO-aware would fix it properly but reclassifies every network error fleet-wide, so that stays its own follow-up.

Your stream=True probe corrected the body, with one addition: CompositeRawDecoder reads response.raw and raises the raw urllib3.exceptions.ReadTimeoutError you saw, while IterableDecoder goes through iter_lines and surfaces requests.exceptions.ConnectionError. Both are outside the retry loop, one connection, no retry either way. For stream=False a mid-body stall raises inside Session.send and is retried. I also qualified timeout=None in the body: it disables per request only. Per client, HttpClient(request_timeout=None) means use the default, and manifests have no per-requester override, only the env vars.

On the red checks, neither comes from this diff. destination-motherduck is a MotherDuck token auth failure inside the test job. source-google-drive fails on SourceGoogleDrive.__init__() missing 3 required positional arguments against the standard test harness. In the last 150 Test Connectors runs that check has executed three times and failed all three, including on a branch whose diff contains no timeout changes at all. It only runs when file_based/ changes, which is why it showed up here.

[Daryna Ishchenko (darynaishchenko)]

Thanks for this — the fix is right in principle (an unbounded hang is worse than any timeout) and it is well tested. What I want to discuss before merging is the approach: one fixed value for every connector, with no way to change it from a manifest.

The read timeout fires when the server sends nothing for 300 s. For most APIs that is fine, but synchronous export/report endpoints that compute server-side before the first byte can legitimately stay silent longer. Python connectors can override the value (request_kwargs() or HttpClient(request_timeout=...)). Manifest-only and Builder connectors — the larger population, and the one that picks up new SDM versions automatically — cannot: there is no schema field, HttpRequester builds HttpClient without request_timeout and passes only {"stream": ...}, and the env-var override was removed in 20ec792. For such a connector a request that used to succeed now times out, retries, and fails as transient_error, with no fix short of a custom component or an SDM pin.

The PR already lists the manifest-level field as a follow-up. I would like it in this PR, or merged before the release that contains this one, so the default ships together with the way to change it:

• declarative_component_schema.yaml: an optional timeout on HttpRequester (either a single request_timeout accepting (connect, read) or separate connect_timeout / read_timeout), with title/description/examples; regenerate the models with /poe build.
• model_to_component_factory.py: pass it through HttpRequester into HttpClient(request_timeout=...).
• A test showing a manifest that sets it reaches session.send with that value, and one showing a manifest without it gets the default.

On the value itself: 300 s is a reasonable first default (iterable, marketo and google-ads chose the same independently), but we have no fleet data on how many endpoints stay silent longer. A /prerelease plus a pinned run on a connector with long synchronous endpoints would be worth doing before release.

[Daryna Ishchenko (darynaishchenko)]

Approving. The opt-in redesign resolves my earlier request-changes point: there is no fleet-wide default any more, and a connector that sets none of the three fields behaves exactly as before (tests pin the adapter type, pool sizes and the missing timeout kwarg).

What's done well

• Keepalive fixes the NAT-drop hang without risking slow-but-alive endpoints. The proxy_manager_for override closes a real requests gap.
• The tests are hermetic and fail fast. I mutated 7 production lines locally (injection removed, caller-timeout precedence flipped, http:// mount dropped, proxy override removed, factory/requester wiring dropped), and each one broke a test within seconds.
• The hand-edited generated model matches what datamodel-codegen 0.26.3 emits (PositiveFloat ≡ Field(gt=0)).

Rollout

• No connector in the Test Connectors matrix opts in, so the source-stripe follow-up is the first real exercise of this path. A /prerelease plus a pinned stripe run before any wider adoption would close that gap.
• Note for the stripe PR: read_timeout_in_seconds: 300 would fail the same 21-minute-silence case that keepalive alone survives in your lab table.

Three optional 🟡 nits inline; none blocks the merge.

[ZaneHyattAB]

Daryna Ishchenko (@darynaishchenko) Thanks for the review. I added the retry-window note and a one-time warning on keepalive fallback in a8e017c, and I'll do config interpolation for the timeouts as a non-breaking follow-up.