BatchResponseContent and BatchResponseContentCollection should have validate success

[svrooij]

Batching should make your application a lot faster, but the SDK has no way to check if all modifications are successful without checking each individual request.

I'm also the creator of #613 so I know a thing or two on batching. This new proposal needs addition input, because I'm not sure on the solution either.

Please provide the following (and please check them off the list with [x]) before submitting this issue:

• Expected behavior. Please provide links to the specific Microsoft Graph documentation you used to determine the expected behavior.
• Actual behavior. Provide error codes, stack information, and a Fiddler capture of the request and response (please remove personally identifiable information before posting).
• Steps to reproduce the behavior. Include your code, IDE versions, client library versions, and any other information that might be helpful to understand your scenario.

Expected behavior

If you added 15 items to a batch and it's executed I would expect there to be a way to check if all individual requests have a success response code (200, 202 or 204).

Something like:

var batchCollection = new BatchRequestContentCollection(graphClient);
foreach (var task in tasks)
{
    await batchCollection.AddBatchRequestStepAsync(graphClient.Me.Todo.Lists[listId].Tasks.ToPostRequestInformation(task));
}
var result = await graphClient.Batch.PostAsync(batchCollection);
var success = await result.ValidateSuccess();

And maybe even return the failed requests somehow for retry.

Actual behavior

var success = await result.ValidateSuccess(); is not available.

Steps to reproduce the behavior

Run this program and see the batch execute successfully, while individual requests not all succeed (16 of the 20 requests are blocked with a 429 error code).

[andrueastman]

Thanks for raising this @svrooij,

To understand the issue better, would this be the equivalent of doing the following?

            var responses = await result.GetResponsesAsync();
            var allReponsesSuccessFull = responses.Any( response => !response.Value.IsSuccessStatusCode);//all the responses are successfull?

            var failedResponses = responses.Where(response => !response.Value.IsSuccessStatusCode);//the responses which do not have a success code.

[svrooij]

I was worried on the message in the xml docs all httpresponsemessage objects need to be disposed

I thinkg the sdk team decided to convert all the small parts in the batch esponse object to HttpResponseMessages but that does not really make it easier for doing these basic checks. I'm worried on the overhead. Since checking the status code (which is just there in the http response) does not involve reading/parsing the body.

And this seems correct for the first part, the second part is to have it return the requests that failed not the responses. So for a batch where 16 out of 20 requests fail (batching Todo tasks only accepts 4 apparently) I would want to get all the 16 requests that failed to do a retry.

Maybe the Batch limit for the BatchRequestContentCollection should be configurable with a default of 20. So I could still use the batching functionally even though it's not as efficient as I wanted.