Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.38k stars 4.79k forks source link

[BUG] SubmitBatchAsync no longer throws on error in Azure.Data.Tables 12.0.0-beta.7 (regression?) #20175

Closed joelverhagen closed 3 years ago

joelverhagen commented 3 years ago

Describe the bug When using a transactional batch in Azure.Data.Tables, I believe it is a good behavior to throw an exception when the batch fails. This is symmetrical with the individual entity methods such as TableClient.UpdateEntityAsync.

Personally I feel that batch and non-batch operations should have the same error behavior (either exception or a response/result object describing the error).

This worked as expected in 12.0.0-beta.6 but no longer happens in 12.0.0-beta.7.

Expected behavior An exception should be thrown when TableTransactionalBatch.SubmitBatchAsync fails on one or more entity update.

Something like this from 12.0.0-beta.6:

Unhandled exception. Azure.RequestFailedException: Service request failed.
Status: 412 (Precondition Failed)

Content:
{"odata.error":{"code":"UpdateConditionNotSatisfied","message":{"lang":"en-US","value":"1:The update condition specified in the request was not satisfied.\nRequestId:3ebd3368-7002-0048-1c2e-2bfbe7000000\nTime:2021-04-06T21:49:56.4487686Z"}}}

Headers:
X-Content-Type-Options: REDACTED
Cache-Control: no-cache
DataServiceVersion: REDACTED
Content-Type: application/json;odata=minimalmetadata;streaming=true;charset=utf-8

You can retrieve the entity that caused the error by calling TryGetFailedEntityFromException and passing this exception instance to the method.
   at Azure.Data.Tables.TableRestClient.SendBatchRequestAsync(HttpMessage message, CancellationToken cancellationToken)
   at Azure.Data.Tables.TableTransactionalBatch.SubmitBatchAsyncInternal(Boolean async, CancellationToken cancellationToken)
   at Azure.Data.Tables.TableTransactionalBatch.SubmitBatchAsync(CancellationToken cancellationToken)
   at ConsoleApp8.Program.Main(String[] args) in C:\Users\joelv\Desktop\ConsoleApp8\ConsoleApp8\Program.cs:line 30
   at ConsoleApp8.Program.<Main>(String[] args)

Actual behavior (include Exception or Stack Trace) No exception is thrown, and a strange "unsubmitted request" error is reported when trying to read the specific response that should contain the error (e.g. a 412 on the entity with an etag mismatch).

Unhandled exception. System.InvalidOperationException: Response was not set, make sure SendAsync was called
   at Azure.Core.HttpMessage.get_Response()
   at Azure.Data.Tables.Models.TableBatchResponse.GetResponseForEntity(String rowKey)
   at ConsoleApp8.Program.Main(String[] args) in C:\Users\joelv\Desktop\ConsoleApp8\ConsoleApp8\Program.cs:line 32
   at ConsoleApp8.Program.<Main>(String[] args)

To Reproduce Steps to reproduce the behavior (include a code snippet, screenshot, or any additional information that might help us reproduce the issue)

using System;
using System.Threading.Tasks;
using Azure.Data.Tables;
using Azure.Data.Tables.Models;

namespace ConsoleApp8
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var connectionString = "AccountName=REDACTED;AccountKey=REDACTED";
            var tableName = "test";

            var table = new TableServiceClient(connectionString).GetTableClient(tableName);
            var entityA = new TableEntity("zaphod", "breedlebrox-1");
            var entityB = new TableEntity("zaphod", "breedlebrox-2");
            var responseA = await table.UpsertEntityAsync(entityA, mode: TableUpdateMode.Replace);
            var responseB = await table.UpsertEntityAsync(entityB, mode: TableUpdateMode.Replace);

            // Modify the second entity to change the remote etag.
            await table.UpsertEntityAsync(new TableEntity("zaphod", "breedlebrox-2"), mode: TableUpdateMode.Replace);

            // To test individual failures:
            // await table.UpdateEntityAsync(entityB, responseB.Headers.ETag.Value, mode: TableUpdateMode.Replace);

            var batch = table.CreateTransactionalBatch("zaphod");
            batch.UpdateEntity(entityA, responseA.Headers.ETag.Value);
            batch.UpdateEntity(entityB, responseB.Headers.ETag.Value);

            TableBatchResponse response = await batch.SubmitBatchAsync();
            Console.WriteLine("No exceptions!");
            var batchResponseB = response.GetResponseForEntity(entityB.RowKey);
            Console.WriteLine(batchResponseB.Status);
        }
    }
}

Environment:

jsquire commented 3 years ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

christothes commented 3 years ago

Thank you, @joelverhagen! This was, in fact, a regression that was missed due to a bug in a test.