Skip to content

Re-tier HTTP request timeouts around per-host memory - #3746

Open
ajpallares wants to merge 43 commits into
mainfrom
pallares/api-sources-timeouts
Open

Re-tier HTTP request timeouts around per-host memory#3746
ajpallares wants to merge 43 commits into
mainfrom
pallares/api-sources-timeouts

Conversation

@ajpallares

@ajpallares ajpallares commented Jul 9, 2026

Copy link
Copy Markdown
Member

Checklist

  • Unit tests

Motivation

Now that a single request can fail over across several hosts, the old timeout handling doesn't hold up. We only remember the most recent timeout in one global slot, so one slow host shortens timeouts for unrelated hosts (and a success on any host resets them all). The timeouts are also the same regardless of the request, so we wait too long on a host that's down before failing over to the next one.

Description

New per-attempt timeout tiers:

Request kind Base Reduced (source timed out in last 10 min)
Main-source, endpoint has NO fallback-URL support 15s 5s
Main-source, endpoint HAS fallback-URL support 5s 2s
Fallback-host request 30s n/a (flat)
Proxied request 30s n/a (flat)
  • Replace the single AtomicLong last-timeout slot in HTTPTimeoutManager with a per-host ConcurrentHashMap (host -> lastTimeoutDate, 10-minute per-entry expiry, pruned on access).
  • Re-tier the request timeouts as above; fallback-host and proxied requests never consult the memory.
  • Record a timeout for any main-source request that timed out (keyed by attempt host), dropping the previous fallback-support-only condition; a successful main-source response clears only that host's entry.
  • Rename the TIMEOUT_ON_MAIN_BACKEND_FOR_FALLBACK_SUPPORTED_ENDPOINT result to MAIN_SOURCE_TIMED_OUT; add AppConfig.hasProxyURL.

iOS counterpart: RevenueCat/purchases-ios#7179


Note

Medium Risk
Changes core HTTP timing and failover behavior for all backend calls; behavior is well covered by unit tests but mis-tuned timeouts could affect reliability under poor network conditions.

Overview
Reworks how the Android SDK picks connect timeouts when requests can fail over across multiple hosts. Per-host memory replaces a single global “last timeout” slot so one slow host no longer shortens timeouts for others, and a success on one host only clears that host’s entry (10-minute expiry per host).

New timeout tiers depend on attempt type: main-source without fallback-URL support uses 15s / 5s reduced; main-source with fallback support uses 5s / 2s; fallback-host and proxied requests use a flat 30s and ignore the memory.

HTTPClient records MAIN_SOURCE_TIMED_OUT for any main-source SocketTimeoutException (not only fallback-capable endpoints), passes resolved host into HTTPTimeoutManager, and uses AppConfig.hasProxyURL when computing timeouts. Test mocks and unit/integration tests are updated for the new API and enum name.

Reviewed by Cursor Bugbot for commit 7263274. Bugbot is set up for automated code reviews on this repo. Configure here.

ajpallares and others added 19 commits July 6, 2026 10:01
Main-API requests resolve their base host from the shared
RemoteConfigSourceProvider (always wired into HTTPClient), rather than
always using AppConfig.baseURL. Behavior-preserving groundwork for
remote-config-driven API host failover.

Co-authored-by: Cursor <cursoragent@cursor.com>
The inline comment restated the method KDoc; keep only the KDoc.

Co-authored-by: Cursor <cursoragent@cursor.com>
…mment

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Drop the default values for RemoteConfigManager's topicStore and
sourceProvider parameters so the shared instances from PurchasesFactory
are always injected, rather than silently constructing separate ones.
Callers now pass them explicitly; tests build equivalents wired to the
same disk cache.

Co-authored-by: Cursor <cursoragent@cursor.com>
The androidTest reachability helper builds an HTTPClient directly and
was missing the now-required apiSourceProvider argument. Pass null to
keep the legacy base-host behavior, matching the other integration test
helpers.

Co-authored-by: Cursor <cursoragent@cursor.com>
Express the fallback-attempt check with the already-defined isMainBackend
flag instead of re-deriving fallbackURLIndex > 0.

Co-authored-by: Cursor <cursoragent@cursor.com>
WebBillingGetProducts is hosted on the same api.revenuecat.com host as the
main API, so it should resolve its base host from the API source provider
too (with failover) instead of the static base URL.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…base-host

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt
…sConfigIntegrationTest

The main merge brought a new integration test that constructs RemoteConfigManager
without the topicStore/sourceProvider params introduced on this branch. Pass them
so the merged tree compiles.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ceProvider directly

The networking layer depended on the narrow APISourceProvider interface, an extra
abstraction over RemoteConfigSourceProvider that added little. Fold currentAPISource()
into RemoteConfigSourceProvider and have HTTPClient depend on it directly.

Co-authored-by: Cursor <cursoragent@cursor.com>
Replaces the single global "recent timeout" slot in `HTTPTimeoutManager`
(dropping the `AtomicLong`) with a per-host `ConcurrentHashMap`
(`host -> lastTimeoutDate`, 10-minute per-entry expiry, pruned on access) and
re-tiers the request timeouts:

- Main-source, endpoint with no fallback-URL support: 15s base / 5s reduced
- Main-source, endpoint with fallback-URL support: 5s base / 2s reduced
- Fallback-host and proxied requests: flat 30s (never consult the memory)

A main-source request to host H uses the reduced tier iff H has a non-expired
timeout entry. Timeouts are now recorded for any main-source request that timed
out (dropping the previous fallback-support-only condition), keyed by attempt
host; a successful main-source response clears only that host's entry. Renames
the `TIMEOUT_ON_MAIN_BACKEND_FOR_FALLBACK_SUPPORTED_ENDPOINT` result to
`MAIN_SOURCE_TIMED_OUT`. Adds `AppConfig.hasProxyURL` to signal proxied requests.

Co-authored-by: Cursor <cursoragent@cursor.com>
…base-host

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	purchases/src/main/kotlin/com/revenuecat/purchases/PurchasesFactory.kt
ajpallares and others added 6 commits July 10, 2026 12:28
Co-authored-by: Cursor <cursoragent@cursor.com>
…into pallares/api-sources-timeouts

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	purchases/src/main/kotlin/com/revenuecat/purchases/common/HTTPClient.kt
The usesAPISources when-expression did not handle GetRemoteConfigFallback,
which was added later via #3750. That endpoint is deliberately targeted at a
fallback base URL chosen by the domain layer and must not resolve its host
from the API source provider, so it now returns false alongside the other
non-API-source endpoints.

Co-authored-by: Cursor <cursoragent@cursor.com>
ajpallares and others added 5 commits July 13, 2026 15:58
…egration test

ProductionRemoteConfigFallbackIntegrationTest arrived via a main merge and
constructs RemoteConfigManager without the topicStore/sourceProvider params
that this branch made required (09cc7ee), breaking compilation. Wire them to
the same disk cache, mirroring ProductionRemoteConfigIntegrationTest.

Co-authored-by: Cursor <cursoragent@cursor.com>
…n tests

- Rewrite HTTPTimeoutManager.hasRecentTimeout to use a single expired flag so
  it stays within detekt's ReturnCount limit while keeping the prune-on-expiry
  behavior.
- Stub AppConfig.hasProxyURL in the integration reachability HTTPClient mock;
  the timeout manager now reads it during performRequest, so the unstubbed mock
  was throwing MockKException in the load-shedder/production integration tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
…onfig mock

The re-tiered HTTPTimeoutManager reads appConfig.hasProxyURL during
performRequest, but BaseBackendIntegrationTest's AppConfig mock never stubbed
it, so every backend integration test threw MockKException. Stub it to false,
matching the other HTTPClient AppConfig mocks.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.15385% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.61%. Comparing base (65f3fb5) to head (7263274).

Files with missing lines Patch % Lines
...tlin/com/revenuecat/purchases/common/HTTPClient.kt 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3746      +/-   ##
==========================================
+ Coverage   80.60%   80.61%   +0.01%     
==========================================
  Files         423      423              
  Lines       17401    17406       +5     
  Branches     2682     2687       +5     
==========================================
+ Hits        14026    14032       +6     
  Misses       2362     2362              
+ Partials     1013     1012       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ajpallares
ajpallares marked this pull request as ready for review July 13, 2026 16:54
@ajpallares
ajpallares requested a review from a team as a code owner July 13, 2026 16:54
@ajpallares
ajpallares requested a review from a team July 13, 2026 16:54

@AlvaroBrey AlvaroBrey left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good and makes sense I think! Just one question

ajpallares and others added 9 commits July 15, 2026 11:34
…s setting

Disabled-by-default internal DangerousSettings flag that the API-sources
host-resolution work (#3715) will gate behind, so it can merge without
enabling the behavior in production and tests can flip it on in CI.

Co-authored-by: Cursor <cursoragent@cursor.com>
…toString assertion

Co-authored-by: Cursor <cursoragent@cursor.com>
…erous-setting' into pallares/api-sources-base-host
Resolve main-API request hosts from the remote-config API sources only when
the `usesRemoteConfigAPISources` dangerous setting is enabled. When disabled
(the default), requests keep targeting the static AppConfig base URL.

Expose the setting via `AppConfig.usesRemoteConfigAPISources`, enable it in the
API-source HTTPClient tests, and add a test asserting an injected provider is
ignored while the setting is off.

Co-authored-by: Cursor <cursoragent@cursor.com>
Base automatically changed from pallares/api-sources-base-host to main July 17, 2026 07:52
ajpallares and others added 3 commits July 21, 2026 16:28
…timeouts

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	purchases/src/main/kotlin/com/revenuecat/purchases/common/HTTPClient.kt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants