Use HttpClient.Send instead of AsyncHelpers.RunSync()

[Edgaras91]

I have noticed that the synchronous Execute methods are just wrapping native HttpClient.SendAsync,

public RestResponse Execute(RestRequest request) => AsyncHelpers.RunSync(() => ExecuteAsync(request));

I was wondering if there already is an implementation or if there is a plan to use the native HttpClient.Send, which was only introduced in NET 5 and onwards?

A some related discussion is here:
https://stackoverflow.com/questions/53529061/whats-the-right-way-to-use-httpclient-synchronously

[alexeyzimarev]

Is there a problem with current implementation? I am 99.9% sure that HttpClient sync methods are just wrappers over async API. Making RestSharp use .NET 5+ code requires more pragma #if's and there are already too many of those. If I understand if it actually makes sense, and it works better compared with the current implementation, I'd be ready to make a change.

[Edgaras91]

It was my general understanding that async code needs to "spread" through the calling stack and calling the async method synchronously is bad practice. We have this as a coding standard and we won't be using RestSharp for cases like this. If HttpClient is calling async code synchronously, then it would also be concerning, but less so because Microsoft would be managing it and maybe making it true synchronous one day too.

We simply want to save ourselves from hard-to-debug / not-replicable issues that this can cause.

[alexeyzimarev]

I am not sure. It seems like loads of work. Because it starts with a copy-paste implementation of ExecuteRequestAsync, which bubbles up to ExecuteAsync and DownloadDataAsync, which then bubbles up to all the extensions.

calling the async method synchronously is bad practice

I am not sure how using Send will help. Here's the code from System.Net.Http.HttpMessageHandlerStage.Send, which is called by SocketHttpMessageHandler.Send:

protected internal sealed override HttpResponseMessage Send(HttpRequestMessage request,
    CancellationToken cancellationToken)
{
    ValueTask<HttpResponseMessage> sendTask = SendAsync(request, async: false, cancellationToken);
    return sendTask.IsCompleted ?
        sendTask.Result :
        sendTask.AsTask().GetAwaiter().GetResult();
}

It is way less comprehensive compared to RestSharp async wrapper, which does a lot more than getting an awaiter and then getting it's result.

[Edgaras91]

I am not sure. It seems like loads of work. Because it starts with a copy-paste implementation of ExecuteRequestAsync, which bubbles up to ExecuteAsync and DownloadDataAsync, which then bubbles up to all the extensions.

calling the async method synchronously is bad practice

I am not sure how using Send will help. Here's the code from System.Net.Http.HttpMessageHandlerStage.Send, which is called by SocketHttpMessageHandler.Send:

protected internal sealed override HttpResponseMessage Send(HttpRequestMessage request,
    CancellationToken cancellationToken)
{
    ValueTask<HttpResponseMessage> sendTask = SendAsync(request, async: false, cancellationToken);
    return sendTask.IsCompleted ?
        sendTask.Result :
        sendTask.AsTask().GetAwaiter().GetResult();
}

It is way less comprehensive compared to RestSharp async wrapper, which does a lot more than getting an awaiter and then getting it's result.

I think what you sharing is not running async, and the call stack is not awaited.
See the async: false. If we go a little deeper, none of the await methods would be called:

response = async ?
    await base.SendAsync(request, cts.Token).ConfigureAwait(false) :
    base.Send(request, cts.Token);

    if (async)
    {
        await response.Content.LoadIntoBufferAsync(_maxResponseContentBufferSize, cts.Token).ConfigureAwait(false);
    }
    else
    {
        response.Content.LoadIntoBuffer(_maxResponseContentBufferSize, cts.Token);
    }

This makes me believe that while method signatures are async, the running code never awaits anything, staying on the same thread and is synchronous.