Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Possible inconvenient implementation of timeout in OData.Client #3131

Closed
dotnetero opened this issue Nov 23, 2024 · 4 comments · Fixed by #3135
Closed

Possible inconvenient implementation of timeout in OData.Client #3131

dotnetero opened this issue Nov 23, 2024 · 4 comments · Fixed by #3135
Assignees
Labels
bug client only related to OData.Client

Comments

@dotnetero
Copy link

I'm working in a data collector, on a distributed system, and some methods take me between 2 to 5 minutes.
The databases have a size greater than 500 gb, and it is required to obtain 19 complex queries, doing the service test, it responds correctly, so on the server side everything works correctly.
Perform tests with console clients, winforms, wpf and blazor server, in all of them it happens that when assigning the timeout in 5 minutes, in all cases the thread is canceled at minute 1 with 40 seconds.

// timeout = 300 seconds.
IEvaluaDbUnitOfWork IUnitOfWorkFactory<IEvaluaDbUnitOfWork>.CreateUnitOfWork(string url, MergeOption mergeOption, int? timeout)
{
    return new EvaluaDbUnitOfWork(() => new Evalua.Default.Container(new Uri(url)) { MergeOption = mergeOption, Timeout = timeOut ?? 30, ReadWriteTimeout = timeOut ?? 30 });
}

Digging into the code of the OData.Client library, in the constructor of the HttpClientRequestMessage class, the HttpClient is initialized, and it has a default timeout of 1 minute and 40 seconds.

public HttpClientRequestMessage(DataServiceClientRequestMessageArgs args) 
    : base(args.ActualMethod)
{
    _messageStream = new MemoryStream();

    _abortRequestCancellationTokenSource = new CancellationTokenSource();

    IHttpClientFactory clientFactory = args.HttpClientFactory;
    if (clientFactory == null)
    {
        _client = _defaultClient;
    }
    else
    {
        try
        {
            _client = clientFactory.CreateClient();
        }
        catch
        {
            _messageStream.Dispose();
            throw;
        }
    }
            
    _contentHeaderValueCache = new Dictionary<string, string>();
    _effectiveHttpMethod = args.Method;
    _requestMessage = new HttpRequestMessage(new HttpMethod(this.ActualMethod), args.RequestUri);
    _timeout = _client.Timeout;

    // Now set the headers.
    foreach (KeyValuePair<string, string> keyValue in args.Headers)
    {
        this.SetHeader(keyValue.Key, keyValue.Value);
    }
}

The timeout set by the developer is respected in the DataServiceContext timeout property. But the HttpClient is never notified of this change, so its default value is always maintained.

private Task<HttpResponseMessage> CreateSendTask()
{
    // Only set the message content if the stream has been written to, otherwise
    // HttpClient will complain if it's a GET request.
    Byte[] messageContent = _messageStream.ToArray();
    if (messageContent.Length > 0)
    {
        _requestMessage.Content = new ByteArrayContent(messageContent);

        _messageStream.Dispose();

        // Apply cached "Content" header values
        foreach (KeyValuePair<string, string> contentHeader in _contentHeaderValueCache)
        {
            _requestMessage.Content.Headers.Add(contentHeader.Key, contentHeader.Value);
        }
    }

    _requestMessage.Method = new HttpMethod(_effectiveHttpMethod);
    // If the timeout value is still the default, don't schedule cancellation.
    // The timeout from the HttpClient will take effect.
    if (_isTimeoutProvidedByCaller)
    {
        _abortRequestCancellationTokenSource.CancelAfter(_timeout);
    }

    return _client.SendAsync(_requestMessage, _abortRequestCancellationTokenSource.Token);
}

Add a line of code, to set the timeout of the DataServiceContext to the HttpClient, prior to requesting data from the server, working properly according to the needs of each scenario.

if (_isTimeoutProvidedByCaller)
{
    _abortRequestCancellationTokenSource.CancelAfter(_timeout);
}

// *** Added Lines *** 
if (_timeout != _client.Timeout)
{
    _client.Timeout = _timeout;
}
return _client.SendAsync(_requestMessage, _abortRequestCancellationTokenSource.Token);

The problem I see, is that I don't have full knowledge of the entire library, and if it is necessary to make this change in the proper way, so as not to break something else.

Assemblies affected

Microsoft.OData.Client

Reproduce steps

  • Create an OData data service, and set a Task.Delay(150000) in a test method.
  • Create a client to consume the service using the Microsoft.OData.Client library.
  • Modify the timeout in the DataServiceContext constructor, with a value greater than 100 seconds.

When you run they should see the following result:

image

Expected result

To be able to modify the default value of the timeout in the DataServiceContext constructor, and that it is respected in each request made to the multiple services.

Actual result

Any request that exceeds the wait for the response in 1 minute and 40 seconds, is automatically canceled, as shown in the image above.

Additional detail

The HttpClient is never notified that the default timeout value has been changed.

@WanjohiSammy
Copy link
Member

@dotnetero Thanks for reporting this. As we investigate this further, please setup as following on the client side and try out:

  1. Create CustomHttpClientFactory
public class CustomHttpClientFactory : IHttpClientFactory
{
    private readonly HttpClient _httpClient;

    public CustomHttpClientFactory(HttpClient httpClient)
    {
        _httpClient = httpClient;
    }

    public HttpClient CreateClient(string name)
    {
        return _httpClient;
    }
}
  1. Create HttpClient and specify the Timeout
var httpClient = new HttpClient
{
    Timeout = TimeSpan.FromSeconds(160)
};

var httpClientFactory= new CustomHttpClientFactory(httpClient);

var context = new Container(new Uri("https://localhost:7214/odata"))
{
    HttpClientFactory = httpClientFactory
};

In your example, this will look like:

IEvaluaDbUnitOfWork IUnitOfWorkFactory<IEvaluaDbUnitOfWork>.CreateUnitOfWork(string url, MergeOption mergeOption, int? timeout)
{
    return new EvaluaDbUnitOfWork(() => new Evalua.Default.Container(new Uri(url)) { MergeOption = mergeOption, Timeout = timeOut ?? 30, ReadWriteTimeout = timeOut ?? 30, HttpClientFactory = httpClientFactory});
}

Let us know if this sort you out in the meantime.

@WanjohiSammy WanjohiSammy added bug client only related to OData.Client labels Nov 26, 2024
@WanjohiSammy
Copy link
Member

@dotnetero We will be removing the Timeout property and only allow this to be configured using HttpClient

@WanjohiSammy WanjohiSammy self-assigned this Nov 26, 2024
@WanjohiSammy
Copy link
Member

WanjohiSammy commented Nov 26, 2024

@dotnetero You can also use the following in ServiceCollection():

var services = new ServiceCollection();
services.AddHttpClient("", client =>  // Note that the httpClient is not named
{
    client.Timeout = TimeSpan.FromSeconds(160);
});

var serviceProvider = services.BuildServiceProvider();
var httpClientFactory = serviceProvider.GetRequiredService<IHttpClientFactory>();

IEvaluaDbUnitOfWork IUnitOfWorkFactory<IEvaluaDbUnitOfWork>.CreateUnitOfWork(string url, MergeOption mergeOption, int? timeout)
{
    return new EvaluaDbUnitOfWork(() => new Evalua.Default.Container(new Uri(url)) { MergeOption = mergeOption, Timeout = timeOut ?? 30, ReadWriteTimeout = timeOut ?? 30, HttpClientFactory = httpClientFactory});
}

@dotnetero
Copy link
Author

@WanjohiSammy I am grateful for the time and the examples you share, for the moment my projects are already stable.
I will stay tuned for the next updates, thanks to all the collaborators for the effort.
Nice day.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug client only related to OData.Client
Projects
None yet
2 participants