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

feat(clients): endpoint level timeout part 2 [skip-bc] #4318

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ public InternalRequestOptions(RequestOptions options = null)

CustomPathParameters = new Dictionary<string, string>();
PathParameters = new Dictionary<string, string>();
Timeout = options?.Timeout;
ConnectTimeout = options?.ConnectTimeout;
ReadTimeout = options?.ReadTimeout;
WriteTimeout = options?.WriteTimeout;
}

public void AddQueryParameter(string key, object value)
Expand Down Expand Up @@ -76,9 +78,19 @@ public void AddCustomQueryParameters(IDictionary<string, object> values)
public object Data { get; set; }

/// <summary>
/// Request timeout
/// Request read timeout
/// </summary>
public TimeSpan? Timeout { get; set; }
public TimeSpan? ReadTimeout { get; set; }

/// <summary>
/// Request write timeout
/// </summary>
public TimeSpan? WriteTimeout { get; set; }

/// <summary>
/// Request connect timeout
/// </summary>
public TimeSpan? ConnectTimeout { get; set; }

/// <summary>
/// Enforce the Read Transporter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,35 @@ public RequestOptionBuilder AddExtraQueryParameters(string key, object value)
}

/// <summary>
/// Set the request timeout
/// Set the request read timeout
/// </summary>
/// <param name="timeout"></param>
/// <returns></returns>
public RequestOptionBuilder SetTimeout(TimeSpan timeout)
public RequestOptionBuilder SetReadTimeout(TimeSpan timeout)
{
_options.Timeout = timeout;
_options.ReadTimeout = timeout;
return this;
}

/// <summary>
/// Set the request write timeout
/// </summary>
/// <param name="timeout"></param>
/// <returns></returns>
public RequestOptionBuilder SetWriteTimeout(TimeSpan timeout)
{
_options.WriteTimeout = timeout;
return this;
}

/// <summary>
/// Set the request connect timeout
/// </summary>
/// <param name="timeout"></param>
/// <returns></returns>
public RequestOptionBuilder SetConnectTimeout(TimeSpan timeout)
{
_options.ConnectTimeout = timeout;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,19 @@ public class RequestOptions
public IDictionary<string, object> QueryParameters { get; set; }

/// <summary>
/// Request timeout in seconds
/// Request timeout
/// </summary>
public TimeSpan? Timeout { get; set; }
public TimeSpan? ReadTimeout { get; set; }

/// <summary>
/// Request timeout
/// </summary>
public TimeSpan? WriteTimeout { get; set; }

/// <summary>
/// Request timeout
/// </summary>
public TimeSpan? ConnectTimeout { get; set; }

/// <summary>
/// Constructs a new instance of <see cref="RequestOptions"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private async Task<TResult> ExecuteRequestAsync<TResult, TData>(HttpMethod metho
request.Uri = BuildUri(host, uri, requestOptions?.CustomPathParameters, requestOptions?.PathParameters,
requestOptions?.QueryParameters);
var requestTimeout =
TimeSpan.FromTicks((requestOptions?.Timeout ?? GetTimeOut(callType)).Ticks * (host.RetryCount + 1));
TimeSpan.FromTicks((GetTimeOut(callType, requestOptions)).Ticks * (host.RetryCount + 1));

if (request.Body == null && (method == HttpMethod.Post || method == HttpMethod.Put))
{
Expand All @@ -137,7 +137,7 @@ private async Task<TResult> ExecuteRequestAsync<TResult, TData>(HttpMethod metho
}

var response = await _httpClient
.SendRequestAsync(request, requestTimeout, _algoliaConfig.ConnectTimeout ?? Defaults.ConnectTimeout, ct)
.SendRequestAsync(request, requestTimeout, requestOptions?.ConnectTimeout ?? _algoliaConfig.ConnectTimeout ?? Defaults.ConnectTimeout, ct)
.ConfigureAwait(false);

_errorMessage = response.Error;
Expand Down Expand Up @@ -280,13 +280,14 @@ private static Uri BuildUri(StatefulHost host, string baseUri,
/// Compute the request timeout with the given call type and configuration
/// </summary>
/// <param name="callType"></param>
/// <param name="requestOptions"></param>
/// <returns></returns>
private TimeSpan GetTimeOut(CallType callType)
private TimeSpan GetTimeOut(CallType callType, InternalRequestOptions requestOptions = null)
{
return callType switch
{
CallType.Read => _algoliaConfig.ReadTimeout ?? Defaults.ReadTimeout,
CallType.Write => _algoliaConfig.WriteTimeout ?? Defaults.WriteTimeout,
CallType.Read => requestOptions?.ReadTimeout ?? _algoliaConfig.ReadTimeout ?? Defaults.ReadTimeout,
CallType.Write => requestOptions?.WriteTimeout ?? _algoliaConfig.WriteTimeout ?? Defaults.WriteTimeout,
_ => Defaults.WriteTimeout
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,4 +121,12 @@ class DioRequester implements Requester {
void setClientApiKey(String apiKey) {
_authInterceptor.apiKey = apiKey;
}

@override
get connectTimeout => _client.options.connectTimeout;

@override
void setConnectTimeout(Duration connectTimeout) {
_client.options.connectTimeout = connectTimeout;
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/// Represents options for configuring a request to an endpoint.
final class RequestOptions {
/// The write timeout for the request in milliseconds.
/// The write timeout for the request.
final Duration? writeTimeout;

/// The read timeout for the request in milliseconds.
/// The read timeout for the request.
final Duration? readTimeout;

/// The connect timeout for the request.
final Duration? connectTimeout;

/// Header names to their respective values to be sent with the request.
final Map<String, dynamic> headers;

Expand All @@ -18,18 +21,35 @@ final class RequestOptions {
const RequestOptions({
this.writeTimeout,
this.readTimeout,
this.connectTimeout,
this.headers = const {},
this.urlParameters = const {},
this.body,
});

RequestOptions operator +(RequestOptions? other) {
if (other == null) {
return this;
}

return RequestOptions(
writeTimeout: other.writeTimeout ?? writeTimeout,
readTimeout: other.readTimeout ?? readTimeout,
connectTimeout: other.connectTimeout ?? connectTimeout,
headers: {...headers, ...other.headers},
urlParameters: {...urlParameters, ...other.urlParameters},
body: other.body ?? body,
);
}

@override
bool operator ==(Object other) =>
identical(this, other) ||
other is RequestOptions &&
runtimeType == other.runtimeType &&
writeTimeout == other.writeTimeout &&
readTimeout == other.readTimeout &&
connectTimeout == other.connectTimeout &&
headers == other.headers &&
urlParameters == other.urlParameters &&
body == other.body;
Expand All @@ -38,12 +58,13 @@ final class RequestOptions {
int get hashCode =>
writeTimeout.hashCode ^
readTimeout.hashCode ^
connectTimeout.hashCode ^
headers.hashCode ^
urlParameters.hashCode ^
body.hashCode;

@override
String toString() {
return 'RequestOptions{writeTimeout: $writeTimeout, readTimeout: $readTimeout, headers: $headers, urlParameters: $urlParameters, body: $body}';
return 'RequestOptions{writeTimeout: $writeTimeout, readTimeout: $readTimeout, connectTimeout: $connectTimeout, headers: $headers, urlParameters: $urlParameters, body: $body}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ abstract class Requester {
/// Allows to switch the API key used to authenticate requests.
void setClientApiKey(String apiKey);

/// Allows to customise the connect timeout for the requester.
get connectTimeout => null;
void setConnectTimeout(Duration connectTimeout);

/// Closes any underlying resources that the Requester might be using.
///
/// By default, it does nothing (no-op), but it can be implemented to handle resource cleanup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,13 @@ final class RetryStrategy {
final List<AlgoliaException> errors = [];
for (final host in hosts) {
final httpRequest = _buildRequest(host, request, callType, options);
final requesterConnectTimeout = requester.connectTimeout;
if (options?.connectTimeout != null) {
requester.setConnectTimeout(options!.connectTimeout!);
}
try {
final response = await requester.perform(httpRequest);
requester.setConnectTimeout(requesterConnectTimeout);
return response.body ?? const {};
} on AlgoliaTimeoutException catch (e) {
host.timedOut();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,9 @@ type Configuration struct {
Compression compression.Compression
ExposeIntermediateNetworkErrors bool
}

type RequestConfiguration struct {
ReadTimeout *time.Duration
WriteTimeout *time.Duration
ConnectTimeout *time.Duration
}
26 changes: 23 additions & 3 deletions clients/algoliasearch-client-go/algolia/transport/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func New(cfg Configuration) *Transport {
return transport
}

func (t *Transport) Request(ctx context.Context, req *http.Request, k call.Kind) (*http.Response, []byte, error) {
func (t *Transport) Request(ctx context.Context, req *http.Request, k call.Kind, c RequestConfiguration) (*http.Response, []byte, error) {
var intermediateNetworkErrors []error

// Add Content-Encoding header, if needed
Expand All @@ -59,9 +59,29 @@ func (t *Transport) Request(ctx context.Context, req *http.Request, k call.Kind)
// before the early returns, but when we do so, we do it **after**
// reading the body content of the response. Otherwise, a `context
// cancelled` error may happen when the body is read.
perRequestCtx, cancel := context.WithTimeout(ctx, h.timeout)
var (
ctxTimeout time.Duration
connectTimeout time.Duration
)

switch {
case k == call.Read && c.ReadTimeout != nil:
ctxTimeout = *c.ReadTimeout
case k == call.Write && c.WriteTimeout != nil:
ctxTimeout = *c.WriteTimeout
default:
ctxTimeout = h.timeout
}

if c.ConnectTimeout != nil {
connectTimeout = *c.ConnectTimeout
} else {
connectTimeout = t.connectTimeout
}

perRequestCtx, cancel := context.WithTimeout(ctx, ctxTimeout)
req = req.WithContext(perRequestCtx)
res, err := t.request(req, h, h.timeout, t.connectTimeout)
res, err := t.request(req, h, ctxTimeout, connectTimeout)

code := 0
if res != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import java.util.concurrent.ExecutorService;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import org.jetbrains.annotations.Nullable;
import javax.annotation.Nullable;

/**
* Represents a base client for making API requests. The client uses a {@link Requester} for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;

/**
* Request options are used to pass extra parameters, headers, timeout to the request. Parameters
Expand All @@ -15,6 +16,7 @@ public final class RequestOptions {
private final Map<String, String> queryParameters = new HashMap<>();
private Duration readTimeout;
private Duration writeTimeout;
private Duration connectTimeout;

public RequestOptions addExtraHeader(String key, Object value) {
if (value == null) return this;
Expand Down Expand Up @@ -62,6 +64,32 @@ public RequestOptions setWriteTimeout(Duration writeTimeout) {
return this;
}

public Duration getConnectTimeout() {
return connectTimeout;
}

public RequestOptions setConnectTimeout(Duration connectTimeout) {
this.connectTimeout = connectTimeout;
return this;
}

// `this` will be merged in `other`. Values in `other` will take precedence over `this`'s.
public RequestOptions mergeRight(@Nullable RequestOptions other) {
if (other == null) {
return this;
}

RequestOptions requestOptions = new RequestOptions();
requestOptions.headers.putAll(this.headers);
requestOptions.headers.putAll(other.headers);
requestOptions.queryParameters.putAll(this.queryParameters);
requestOptions.queryParameters.putAll(other.queryParameters);
requestOptions.readTimeout = other.readTimeout != null ? other.readTimeout : this.readTimeout;
requestOptions.writeTimeout = other.writeTimeout != null ? other.writeTimeout : this.writeTimeout;
requestOptions.connectTimeout = other.connectTimeout != null ? other.connectTimeout : this.connectTimeout;
return requestOptions;
}

@Override
public String toString() {
return (
Expand All @@ -74,6 +102,8 @@ public String toString() {
readTimeout +
", writeTimeout=" +
writeTimeout +
", connectTimeout=" +
connectTimeout +
'}'
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ private OkHttpClient getOkHttpClient(RequestOptions requestOptions) {
if (requestOptions.getWriteTimeout() != null) {
builder.writeTimeout(requestOptions.getWriteTimeout());
}
if (requestOptions.getConnectTimeout() != null) {
builder.connectTimeout(requestOptions.getConnectTimeout());
}
return builder.build();
}

Expand Down
Loading
Loading