Ensure timeout is applied at HttpUrlConenction level

[ianrumac]

Changes in this pull request

Checklist

• All unit tests pass.
• All UI tests pass.
• Demo project builds and runs.
• I added/updated tests or detailed why my change isn't tested.
• I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
• I have run ktlint in the main directory and fixed any issues.
• I have updated the SDK documentation as well as the online docs.
• I have reviewed the contributing guide

Greptile Summary

This PR propagates a timeout: Duration? parameter through NetworkService, NetworkRequestData, and RequestExecutor to set HttpURLConnection.connectTimeout and readTimeout, ensuring blocking HTTP calls are bounded. Specific timeouts are wired up in EnrichmentService (already had a coroutine-level guard) and SubscriptionService (no coroutine guard).

• Effective timeout is doubled: connectTimeout and readTimeout are independent phases; setting both to T means the maximum HTTP-level block is 2×T. For SubscriptionService calls (no coroutine-level withTimeout), a 5 s configured timeout can actually block for up to 10 s.
• toInt() on inWholeMilliseconds silently overflows for durations > ~24.8 days; a coerceIn guard is a cheap safety net.

Confidence Score: 3/5

The PR improves the status quo but the doubled effective timeout at the HTTP layer means configured timeout values are not enforced as intended for SubscriptionService calls.

A P1 finding (effective timeout is silently 2× the configured value for callers without a coroutine guard) pulls the score below the P1 ceiling of 4. The fix is contained to RequestExecutor and is straightforward, so it does not drop further.

superwall/src/main/java/com/superwall/sdk/network/RequestExecutor.kt — the connectTimeout/readTimeout assignment is where the doubled-timeout defect lives.

Important Files Changed

Filename	Overview
superwall/src/main/java/com/superwall/sdk/network/RequestExecutor.kt	Applies connectTimeout and readTimeout from NetworkRequestData.timeout; both are set to the same value, doubling the effective maximum HTTP-level wait, and toInt() can silently overflow for very large durations.
superwall/src/main/java/com/superwall/sdk/network/NetworkRequestData.kt	Adds a non-defaulted timeout: Duration? field; propagated correctly through copyWithUrl, but missing a default value on the constructor which is a minor source-compatibility concern.
superwall/src/main/java/com/superwall/sdk/network/NetworkService.kt	Threads optional timeout: Duration? = null through get() and post() helpers into NetworkRequestData; looks correct.
superwall/src/main/java/com/superwall/sdk/network/SubscriptionService.kt	Adds explicit timeouts (10 s for redeemToken, 5 s for entitlement GETs) with no coroutine-level guard, so the effective maximum blocking time is 2× the configured value due to the separate connectTimeout/readTimeout issue in RequestExecutor.
superwall/src/main/java/com/superwall/sdk/network/EnrichmentService.kt	Now passes timeout to post() alongside the existing eitherWithTimeout coroutine guard; the coroutine guard remains the primary enforcer, with the HTTP timeouts as a safety net.
CHANGELOG.md	Adds a changelog entry for the timeout fix; otherwise fine.

Sequence Diagram

sequenceDiagram
    participant S as Service (Enrichment/Subscription)
    participant NS as NetworkService
    participant CE as CustomHttpUrlConnection
    participant RE as RequestExecutor
    participant HUC as HttpURLConnection

    S->>NS: get/post(path, timeout=T)
    NS->>CE: request(buildRequestData, retryCount)
    CE->>RE: execute(NetworkRequestData(timeout=T))
    RE->>HUC: openConnection()
    RE->>HUC: connectTimeout = T.inWholeMilliseconds.toInt()
    RE->>HUC: readTimeout = T.inWholeMilliseconds.toInt()
    Note over HUC: Max HTTP wait = connectTimeout + readTimeout = 2×T
    HUC-->>RE: responseCode / data
    RE-->>CE: Either<RequestResult, NetworkError>
    CE-->>NS: Either<T, NetworkError>
    NS-->>S: Either<T, NetworkError>

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
superwall/src/main/java/com/superwall/sdk/network/RequestExecutor.kt:146-147
**Timeout doubled at HTTP level**

`connectTimeout` and `readTimeout` are independent phases — they apply to TCP connection establishment and data read separately. Setting both to the same `timeout` value means the maximum HTTP-level blocking time is `2 × timeout`. For callers without a coroutine-level guard (e.g., `SubscriptionService.webEntitlementsByUserId` with `timeout = 5.seconds`), the call can block for up to 10 seconds instead of 5. If the intent is "the entire operation must complete within `timeout`", you'd need to split the budget or wrap every call in a coroutine-level `withTimeout` as `EnrichmentService` already does.

### Issue 2 of 3
superwall/src/main/java/com/superwall/sdk/network/RequestExecutor.kt:146-147
**`toInt()` silently overflows for large durations**

`Duration.inWholeMilliseconds` returns a `Long`; `.toInt()` truncates it without overflow protection. For any duration above ~24.8 days (`Int.MAX_VALUE` ms) the value wraps to a negative number. `HttpURLConnection` rejects negative timeouts with `IllegalArgumentException`, turning this into a runtime crash. The values used in this PR (1 s / 5 s / 10 s) are safe, but a defensive cast is cheap:

```suggestion
        connection.connectTimeout = timeout?.inWholeMilliseconds?.coerceIn(0, Int.MAX_VALUE.toLong())?.toInt() ?: 0
        connection.readTimeout = timeout?.inWholeMilliseconds?.coerceIn(0, Int.MAX_VALUE.toLong())?.toInt() ?: 0
```

### Issue 3 of 3
superwall/src/main/java/com/superwall/sdk/network/NetworkRequestData.kt:20
**`timeout` parameter has no default value**

All existing callsites in `NetworkService` default `timeout` to `null`, but `NetworkRequestData` itself has no default. Any direct construction of this class (e.g., in tests or future callers) now requires an explicit `timeout` argument, which is a source-breaking change. Adding `= null` makes the field consistent with how `NetworkService` exposes it.

```suggestion
    val timeout: Duration? = null
```

_{Reviews (1): Last reviewed commit: "Ensure timeout is applied at HttpUrlCone..." | Re-trigger Greptile}

Greptile also left 3 inline comments on this PR.