-
Notifications
You must be signed in to change notification settings - Fork 348
Use fallback API hosts when receiving server down response #4970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
9577ba5
85267dc
720b3cd
b7ae271
f51040c
23420f1
e149e07
be78693
5146304
a18a382
1c365cd
88b9c03
0c9e623
c2facde
a9275b6
89e8d79
3c5a9d3
854a9f6
ba66359
cafbf9c
cd0dc1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -245,6 +245,7 @@ internal extension HTTPClient { | |
var headers: HTTPClient.RequestHeaders | ||
var verificationMode: Signing.ResponseVerificationMode | ||
var completionHandler: HTTPClient.Completion<Data>? | ||
var currentHostIndex: Int = 0 | ||
|
||
/// Whether the request has been retried. | ||
var retried: Bool { | ||
|
@@ -281,20 +282,35 @@ internal extension HTTPClient { | |
var method: HTTPRequest.Method { self.httpRequest.method } | ||
var path: String { self.httpRequest.path.relativePath } | ||
|
||
func getCurrentRequestURL(proxyURL: URL?) -> URL? { | ||
return self.httpRequest.path.url(hostURLIndex: self.currentHostIndex, proxyURL: proxyURL) | ||
} | ||
|
||
func retriedRequest() -> Self { | ||
var copy = self | ||
copy.retryCount += 1 | ||
copy.headers[RequestHeader.retryCount.rawValue] = "\(copy.retryCount)" | ||
return copy | ||
} | ||
|
||
func requestWithNextHost() -> Self? { | ||
let nextHostIndex = self.currentHostIndex + 1 | ||
guard self.httpRequest.path.url(hostURLIndex: nextHostIndex) != nil else { | ||
// No more hosts available | ||
return nil | ||
} | ||
var copy = self | ||
copy.currentHostIndex = nextHostIndex | ||
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) | ||
currentHostIndex=\(self.currentHostIndex) | ||
> | ||
""" | ||
} | ||
|
@@ -433,18 +449,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): | ||
|
@@ -463,26 +477,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.retryRequestWithNextHostIfNeeded(request: request, | ||
httpURLResponse: httpURLResponse) | ||
if !retryScheduled { | ||
retryScheduled = self.retryRequestIfNeeded(request: request, | ||
httpURLResponse: httpURLResponse) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These lines are the actual changes in this method. The other changes in this method are to accommodate the linter ( |
||
} | ||
|
||
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) | ||
|
@@ -540,7 +556,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) | ||
|
@@ -623,6 +639,37 @@ private extension HTTPClient { | |
// MARK: - Request Retry Logic | ||
extension HTTPClient { | ||
|
||
/// Evaluates whether a request should be retried with the next host in the list. | ||
/// | ||
/// This function checks the HTTP response status code to determine if the request should be retried | ||
/// with the next host in the list. 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 retryRequestWithNextHostIfNeeded( | ||
request: HTTPClient.Request, | ||
httpURLResponse: HTTPURLResponse? | ||
) -> Bool { | ||
|
||
guard let statusCode = httpURLResponse?.statusCode, HTTPStatusCode(rawValue: statusCode).isServerError, | ||
let nextRequest = request.requestWithNextHost() else { | ||
return false | ||
} | ||
|
||
Logger.debug(Strings.network.retrying_request_with_next_host( | ||
httpMethod: nextRequest.method.httpMethod, | ||
path: nextRequest.path, | ||
nextHost: nextRequest.currentHostIndex | ||
)) | ||
self.state.modify { | ||
$0.queuedRequests.insert(nextRequest, at: 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,8 @@ import Foundation | |
|
||
protocol HTTPRequestPath { | ||
|
||
/// The base URL for requests to this path. | ||
static var serverHostURL: URL { get } | ||
/// The base URLs for requests to this path, in order of preference. | ||
static var serverHostURLs: [URL] { get } | ||
|
||
/// Whether requests to this path are authenticated. | ||
var authenticated: Bool { get } | ||
|
@@ -45,12 +45,14 @@ extension HTTPRequestPath { | |
return "/v1/\(self.pathComponent)" | ||
} | ||
|
||
var url: URL? { return self.url(proxyURL: nil) } | ||
var url: URL? { return self.url(hostURLIndex: 0, proxyURL: nil) } | ||
|
||
func url(proxyURL: URL? = nil) -> URL? { | ||
return URL(string: self.relativePath, relativeTo: proxyURL ?? Self.serverHostURL) | ||
func url(hostURLIndex: Int, proxyURL: URL? = nil) -> URL? { | ||
guard let baseURL = proxyURL ?? Self.serverHostURLs[safe: hostURLIndex] else { | ||
return nil | ||
} | ||
return URL(string: self.relativePath, relativeTo: baseURL) | ||
} | ||
|
||
} | ||
|
||
// MARK: - Main paths | ||
|
@@ -91,8 +93,14 @@ extension HTTPRequest { | |
|
||
extension HTTPRequest.Path: HTTPRequestPath { | ||
|
||
// swiftlint:disable:next force_unwrapping | ||
static let serverHostURL = URL(string: "https://api.revenuecat.com")! | ||
static let serverHostURLs = [ | ||
"https://api.revenuecat.com", | ||
"https://api2.revenuecat.com", // TODO: Add real values | ||
"https://api3.revenuecat.com" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: add final hosts list |
||
].map { | ||
// swiftlint:disable:next force_unwrapping | ||
URL(string: $0)! | ||
} | ||
|
||
var authenticated: Bool { | ||
switch self { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ import Foundation | |
extension HTTPRequest.PaywallPath: HTTPRequestPath { | ||
|
||
// swiftlint:disable:next force_unwrapping | ||
static let serverHostURL = URL(string: "https://api-paywalls.revenuecat.com")! | ||
static let serverHostURLs = [URL(string: "https://api-paywalls.revenuecat.com")!] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: add final hosts list |
||
|
||
var authenticated: Bool { | ||
switch self { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: add final hosts list