aws / aws-sdk-net

The official AWS SDK for .NET. For more information on the AWS SDK for .NET, see our web site:
http://aws.amazon.com/sdkfornet/
Apache License 2.0
2.05k stars 853 forks source link

AWSSDK.DynamoDB uses 'System.Net.Http.HttpClientHandler.Send' method which produces a 'System.PlatformNotSupportedException' in MAUI apps targeting Android/iOS - Use method 'SendAsync' instead, which is supported. #3397

Closed pbru87 closed 1 month ago

pbru87 commented 1 month ago

Describe the bug

When using the NuGet package "AWSSDK.DynamoDB v2 3.7.305.2" to make requests to AWS DynamoDB from an MAUI app (Android as well as iOS), the DynamoDbContext.LoadAsync() method fails with the following error message:

{System.PlatformNotSupportedException: Operation is not supported on this platform.
   at System.Net.Http.HttpClientHandler.Send(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpMessageInvoker.Send(HttpRequestMessage request, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.Send(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.Send(HttpRequestMessage request, HttpCompletionOption completionOption)
   at Amazon.Runtime.HttpWebRequestMessage.GetResponse() in C:\codebuild\tmp\output\src302497567\src\aws-sdk-net\sdk\src\Core\Amazon.Runtime\Pipeline\HttpHandler\_netstandard\HttpRequestMessageFactory.cs:line 492
   ...

That error does not occur under e.g. Windows OS.

I was able to nail down the problem to the following method within class Amazon.Runtime.HttpWebRequestMessage:

        /// <summary>
        /// Returns the HTTP response.
        /// </summary>
        /// <returns>The HTTP response.</returns>
#if NET8_0_OR_GREATER
        public IWebResponseData GetResponse()
        {
            try
            {
                var responseMessage = _httpClient.Send(_request, HttpCompletionOption.ResponseHeadersRead);

                return ProcessHttpResponseMessage(responseMessage);
            }
            catch (HttpRequestException httpException)
            {
                if (httpException.InnerException != null)
                {
                    if (httpException.InnerException is IOException)
                    {
                        throw httpException.InnerException;
                    }

                    if (httpException.InnerException is WebException)
                        throw httpException.InnerException;
                }

                throw;
            }
        }
#else
        public IWebResponseData GetResponse()
        {
            try
            {
                return this.GetResponseAsync(System.Threading.CancellationToken.None).Result;
            }
            catch (AggregateException e)
            {
                throw e.InnerException;
            }
        }
**#endif**

The problem seems to be that this method is using .Send() method instead of .SendAsync(). (The first is not supported for .NET 8 targeting Android/iOS, but the latter is.)

Expected Behavior

Being compatible with .NET 8 MAUI apps targeting iOS and Android - i.e. not throwing a 'System.PlatformNotSupportedException'.

Current Behavior

Throws a 'System.PlatformNotSupportedException'.

Reproduction Steps

Easiest way to reproduce the issue (without to bother with DynamoDB):

  1. Create a new MAUI app from Visual Studio 2022 templates.
  2. Add the below code and execute it somehow from Android/iOS emulators (which are included in Visual Studio).
  3. The code should throw a 'System.PlatformNotSupportedException'.
  4. Change the code by commenting out HttpResponseMessage response = client.Send(request); and commenting in HttpResponseMessage response = await client.SendAsync(request);.
  5. After re-executing, the new code with active 'SendAsync()' should succeed without throwing an exception.
            using (HttpClient client = new HttpClient())
            {
                try
                {
                    // Create the request message
                    var request = new HttpRequestMessage
                    {
                        Method = HttpMethod.Get,
                        RequestUri = new Uri("https://jsonplaceholder.typicode.com/posts")
                    };
                    request.Headers.Accept.Clear();
                    request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

                    // Send a GET request
                    HttpResponseMessage response = client.Send(request);
                    //HttpResponseMessage response = await client.SendAsync(request);

                    // Ensure the request was successful
                    response.EnsureSuccessStatusCode();

                    // Read and display the response body
                    string responseBody = await response.Content.ReadAsStringAsync();
                    Console.WriteLine(responseBody);
                }
                catch (HttpRequestException e)
                {
                    Console.WriteLine("\nException Caught!");
                    Console.WriteLine("Message :{0} ", e.Message);
                }
            }

Possible Solution

Within method 'Amazon.Runtime.HttpWebRequestMessage.GetResponse()' call '_httpClient.SendAsync' instead of '_httpClient.Send'. ('.Send()' is not supported for .NET 8 targeting Android/iOS, but 'SendAsync()' is.)

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

AWSSDK.DynamoDB v2 3.7.305.2

Targeted .NET Platform

.NET 8 (or more specifically: net8.0-android;net8.0-ios;net8.0;)

Operating System and version

Windows 11 (with Visual Studio 2022 including default Android/iOS emulators for MAUI apps)

normj commented 1 month ago

As I recall the DynamoDB context only does the sync Send method if an action triggers context object to need load the metadata of a table. Essentially a DescribeTable method is called. You can avoid the DescribeTable call by providing the table metadata to the context object. This blog post explains how to do that and going forward this will be our preferred mechanism moving away from the DescribeTable call. https://aws.amazon.com/blogs/developer/improved-dynamodb-initialization-patterns-for-the-aws-sdk-for-net/

pbru87 commented 1 month ago

Hi @normj, thank you very much for your quick response and the additional context, you added.

In the blog article you mentioned, it is written that:

The two high-level programming models predate the introduction of async/await in .NET, and offer both synchronous and asynchronous APIs.

But I do not understand why within method Amazon.Runtime.HttpWebRequestMessage.GetResponse() and only for .NET 8 and greater, calling the 'Send' method is preferred over 'SendAsync'. .NET 8 supports 'SendAsync', so we wouldn't break any downward-compatibilities.

For me, the current behavior seems buggy and the things described in the blog article seemingly just show some kind of unnecessary workaround.

It is also weird that the NuGet package works differently for distinct environments (works under Windows without the need for a workaround, but that's not the case for .NET 8 targeting Android/iOS). That's not an optimal use of .NET 8. With a minor code change, the NuGet package could work the same under all three environments (at least for the case described above in the issue description).

I look forward to your reply. Thanks for your support!

normj commented 1 month ago

.NET added the sync Send method in I believe .NET 5. That is in the middle of our SDK's .NET Core 3.1 and .NET 8 targets which is why we only switched to use the sync Send method in .NET 8. I admit I didn't know about Maui's limitation of not being able to call the sync Send method when I switched .NET 8 over. The previous pattern of calling SendAsync and then blocking has ugly threading / deadlock scenarios which are somewhat alleviated but using the newer sync Send method which is why we switched over to it. You can find many issues in this repo with users that ran into those deadlock issues and I'm hoping less users get tripped up by switching over the the sync Send method.

The whole mechanism of the library calling DescribeTable is problematic. We are working on a v4 of the SDK and the current plan is essentially deprecate the automatic DescribeTable mechanism and push users to use the patterns used in the blog post by default.

pbru87 commented 1 month ago

Hi @normj, thanks a lot again - also for explaining why you guys and gals went for the Send method specifically. Sounds plausible to me.

By the way, I was able to resolve my issue by just changing

var context = new DynamoDBContext(client);

to

var context = new DynamoDBContext(client, new DynamoDBContextConfig()
{
    DisableFetchingTableMetadata = true,
});

as also mentioned in the blog post you shared earlier.

Thanks!

github-actions[bot] commented 1 month ago

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.