Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ extension HTTPRequest.DiagnosticsPath: HTTPRequestPath {
}
}

var pathComponent: String {
var relativePath: String {
switch self {
case .postDiagnostics:
return "diagnostics"
return "/v1/diagnostics"
}
}

Expand Down
4 changes: 4 additions & 0 deletions Sources/Logging/Strings/NetworkStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ enum NetworkStrings {
case starting_next_request(request: String)
case starting_request(httpMethod: String, path: String)
case retrying_request(httpMethod: String, path: String)
case retrying_request_with_fallback_path(httpMethod: String, path: String)
case failing_url_resolved_to_host(url: URL, resolvedHost: String)
case blocked_network(url: URL, newHost: String?)
case api_request_redirect(from: URL, to: URL)
Expand Down Expand Up @@ -119,6 +120,9 @@ extension NetworkStrings: LogMessage {
case let .retrying_request(httpMethod, path):
return "Retrying request \(httpMethod) \(path)"

case let .retrying_request_with_fallback_path(httpMethod, path):
return "Retrying request using fallback host: \(httpMethod) \(path)"

case let .failing_url_resolved_to_host(url, resolvedHost):
return "Failing url '\(url)' resolved to host '\(resolvedHost)'"

Expand Down
96 changes: 74 additions & 22 deletions Sources/Networking/HTTPClient/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ internal extension HTTPClient {
var headers: HTTPClient.RequestHeaders
var verificationMode: Signing.ResponseVerificationMode
var completionHandler: HTTPClient.Completion<Data>?
private(set) var fallbackPathIndex: Int?

/// Whether the request has been retried.
var retried: Bool {
Expand Down Expand Up @@ -279,25 +280,46 @@ internal extension HTTPClient {
}

var method: HTTPRequest.Method { self.httpRequest.method }
var path: String { self.httpRequest.path.relativePath }
var path: String { (self.getCurrentPath() ?? self.httpRequest.path).relativePath }

func getCurrentRequestURL(proxyURL: URL?) -> URL? {
return self.getCurrentPath()?.url(proxyURL: proxyURL)
}

func retriedRequest() -> Self {
var copy = self
copy.retryCount += 1
copy.headers[RequestHeader.retryCount.rawValue] = "\(copy.retryCount)"
return copy
}

func requestWithNextFallbackPath() -> Self? {
var copy = self
copy.fallbackPathIndex = self.fallbackPathIndex?.advanced(by: 1) ?? 0
guard copy.getCurrentPath() != nil else {
// No more fallback paths available
return nil
}
return copy
}

var description: String {
"""
<\(type(of: self)): httpMethod=\(self.method.httpMethod)
path=\(self.path)
headers=\(self.headers.description )
headers=\(self.headers.description)
retried=\(self.retried)
>
"""
}

private func getCurrentPath() -> HTTPRequestPath? {
if let fallbackPathIndex = self.fallbackPathIndex {
return self.httpRequest.path.fallbackPaths[safe: fallbackPathIndex]
} else {
return self.httpRequest.path
}
}
Copy link
Member

Choose a reason for hiding this comment

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

So only the path changes in a fallback scenario? I'm a bit confused as this PR title mentions hosts (and the Diagnostics PR does too), but I don't see hosts being changed. Maybe I'm misunderstanding something.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry about that. Originally, the host would only change, but we realized that the fallback backend had different paths. In the end, after talking with COIN, they've added a mapping in the fallback backend which means that the original premise was correct: only the host will change.
I'll put this back as a draft until I've made the changes back. Sorry again!

Copy link
Member

Choose a reason for hiding this comment

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

No worries at all! Thanks for explaining 😄

}
}

Expand Down Expand Up @@ -433,18 +455,16 @@ private extension HTTPClient {
requestStartTime: Date) {
RCTestAssertNotMainThread()

let response = self.parse(
urlResponse: urlResponse,
request: request,
urlRequest: urlRequest,
data: data,
error: networkError,
requestStartTime: requestStartTime
)
let response = self.parse(urlResponse: urlResponse,
request: request,
urlRequest: urlRequest,
data: data,
error: networkError,
requestStartTime: requestStartTime)

if let response = response {
let httpURLResponse = urlResponse as? HTTPURLResponse
var requestRetryScheduled = false
var retryScheduled = false

switch response {
case let .success(response):
Expand All @@ -463,26 +483,28 @@ private extension HTTPClient {
case let .failure(error):
let httpURLResponse = urlResponse as? HTTPURLResponse

Logger.debug(Strings.network.api_request_failed(
request.httpRequest,
httpCode: httpURLResponse?.httpStatusCode,
error: error,
metadata: httpURLResponse?.metadata)
)
Logger.debug(Strings.network.api_request_failed(request.httpRequest,
httpCode: httpURLResponse?.httpStatusCode,
error: error,
metadata: httpURLResponse?.metadata))

if httpURLResponse?.isLoadShedder == true {
Logger.debug(Strings.network.request_handled_by_load_shedder(request.httpRequest.path))
}

requestRetryScheduled = self.retryRequestIfNeeded(request: request, httpURLResponse: httpURLResponse)
retryScheduled = self.retryRequestWithNextFallbackPathIfNeeded(request: request,
httpURLResponse: httpURLResponse)
if !retryScheduled {
retryScheduled = self.retryRequestIfNeeded(request: request,
httpURLResponse: httpURLResponse)
}
}

if !requestRetryScheduled {
if !retryScheduled {
request.completionHandler?(response)
}
} else {
Logger.debug(Strings.network.retrying_request(httpMethod: request.method.httpMethod,
path: request.path))
Logger.debug(Strings.network.retrying_request(httpMethod: request.method.httpMethod, path: request.path))

self.state.modify {
$0.queuedRequests.insert(request.retriedRequest(), at: 0)
Expand Down Expand Up @@ -540,7 +562,7 @@ private extension HTTPClient {
}

func convert(request: Request) -> URLRequest? {
guard let requestURL = request.httpRequest.path.url(proxyURL: SystemInfo.proxyURL) else {
guard let requestURL = request.getCurrentRequestURL(proxyURL: SystemInfo.proxyURL) else {
return nil
}
var urlRequest = URLRequest(url: requestURL)
Expand Down Expand Up @@ -623,6 +645,36 @@ private extension HTTPClient {
// MARK: - Request Retry Logic
extension HTTPClient {

/// Evaluates whether a request should be retried with the next path in the list of fallback paths.
///
/// This function checks the HTTP response status code to determine if the request should be retried
/// with the next fallback path. If the retry conditions are met, it schedules the request immediately and
/// returns `true` to indicate that the request was retried.
///
/// - Parameters:
/// - request: The original `HTTPClient.Request` that may need to be retried.
/// - httpURLResponse: An optional `HTTPURLResponse` that contains the status code of the response.
/// - Returns: A Boolean value indicating whether the request was retried.
internal func retryRequestWithNextFallbackPathIfNeeded(
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, nothing to do in this PR, but I was thinking it would be great to have a way to test this more easily. One thing that came to mind would be to add an option to our test proxy to block only these endpoints in the main host...

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I like that! In fact I'm trying to add these fallback endpoints to our backend tests and I'm really struggling so far 😅

request: HTTPClient.Request,
httpURLResponse: HTTPURLResponse?
) -> Bool {

guard let statusCode = httpURLResponse?.statusCode, HTTPStatusCode(rawValue: statusCode).isServerError,
let nextRequest = request.requestWithNextFallbackPath() else {
return false
}

Logger.debug(Strings.network.retrying_request_with_fallback_path(
httpMethod: nextRequest.method.httpMethod,
path: nextRequest.path
))
self.state.modify {
$0.queuedRequests.insert(nextRequest, at: 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm nothing to do for this PR since I believe this could actually happen already, but I wonder if we should dedupe any queued requests here... In android we do something like that, and just "hook" the callbacks to the existing queued/in progress request if it exists.

}
return true
}

/// Evaluates whether a request should be retried and schedules a retry if necessary.
///
/// This function checks the HTTP response status code to determine if the request should be retried.
Expand Down
77 changes: 70 additions & 7 deletions Sources/Networking/HTTPClient/HTTPRequestPath.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ protocol HTTPRequestPath {
/// The base URL for requests to this path.
static var serverHostURL: URL { get }

/// The fallback paths to use when the main server is down.
///
/// The structure of the response must be the same as that of the main path.
var fallbackPaths: [HTTPRequestPath] { get }

/// Whether requests to this path are authenticated.
var authenticated: Bool { get }

Expand All @@ -30,27 +35,24 @@ protocol HTTPRequestPath {
/// Whether endpoint requires a nonce for signature verification.
var needsNonceForSigning: Bool { get }

/// The path component for this endpoint.
var pathComponent: String { get }

/// The name of the endpoint.
var name: String { get }

/// The full relative path for this endpoint.
var relativePath: String { get }
}

extension HTTPRequestPath {

/// The full relative path for this endpoint.
var relativePath: String {
return "/v1/\(self.pathComponent)"
var fallbackPaths: [HTTPRequestPath] {
return []
}

var url: URL? { return self.url(proxyURL: nil) }

func url(proxyURL: URL? = nil) -> URL? {
return URL(string: self.relativePath, relativeTo: proxyURL ?? Self.serverHostURL)
}

}

// MARK: - Main paths
Expand Down Expand Up @@ -94,6 +96,17 @@ extension HTTPRequest.Path: HTTPRequestPath {
// swiftlint:disable:next force_unwrapping
static let serverHostURL = URL(string: "https://api.revenuecat.com")!

var fallbackPaths: [HTTPRequestPath] {
switch self {
case .getOfferings:
return [HTTPRequest.FallbackPath.getOfferings]
case .getProductEntitlementMapping:
return [HTTPRequest.FallbackPath.getProductEntitlementMapping]
default:
return []
}
}

var authenticated: Bool {
switch self {
case .getCustomerInfo,
Expand Down Expand Up @@ -175,6 +188,10 @@ extension HTTPRequest.Path: HTTPRequestPath {
}
}

var relativePath: String {
return "/v1/\(self.pathComponent)"
}

var pathComponent: String {
switch self {
case let .getCustomerInfo(appUserID):
Expand Down Expand Up @@ -267,3 +284,49 @@ extension HTTPRequest.Path: HTTPRequestPath {
return appUserID.trimmedAndEscaped
}
}

extension HTTPRequest {

enum FallbackPath: HTTPRequestPath {

case getOfferings
case getProductEntitlementMapping

// swiftlint:disable:next force_unwrapping
static let serverHostURL = URL(string: "https://api-production.8-lives-cat.io")!

var authenticated: Bool {
return true
}

var shouldSendEtag: Bool {
return true
}

var supportsSignatureVerification: Bool {
return false
}

var needsNonceForSigning: Bool {
return false
}

var relativePath: String {
switch self {
case .getOfferings:
return "/offerings"
case .getProductEntitlementMapping:
return "/product_entitlement_mapping"
}
}

var name: String {
switch self {
case .getOfferings:
return "fallback_get_offerings"
case .getProductEntitlementMapping:
return "fallback_get_product_entitlement_mapping"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ extension HTTPRequest.PaywallPath: HTTPRequestPath {
}
}

var pathComponent: String {
var relativePath: String {
switch self {
case .postEvents:
return "events"
return "/v1/events"
}
}

Expand Down
Loading