Skip to content
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

Urlsession retry strategy #7708

Merged
merged 1 commit into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions ios/MullvadREST/ApiHandlers/MullvadApiRequestFactory.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import MullvadRustRuntime
import MullvadTypes

enum MullvadApiRequest {
case getAddressList
case getAddressList(retryStrategy: REST.RetryStrategy)
}

struct MullvadApiRequestFactory {
Expand All @@ -25,8 +25,12 @@ struct MullvadApiRequestFactory {
let rawPointer = Unmanaged.passRetained(pointerClass).toOpaque()

return switch request {
case .getAddressList:
MullvadApiCancellable(handle: mullvad_api_get_addresses(apiContext.context, rawPointer))
case let .getAddressList(retryStrategy):
MullvadApiCancellable(handle: mullvad_api_get_addresses(
apiContext.context,
rawPointer,
retryStrategy.toRustStrategy()
))
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions ios/MullvadREST/ApiHandlers/RESTAPIProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ extension REST {
retryStrategy: REST.RetryStrategy,
completionHandler: @escaping @Sendable ProxyCompletionHandler<[AnyIPEndpoint]>
) -> Cancellable {
let requestHandler = mullvadApiRequestFactory.makeRequest(.getAddressList)
let requestHandler = mullvadApiRequestFactory.makeRequest(.getAddressList(retryStrategy: retryStrategy))

let responseHandler = rustResponseHandler(
decoding: [AnyIPEndpoint].self,
Expand All @@ -76,7 +76,6 @@ extension REST {
let networkOperation = MullvadApiNetworkOperation(
name: "get-api-addrs",
dispatchQueue: dispatchQueue,
retryStrategy: retryStrategy,
requestHandler: requestHandler,
responseDecoder: responseDecoder,
responseHandler: responseHandler,
Expand Down
63 changes: 1 addition & 62 deletions ios/MullvadREST/ApiHandlers/RESTRustNetworkOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,14 @@ extension REST {
private let responseHandler: any RESTRustResponseHandler<Success>
private var networkTask: MullvadApiCancellable?

private let retryStrategy: RetryStrategy
private var retryDelayIterator: AnyIterator<Duration>
private var retryTimer: DispatchSourceTimer?
private var retryCount = 0

init(
name: String,
dispatchQueue: DispatchQueue,
retryStrategy: RetryStrategy,
requestHandler: @escaping MullvadApiRequestHandler,
responseDecoder: JSONDecoder,
responseHandler: some RESTRustResponseHandler<Success>,
completionHandler: CompletionHandler? = nil
) {
self.retryStrategy = retryStrategy
retryDelayIterator = retryStrategy.makeDelayIterator()
self.responseDecoder = responseDecoder
self.requestHandler = requestHandler
self.responseHandler = responseHandler
Expand All @@ -53,10 +45,7 @@ extension REST {
}

override public func operationDidCancel() {
retryTimer?.cancel()
networkTask?.cancel()

retryTimer = nil
networkTask = nil
}

Expand All @@ -76,12 +65,7 @@ extension REST {
guard let self else { return }

if let error = response.restError() {
if response.shouldRetry {
retryRequest(with: error)
} else {
finish(result: .failure(error))
}

finish(result: .failure(error))
return
}

Expand All @@ -97,51 +81,6 @@ extension REST {
}
}
}

private func retryRequest(with error: REST.Error) {
// Check if retry count is not exceeded.
guard retryCount < retryStrategy.maxRetryCount else {
if retryStrategy.maxRetryCount > 0 {
logger.debug("Ran out of retry attempts (\(retryStrategy.maxRetryCount))")
}
finish(result: .failure(error))
return
}

// Increment retry count.
retryCount += 1

// Retry immediately if retry delay is set to never.
guard retryStrategy.delay != .never else {
startRequest()
return
}

guard let waitDelay = retryDelayIterator.next() else {
logger.debug("Retry delay iterator failed to produce next value.")

finish(result: .failure(error))
return
}

logger.debug("Retry in \(waitDelay.logFormat()).")

// Create timer to delay retry.
let timer = DispatchSource.makeTimerSource(queue: dispatchQueue)

timer.setEventHandler { [weak self] in
self?.startRequest()
}

timer.setCancelHandler { [weak self] in
self?.finish(result: .failure(OperationError.cancelled))
}

timer.schedule(wallDeadline: .now() + waitDelay.timeInterval)
timer.activate()

retryTimer = timer
}
}
}

Expand Down
18 changes: 18 additions & 0 deletions ios/MullvadREST/RetryStrategy/RetryStrategy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import Foundation
import MullvadRustRuntime
import MullvadTypes

extension REST {
Expand All @@ -21,6 +22,23 @@ extension REST {
self.applyJitter = applyJitter
}

/// The return value of this function *must* be passed to a Rust FFI function that will consume it, otherwise it will leak.
public func toRustStrategy() -> SwiftRetryStrategy {
switch delay {
case .never:
return mullvad_api_retry_strategy_never()
case let .constant(duration):
return mullvad_api_retry_strategy_constant(UInt(maxRetryCount), UInt64(duration.seconds))
case let .exponentialBackoff(initial, multiplier, maxDelay):
return mullvad_api_retry_strategy_exponential(
UInt(maxRetryCount),
UInt64(initial.seconds),
UInt32(multiplier),
UInt64(maxDelay.seconds)
)
}
}

public func makeDelayIterator() -> AnyIterator<Duration> {
let inner = delay.makeIterator()

Expand Down
4 changes: 0 additions & 4 deletions ios/MullvadRustRuntime/MullvadApiResponse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ public class MullvadApiResponse {
}
}

public var shouldRetry: Bool {
response.should_retry
}

public var success: Bool {
response.success
}
Expand Down
34 changes: 31 additions & 3 deletions ios/MullvadRustRuntime/include/mullvad_rust_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ typedef struct ExchangeCancelToken ExchangeCancelToken;

typedef struct RequestCancelHandle RequestCancelHandle;

typedef struct RetryStrategy RetryStrategy;

typedef struct SwiftApiContext {
const struct ApiContext *_0;
} SwiftApiContext;
Expand All @@ -34,15 +36,17 @@ typedef struct SwiftCancelHandle {
struct RequestCancelHandle *ptr;
} SwiftCancelHandle;

typedef struct SwiftRetryStrategy {
struct RetryStrategy *_0;
} SwiftRetryStrategy;

typedef struct SwiftMullvadApiResponse {
uint8_t *body;
uintptr_t body_size;
uint16_t status_code;
uint8_t *error_description;
uint8_t *server_response_code;
bool success;
bool should_retry;
uint64_t retry_after;
} SwiftMullvadApiResponse;

typedef struct CompletionCookie {
Expand Down Expand Up @@ -106,7 +110,8 @@ struct SwiftApiContext mullvad_api_init_new(const uint8_t *host,
* This function is not safe to call multiple times with the same `CompletionCookie`.
*/
struct SwiftCancelHandle mullvad_api_get_addresses(struct SwiftApiContext api_context,
void *completion_cookie);
void *completion_cookie,
struct SwiftRetryStrategy retry_strategy);

/**
* Called by the Swift side to signal that a Mullvad API call should be cancelled.
Expand Down Expand Up @@ -156,6 +161,29 @@ extern void mullvad_api_completion_finish(struct SwiftMullvadApiResponse respons
*/
void mullvad_response_drop(struct SwiftMullvadApiResponse response);

/**
* Creates a retry strategy that never retries after failure.
* The result needs to be consumed.
*/
struct SwiftRetryStrategy mullvad_api_retry_strategy_never(void);

/**
* Creates a retry strategy that retries `max_retries` times with a constant delay of `delay_sec`.
* The result needs to be consumed.
*/
struct SwiftRetryStrategy mullvad_api_retry_strategy_constant(uintptr_t max_retries,
uint64_t delay_sec);

/**
* Creates a retry strategy that retries `max_retries` times with a exponantially increating delay.
* The delay will never exceed `max_delay_sec`
* The result needs to be consumed.
*/
struct SwiftRetryStrategy mullvad_api_retry_strategy_exponential(uintptr_t max_retries,
uint64_t initial_sec,
uint32_t factor,
uint64_t max_delay_sec);

/**
* Initializes a valid pointer to an instance of `EncryptedDnsProxyState`.
*
Expand Down
4 changes: 4 additions & 0 deletions ios/MullvadTypes/Duration+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ extension Duration {
return timeInterval.isFinite
}

public var seconds: Int64 {
return components.seconds
}

public var timeInterval: TimeInterval {
return TimeInterval(components.seconds) + (TimeInterval(components.attoseconds) * 1e-18)
}
Expand Down
1 change: 1 addition & 0 deletions mullvad-ios/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ hyper-util = { workspace = true }
tower = { workspace = true }
tunnel-obfuscation = { path = "../tunnel-obfuscation" }
oslog = "0.2"
talpid-future = { path = "../talpid-future" }
talpid-types = { path = "../talpid-types" }
talpid-tunnel-config-client = { path = "../talpid-tunnel-config-client" }
mullvad-encrypted-dns-proxy = { path = "../mullvad-encrypted-dns-proxy" }
Expand Down
17 changes: 15 additions & 2 deletions mullvad-ios/src/api_client/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ use mullvad_api::{
rest::{self, MullvadRestHandle},
ApiProxy,
};
use talpid_future::retry::retry_future;

use super::{
cancellation::{RequestCancelHandle, SwiftCancelHandle},
completion::{CompletionCookie, SwiftCompletionHandler},
response::SwiftMullvadApiResponse,
retry_strategy::{RetryStrategy, SwiftRetryStrategy},
SwiftApiContext,
};

Expand All @@ -24,6 +26,7 @@ use super::{
pub unsafe extern "C" fn mullvad_api_get_addresses(
api_context: SwiftApiContext,
completion_cookie: *mut libc::c_void,
retry_strategy: SwiftRetryStrategy,
) -> SwiftCancelHandle {
let completion_handler = SwiftCompletionHandler::new(CompletionCookie(completion_cookie));

Expand All @@ -33,10 +36,11 @@ pub unsafe extern "C" fn mullvad_api_get_addresses(
};

let api_context = api_context.into_rust_context();
let retry_strategy = unsafe { retry_strategy.into_rust() };

let completion = completion_handler.clone();
let task = tokio_handle.clone().spawn(async move {
match mullvad_api_get_addresses_inner(api_context.rest_handle()).await {
match mullvad_api_get_addresses_inner(api_context.rest_handle(), retry_strategy).await {
Ok(response) => completion.finish(response),
Err(err) => {
log::error!("{err:?}");
Expand All @@ -50,9 +54,18 @@ pub unsafe extern "C" fn mullvad_api_get_addresses(

async fn mullvad_api_get_addresses_inner(
rest_client: MullvadRestHandle,
retry_strategy: RetryStrategy,
) -> Result<SwiftMullvadApiResponse, rest::Error> {
let api = ApiProxy::new(rest_client);
let response = api.get_api_addrs_response().await?;

let future_factory = || api.get_api_addrs_response();

let should_retry = |result: &Result<_, rest::Error>| match result {
Err(err) => err.is_network_error(),
Ok(_) => false,
};

let response = retry_future(future_factory, should_retry, retry_strategy.delays()).await?;

SwiftMullvadApiResponse::with_body(response).await
}
1 change: 1 addition & 0 deletions mullvad-ios/src/api_client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod api;
mod cancellation;
mod completion;
mod response;
mod retry_strategy;

#[repr(C)]
pub struct SwiftApiContext(*const ApiContext);
Expand Down
Loading
Loading