Merge pull request #916 from microsoftgraph/andrueastman/fixBatch

fix: resolved error handling of larger batch request message

src/Microsoft.Graph.Core/Exceptions/ErrorConstants.cs

@@ -8,82 +8,30 @@ internal static class ErrorConstants
     {
         internal static class Codes
         {
-            internal static string GeneralException = "generalException";
-
-            internal static string InvalidRequest = "invalidRequest";
-
-            internal static string ItemNotFound = "itemNotFound";
-
-            internal static string NotAllowed = "notAllowed";
-
-            internal static string Timeout = "timeout";
-
-            internal static string TooManyRedirects = "tooManyRedirects";
-
-            internal static string TooManyRetries = "tooManyRetries";
-
-            internal static string MaximumValueExceeded = "MaximumValueExceeded";
-
-            internal static string InvalidArgument = "invalidArgument";
-
-            internal const string TemporarilyUnavailable = "temporarily_unavailable";
+            internal const string GeneralException = "generalException";
         }
 
         internal static class Messages
         {
-            internal static string AuthenticationProviderMissing = "Authentication provider is required before sending a request.";
-
-            internal static string BaseUrlMissing = "Base URL cannot be null or empty.";
-
-            internal static string InvalidTypeForDateConverter = "DateConverter can only serialize objects of type Date.";
-
-            internal static string InvalidTypeForDateTimeOffsetConverter = "DateTimeOffsetConverter can only serialize objects of type DateTimeOffset.";
-
-            internal static string LocationHeaderNotSetOnRedirect = "Location header not present in redirection response.";
-
-            internal static string OverallTimeoutCannotBeSet = "Overall timeout cannot be set after the first request is sent.";
-
-            internal static string RequestTimedOut = "The request timed out.";
-
-            internal static string RequestUrlMissing = "Request URL is required to send a request.";
-
-            internal static string TooManyRedirectsFormatString = "More than {0} redirects encountered while sending the request.";
-
-            internal static string TooManyRetriesFormatString = "More than {0} retries encountered while sending the request.";
-
-            internal static string UnableToCreateInstanceOfTypeFormatString = "Unable to create an instance of type {0}.";
-
-            internal static string UnableToDeserializeDate = "Unable to deserialize the returned Date.";
-
-            internal static string UnableToDeserializeDateTimeOffset = "Unable to deserialize the returned DateDateTimeOffset.";
-
-            internal static string UnexpectedExceptionOnSend = "An error occurred sending the request.";
-
-            internal static string UnexpectedExceptionResponse = "Unexpected exception returned from the service.";
-
-            internal static string MaximumValueExceeded = "{0} exceeds the maximum value of {1}.";
-
-            internal static string NullParameter = "{0} parameter cannot be null.";
-
-            internal static string UnableToDeserializeContent = "Unable to deserialize content.";
+            internal const string MaximumValueExceeded = "{0} exceeds the maximum value of {1}.";
 
-            internal static string InvalidDependsOnRequestId = "Corresponding batch request id not found for the specified dependsOn relation.";
+            internal const string NullParameter = "{0} parameter cannot be null.";
 
-            internal static string ExpiredUploadSession = "Upload session expired. Upload cannot resume";
+            internal const string UnableToDeserializeContent = "Unable to deserialize content.";
 
-            internal static string NoResponseForUpload = "No Response Received for upload.";
+            internal const string InvalidDependsOnRequestId = "Corresponding batch request id not found for the specified dependsOn relation.";
 
-            internal static string NullValue = "{0} cannot be null.";
+            internal const string ExpiredUploadSession = "Upload session expired. Upload cannot resume";
 
-            internal static string UnexpectedMsalException = "Unexpected exception returned from MSAL.";
+            internal const string NoResponseForUpload = "No Response Received for upload.";
 
-            internal static string UnexpectedException = "Unexpected exception occured while authenticating the request.";
+            internal const string MissingRetryAfterHeader = "Missing retry after header.";
 
-            internal static string MissingRetryAfterHeader = "Missing retry after header.";
+            internal const string PageIteratorRequestError = "Error occured when making a request with the page iterator. See inner exception for more details.";
 
-            internal static string PageIteratorRequestError = "Error occured when making a request with the page iterator. See inner exception for more details.";
+            internal const string BatchRequestError = "Error occured when making the batch request. See inner exception for more details.";
 
-            public static string InvalidProxyArgument = "Proxy cannot be set more once. Proxy can only be set on the proxy or defaultHttpHandler argument and not both.";
+            internal const string InvalidProxyArgument = "Proxy cannot be set more once. Proxy can only be set on the proxy or defaultHttpHandler argument and not both.";
         }
     }
 }

src/Microsoft.Graph.Core/Extensions/IParseNodeExtensions.cs

@@ -0,0 +1,16 @@
+using Microsoft.Kiota.Abstractions.Serialization;
+
+namespace Microsoft.Graph;
+
+/// <summary>
+/// Extension helpers for the <see cref="IParseNode"/>
+/// </summary>
+public static class ParseNodeExtensions
+{
+    internal static string GetErrorMessage(this IParseNode responseParseNode)
+    {
+        var errorParseNode = responseParseNode.GetChildNode("error");
+        // concatenate the error code and message
+        return $"{errorParseNode?.GetChildNode("code")?.GetStringValue()} : {errorParseNode?.GetChildNode("message")?.GetStringValue()}";
+    }
+}

src/Microsoft.Graph.Core/Requests/BatchRequestBuilder.cs

@@ -7,10 +7,12 @@ namespace Microsoft.Graph.Core.Requests
     using System;
     using System.Collections.Generic;
     using System.Net.Http;
+    using System.Text.Json;
     using System.Threading;
     using System.Threading.Tasks;
     using Microsoft.Kiota.Abstractions;
     using Microsoft.Kiota.Abstractions.Serialization;
+    using Microsoft.Kiota.Serialization.Json;
 
     /// <summary>
     /// The type BatchRequestBuilder
@@ -58,7 +60,9 @@ public async Task<BatchResponseContent> PostAsync(BatchRequestContent batchReque
             var nativeResponseHandler = new NativeResponseHandler();
             requestInfo.SetResponseHandler(nativeResponseHandler);
             await this.RequestAdapter.SendNoContentAsync(requestInfo, cancellationToken: cancellationToken);
-            return new BatchResponseContent(nativeResponseHandler.Value as HttpResponseMessage, errorMappings);
+            var httpResponseMessage = nativeResponseHandler.Value as HttpResponseMessage;
+            await ThrowIfFailedResponseAsync(httpResponseMessage, cancellationToken);
+            return new BatchResponseContent(httpResponseMessage, errorMappings);
         }
 
         /// <summary>
@@ -99,5 +103,34 @@ public async Task<RequestInformation> ToPostRequestInformationAsync(BatchRequest
             requestInfo.Headers.Add("Content-Type", CoreConstants.MimeTypeNames.Application.Json);
             return requestInfo;
         }
+
+        private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpResponseMessage, CancellationToken cancellationToken)
+        {
+            if (httpResponseMessage.IsSuccessStatusCode) return;
+
+            if (httpResponseMessage is { Content.Headers.ContentType.MediaType: string contentTypeMediaType } && contentTypeMediaType.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase))
+            {
+#if NET5_0_OR_GREATER
+                using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false);
+#else
+                using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false);
+#endif
+                using var document = await JsonDocument.ParseAsync(responseContent, cancellationToken: cancellationToken).ConfigureAwait(false);
+                var parsable = new JsonParseNode(document.RootElement);
+                throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, new Exception(parsable.GetErrorMessage()));
+            }
+
+            var responseStringContent = string.Empty;
+            if (httpResponseMessage.Content != null)
+            {
+#if NET5_0_OR_GREATER
+                responseStringContent = await httpResponseMessage.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
+#else
+                responseStringContent = await httpResponseMessage.Content.ReadAsStringAsync().ConfigureAwait(false);
+#endif
+            }
+
+            throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, responseStringContent);
+        }
     }
 }

src/Microsoft.Graph.Core/Requests/Content/BatchResponseContent.cs

@@ -32,7 +32,9 @@ public class BatchResponseContent
         public BatchResponseContent(HttpResponseMessage httpResponseMessage, Dictionary<string, ParsableFactory<IParsable>> errorMappings = null)
         {
             this.batchResponseMessage = httpResponseMessage ?? throw new ArgumentNullException(nameof(httpResponseMessage));
-            this.apiErrorMappings = errorMappings ?? new();
+            this.apiErrorMappings = errorMappings ?? new Dictionary<string, ParsableFactory<IParsable>>(StringComparer.OrdinalIgnoreCase) {
+                {"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.BatchRequestError, new Exception(parsable.GetErrorMessage())) }
+            };
         }
 
         /// <summary>

src/Microsoft.Graph.Core/Tasks/PageIterator.cs

@@ -115,8 +115,7 @@ public static PageIterator<TEntity, TCollectionPage> CreatePageIterator(IRequest
                 _processPageItemCallback = callback,
                 _requestConfigurator = requestConfigurator,
                 _errorMapping = errorMapping ?? new Dictionary<string, ParsableFactory<IParsable>>(StringComparer.OrdinalIgnoreCase) {
-                    {"4XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) },
-                    {"5XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) }
+                    {"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(parsable.GetErrorMessage())) }
                 },
                 State = PagingState.NotStarted
             };
@@ -172,15 +171,14 @@ public static PageIterator<TEntity, TCollectionPage> CreatePageIterator(IRequest
                 _asyncProcessPageItemCallback = asyncCallback,
                 _requestConfigurator = requestConfigurator,
                 _errorMapping = errorMapping ?? new Dictionary<string, ParsableFactory<IParsable>>(StringComparer.OrdinalIgnoreCase) {
-                    {"4XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) },
-                    {"5XX", (parsable) =>new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) },
+                    {"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(parsable.GetErrorMessage())) }
                 },
                 State = PagingState.NotStarted
             };
         }
 
         /// <summary>
-        /// Iterate across the content of a a single results page with the callback.
+        /// Iterate across the content of a single results page with the callback.
         /// </summary>
         /// <returns>A boolean value that indicates whether the callback cancelled 
         /// iterating across the page results or whether there are more pages to page. 
@@ -393,13 +391,6 @@ private static string ExtractNextLinkFromParsable(TCollectionPage parsableCollec
             // the next link property may not be defined in the response schema so we also check its presence in the additional data bag
             return parsableCollection.AdditionalData.TryGetValue(CoreConstants.OdataInstanceAnnotations.NextLink, out var nextLink) ? nextLink.ToString() : string.Empty;
         }
-
-        private static string GetErrorMessageFromParsable(IParseNode responseParseNode)
-        {
-            var errorParseNode = responseParseNode.GetChildNode("error");
-            // concatenate the error code and message
-            return $"{errorParseNode?.GetChildNode("code")?.GetStringValue()} : {errorParseNode?.GetChildNode("message")?.GetStringValue()}";
-        }
     }
 
     /// <summary>