Add client side timeout logic for endpoints that support fallback URLs by tonidero · Pull Request #2807 · RevenueCat/purchases-android

tonidero · 2025-11-05T11:36:58Z

Description

This adds some logic to implement a client side connection timeout. The logic is like this:

If the request supports fallback URLs and we're attempting the "main" backend, timeout will be 5s.
If the request is performed to a fallback URL or does not support fallbacks, timeout will be 30s
Once we timeout in one of the endpoints that support fallbacks, the new timeout for these endpoints that support timeouts in the main server will be 2s, so we can fail early and try the fallback.
All requests will always hit the "main" backend first.
Whenever the main server is hit successfully, the timeout is reset to the initial 5s for the endpoints that support fallbacks

This is pretty delicate, so lmk what you think!

tonidero · 2025-11-05T13:26:24Z

+    companion object {
+        const val SUPPORTED_FALLBACK_TIMEOUT_MS = 5000L // 5 seconds for requests with fallback support
+        const val REDUCED_TIMEOUT_MS = 2000L // 2 seconds for requests with fallback support after timeout
+        const val DEFAULT_TIMEOUT_MS = 30000L // 30 seconds for requests without fallback support and fallback requests


This is pretty long... but right now we aren't setting anything, and this felt pretty safe IMO.

I agree. I'd be more worried about the SUPPORTED_FALLBACK_TIMEOUT_MS. I wonder if 5 seconds is too short?

rickvdl · 2025-11-05T14:01:53Z

@tonidero Very nice, seems like quite a clean implementation. Reviewed it briefly and will work on the iOS side and then review again with the knowledge gathered :)

rickvdl

Awesome work! I think in the end the behavior is quite complex, but your solution is very clean and testable!

rickvdl · 2025-11-06T16:43:20Z

+    // region Timeout Management
+
+    @Test
+    fun `HTTPClient records SUCCESS_ON_MAIN_BACKEND when successful request to main backend`() {


Great set of tests as well!

I think ideally we also mock the HTTPTimeoutManager and assert on the recorded events.I think these tests are very useful, but they assert on the behavior of the timeout manager, assuming it recorded what we think it recorded.

I think there could be a small role for this mock, making 100% sure it recorded the right event. What do you think?

Added some on the iOS side here

Right, I've wanted to avoid mocking things in tests when possible for a long time, and test a more comprehensive behavior. Since this is relatively coupled, I thought we could use the real dependency, but I think having some extra tests that mock it and verify that calls happen as expected, does make sense as well. I will add those aside from the already added ones 👍 Thanks for the feedback!

In the end what I did was use spies and reuse the existing tests, so I could just verify the correct calls were being made: 47a8212. Lmk what you think!

Looks great @tonidero 🙌

rickvdl · 2025-11-06T16:43:43Z

+    }
+
+    @Test
+    fun `getTimeoutForRequest returns SUPPORTED_FALLBACK_TIMEOUT_MS for endpoints with fallback support when no timeout occurred`() {


Awesome set of tests! Took some inspiration from them :)

codecov · 2025-11-11T12:09:16Z

Codecov Report

❌ Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.02%. Comparing base (6396859) to head (ae28bf0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...tlin/com/revenuecat/purchases/common/HTTPClient.kt	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2807      +/-   ##
==========================================
+ Coverage   77.95%   78.02%   +0.07%     
==========================================
  Files         318      319       +1     
  Lines       12465    12505      +40     
  Branches     1712     1722      +10     
==========================================
+ Hits         9717     9757      +40     
  Misses       2022     2022              
  Partials      726      726

☔ View full report in Codecov by Sentry.
📢 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.

tonidero · 2025-11-11T12:36:17Z

+    private val dateProvider: DateProvider = DefaultDateProvider(),
+) {
+    companion object {
+        const val READ_TIMEOUT_MS = 30000L // We always use 30 seconds for read timeout, once we have established a connection


I decided to change the read timeout to 30 seconds always. I feel that if it's a connectivity issue with the backend (because it's queuing or something like that), it would be catched during the connect timeout, which we can keep at 5s for endpoints supporting fallback. But for the read timeout, that could be an indication of slow connections, so I think it might be better to be more conservative. Lmk what you think!

ajpallares

Wow! This is a very clean implementation and it was a great idea to separate it into its own object. And as Rick mentioned, great set of tests 👌
Thanks for being so thorough! 🙌

I have some small comments.

The first thing that came to mind was when reading the description.

Once we timeout in one of the endpoints that support fallbacks, the new timeout for these endpoints that support timeouts in the main server will be 2s, so we can fail early and try the fallback.

I later realized it is indeed implemented, but probably I'd add that

Whenever the main server is hit successfully, the timeout is reset to the initial 5s for the endpoints that support fallbacks

Other than that, some small comments! Thanks!

ajpallares · 2025-11-11T15:58:12Z

+    companion object {
+        const val SUPPORTED_FALLBACK_TIMEOUT_MS = 5000L // 5 seconds for requests with fallback support
+        const val REDUCED_TIMEOUT_MS = 2000L // 2 seconds for requests with fallback support after timeout
+        const val DEFAULT_TIMEOUT_MS = 30000L // 30 seconds for requests without fallback support and fallback requests


I agree. I'd be more worried about the SUPPORTED_FALLBACK_TIMEOUT_MS. I wonder if 5 seconds is too short?

ajpallares · 2025-11-11T16:08:19Z

+    fun recordRequestResult(result: RequestResult) {
+        when (result) {
+            RequestResult.SUCCESS_ON_MAIN_BACKEND -> {
+                resetTimeout()


Nice! I was exactly looking for this! Makes total sense 👍

…gs and big responses

…y settings and big responses" This reverts commit d21e961.

ajpallares

Makes sense! Thank you for all the iterations! Again, great job on this 🙏

JayShortway

Great stuff!!

JayShortway · 2025-11-18T11:32:20Z

+        if (shouldResetTimeout()) {
+            resetTimeout()
+        }


If my mental compiler is correct I guess we could have 2 threads both resetting the time out, if the second thread reads lastTimeoutRequestTime before the first resets it, but that's probably fine?

Indeed, that would be possible. But as you said, it wouldn't be a problem. We would just be setting to 0 multiple times. Great catch though!

…Client.kt Co-authored-by: Antonio Pallares <ajpallares@users.noreply.github.com>

### Description Seems the backend integration tests were flaky after adding the timeouts in #2807. This is because the timeout was too short during tests, causing it to be hit frequently.

@facumenzella

**This is an automatic release.** > [!WARNING] > If you don't have any login system in your app, please make sure your one-time purchase products have been correctly configured in the RevenueCat dashboard as either consumable or non-consumable. If they're incorrectly configured as consumables, RevenueCat will consume these purchases. This means that users won't be able to restore them from version 9.0.0 onward. > Non-consumables are products that are meant to be bought only once, for example, lifetime subscriptions. ## RevenueCat SDK ### 🐞 Bugfixes * Restore Purchases config automatically in CustomerCenter (#2867) via Facundo Menzella (@facumenzella) * Handle error reading `errorStream` in some devices (#2865) via Toni Rico (@tonidero) * [MON-1122] Revert variable rounding logic to not round up (#2857) via Pol Piella Abadia (@polpielladev) ## RevenueCatUI SDK ### Paywallv2 #### 🐞 Bugfixes * Select default package on Sheet dismissal (#2861) via Cesar de la Vega (@vegaro) ### Customer Center #### ✨ New Features * CC-581 | Allow for support ticket creation (#2810) via Rosie Watson (@RosieWatson) ### 🔄 Other Changes * Bump fastlane-plugin-revenuecat_internal from `7328ea7` to `efca663` (#2864) via dependabot[bot] (@dependabot[bot]) * Bump fastlane from 2.228.0 to 2.229.0 (#2863) via dependabot[bot] (@dependabot[bot]) * Bump fastlane-plugin-revenuecat_internal from `083ced9` to `7328ea7` (#2862) via dependabot[bot] (@dependabot[bot]) * Runs plugin actions from correct directory (#2858) via JayShortway (@JayShortway) * Flush multiple event batches (#2842) via Toni Rico (@tonidero) * Add file size limit to events tracking files (#2841) via Toni Rico (@tonidero) * Make events manager be supported in Android < 24 (#2854) via Toni Rico (@tonidero) * Add non paid revenue reporting infra (#2728) via Toni Rico (@tonidero) * Fix backend integration tests (#2860) via Toni Rico (@tonidero) * Track `connection_error_reason` property in diagnostics (#2855) via Toni Rico (@tonidero) * Uses some git+GitHub lanes from Fastlane plugin (#2856) via JayShortway (@JayShortway) * Add client side timeout logic for endpoints that support fallback URLs (#2807) via Toni Rico (@tonidero) * [EXTERNAL] Fix deprecation warnings in examples module (#2852) contributed by @gojoel (#2853) via Toni Rico (@tonidero) * Bump fastlane-plugin-revenuecat_internal from `9f78bb9` to `083ced9` (#2848) via dependabot[bot] (@dependabot[bot]) Co-authored-by: revenuecat-ops <ops@revenuecat.com>

Add client side timeout logic for endpoints that support fallback URLs

fcb340a

tonidero added the pr:other label Nov 5, 2025

tonidero commented Nov 5, 2025

View reviewed changes

tonidero requested a review from a team November 5, 2025 13:29

tonidero requested a review from bisho November 5, 2025 14:33

Merge branch 'main' into add-client-side-timeout-logic

b372e4f

rickvdl reviewed Nov 6, 2025

View reviewed changes

rickvdl mentioned this pull request Nov 6, 2025

Add client side timeout logic for endpoints that support fallback URLs RevenueCat/purchases-ios#5760

Merged

3 tasks

tonidero and others added 4 commits November 10, 2025 13:17

Merge branch 'main' into add-client-side-timeout-logic

c80cdc8

Merge branch 'main' into add-client-side-timeout-logic

b866f23

Fix tests after merging main

cce79bf

Add verification that correct events were recorded

47a8212

tonidero marked this pull request as ready for review November 10, 2025 16:49

tonidero requested a review from a team November 10, 2025 16:49

Make read timeout always 30s

0ccb0b2

tonidero commented Nov 11, 2025

View reviewed changes

tonidero added 2 commits November 11, 2025 15:46

Fix detekt

c970a89

Fix tests after adding different read timeout

c28a56c

ajpallares approved these changes Nov 11, 2025

View reviewed changes

tonidero and others added 2 commits November 12, 2025 12:53

Merge branch 'main' into add-client-side-timeout-logic

bc080ab

Change how we read text to handle timeouts in low connectivity settin…

d21e961

…gs and big responses

tonidero commented Nov 12, 2025

View reviewed changes

Comment thread purchases/src/main/kotlin/com/revenuecat/purchases/common/HTTPClient.kt

tonidero requested review from ajpallares and rickvdl November 12, 2025 13:12

tonidero added 4 commits November 12, 2025 17:33

Revert "Change how we read text to handle timeouts in low connectivit…

5b0dc91

…y settings and big responses" This reverts commit d21e961.

Remove read timeout for security and avoid unnecessary changes

d722d52

Fix tests

5184e95

Fix tests

694ce7f

Cleanup unnecessary logic

8696d45

ajpallares approved these changes Nov 14, 2025

View reviewed changes

Comment thread purchases/src/main/kotlin/com/revenuecat/purchases/common/HTTPClient.kt Outdated

JayShortway approved these changes Nov 18, 2025

View reviewed changes

tonidero and others added 3 commits November 18, 2025 15:14

Update purchases/src/main/kotlin/com/revenuecat/purchases/common/HTTP…

716588a

…Client.kt Co-authored-by: Antonio Pallares <ajpallares@users.noreply.github.com>

Rename

b566181

Merge branch 'main' into add-client-side-timeout-logic

ae28bf0

tonidero enabled auto-merge November 18, 2025 15:55

tonidero added this pull request to the merge queue Nov 18, 2025

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 18, 2025

tonidero merged commit 4bfc79c into main Nov 18, 2025
23 checks passed

tonidero deleted the add-client-side-timeout-logic branch November 18, 2025 16:16

RCGitBot mentioned this pull request Nov 19, 2025

[AUTOMATIC] Release/9.15.0 #2859

Closed

tonidero mentioned this pull request Nov 20, 2025

Fix backend integration tests #2860

Merged

RCGitBot mentioned this pull request Nov 25, 2025

[AUTOMATIC] Release/9.15.0 #2868

Merged

Uh oh!

Conversation

tonidero commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rickvdl commented Nov 5, 2025

Uh oh!

rickvdl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajpallares left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ajpallares left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JayShortway left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tonidero commented Nov 5, 2025 •

edited

Loading

codecov Bot commented Nov 11, 2025 •

edited

Loading