Add default request timeouts to AuthenticatedClient

[galvana]

Description Of Changes

Add default connect and read timeouts to AuthenticatedClient.send() to prevent unbounded HTTP waits that can stall Celery workers indefinitely.

Previously, session.send() was called with no timeout parameter, meaning the requests library would wait forever for a response. This is particularly problematic for background tasks (e.g., consent webhook processing) where a hung third-party API can block a worker permanently.

The change adds two constructor parameters with sensible defaults:

• connect_timeout (3.05s) — per requests library recommendation to avoid TCP retransmission boundary edge cases
• read_timeout (60s) — generous enough for normal API responses while preventing indefinite waits

Fully backward-compatible: all existing callers get the defaults automatically. Connectors that need longer timeouts can override per-instance.

Code Changes

• src/fides/api/service/connectors/saas/authenticated_client.py - Add DEFAULT_CONNECT_TIMEOUT and DEFAULT_READ_TIMEOUT constants, add connect_timeout and read_timeout constructor parameters, pass timeout tuple to session.send()

Steps to Confirm

1. Verify existing SaaS connector tests pass (especially test_sending_special_characters which hits a real HTTP server through the full send path with the timeout now applied)
2. Verify no existing callers break (new params have defaults, fully backward-compatible)

Pre-Merge Checklist

• Issue requirements met
• All CI pipelines succeeded
• CHANGELOG.md updated
  • Updates unreleased work already in Changelog, no new entry necessary
• UX feedback:
  • No UX review needed
• Followup issues:
  • No followup issues
• Database migrations:
  • No migrations
• Documentation:
  • No documentation updates required

Made with Cursor

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ignored	Preview	Mar 1, 2026 4:33am
fides-privacy-center	Ignored		Mar 1, 2026 4:33am

[greptile-apps]

Greptile Summary

This PR adds default connect (3.05s) and read (60s) timeouts to AuthenticatedClient.send() to prevent unbounded HTTP waits from stalling Celery workers, a well-targeted and backward-compatible fix. The implementation is clean and correctly follows the requests library's tuple-timeout convention.

• DEFAULT_CONNECT_TIMEOUT and DEFAULT_READ_TIMEOUT are introduced as module-level constants and exposed as overridable constructor parameters — good for per-connector customization.
• The timeout is correctly applied to session.send() as a (connect, read) tuple.
• Note: when a requests.exceptions.Timeout is raised it falls into the broad except Exception handler in retry_send, which immediately breaks without retrying (per the existing inline comment). This is pre-existing behavior but is now more commonly exercised; callers will receive a ConnectionException on timeout.
• DEFAULT_READ_TIMEOUT is missing an explanatory comment, unlike DEFAULT_CONNECT_TIMEOUT which documents the reasoning behind its value.
• No new tests were added to assert that session.send() is actually called with the expected timeout tuple; the existing mocked tests do not verify call arguments, so the core behavioral change goes untested.

Confidence Score: 4/5

• Safe to merge — the change is minimal, backward-compatible, and addresses a real reliability risk in background workers.
• The logic change is a single, well-scoped line addition with no functional regressions expected. The only notable gaps are a missing inline comment on DEFAULT_READ_TIMEOUT and the absence of a targeted test verifying the timeout tuple is forwarded to session.send().
• No files require special attention, though adding timeout-specific test coverage to tests/ops/service/connectors/saas/test_authenticated_client.py would strengthen confidence further.

Important Files Changed

Filename	Overview
src/fides/api/service/connectors/saas/authenticated_client.py	Adds DEFAULT_CONNECT_TIMEOUT (3.05s) and DEFAULT_READ_TIMEOUT (60s) constants and wires them into session.send() via constructor parameters. Change is backward-compatible and logically sound; minor issue: DEFAULT_READ_TIMEOUT lacks an explanatory comment unlike its sibling constant.
changelog/7529.yaml	Changelog entry correctly categorizes the change as a fix with an accurate description.

_{Last reviewed commit: c1f5a43}

[greptile-apps]

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

tests/ops/service/connectors/saas/test_authenticated_client.py
No test verifying the timeout is passed to session.send()

The core behavioral change of this PR — passing timeout=(connect_timeout, read_timeout) to session.send() — is not verified by any test. test_client_returns_ok_response mocks Session.send but never asserts what arguments were passed. Without an explicit assertion, the timeout could silently be dropped (e.g., if someone later refactors the send call) and no test would catch it.

Consider adding a focused test:

@mock.patch.object(Session, "send")
def test_send_passes_timeout_to_session(
    self,
    send,
    test_authenticated_client,
    test_saas_request,
    test_config_dev_mode_disabled,
):
    test_response = Response()
    test_response.status_code = 200
    send.return_value = test_response
    test_authenticated_client.send(test_saas_request)
    send.assert_called_once_with(
        mock.ANY,
        timeout=(DEFAULT_CONNECT_TIMEOUT, DEFAULT_READ_TIMEOUT),
    )

Also worth adding a test for custom timeout values set via the constructor to confirm they are propagated correctly.

[Linker44]

looks good