Skip to content

Commit de75d5c

Browse files
andrueastmanandrueastman
authored
Merge pull request #916 from microsoftgraph/andrueastman/fixBatch
fix: resolved error handling of larger batch request message
2 parents 60d7c86 + 2213321 commit de75d5c

File tree

6 files changed

+186
-77
lines changed

6 files changed

+186
-77
lines changed

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

Lines changed: 11 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -8,82 +8,30 @@ internal static class ErrorConstants
88
{
99
internal static class Codes
1010
{
11-
internal static string GeneralException = "generalException";
12-
13-
internal static string InvalidRequest = "invalidRequest";
14-
15-
internal static string ItemNotFound = "itemNotFound";
16-
17-
internal static string NotAllowed = "notAllowed";
18-
19-
internal static string Timeout = "timeout";
20-
21-
internal static string TooManyRedirects = "tooManyRedirects";
22-
23-
internal static string TooManyRetries = "tooManyRetries";
24-
25-
internal static string MaximumValueExceeded = "MaximumValueExceeded";
26-
27-
internal static string InvalidArgument = "invalidArgument";
28-
29-
internal const string TemporarilyUnavailable = "temporarily_unavailable";
11+
internal const string GeneralException = "generalException";
3012
}
3113

3214
internal static class Messages
3315
{
34-
internal static string AuthenticationProviderMissing = "Authentication provider is required before sending a request.";
35-
36-
internal static string BaseUrlMissing = "Base URL cannot be null or empty.";
37-
38-
internal static string InvalidTypeForDateConverter = "DateConverter can only serialize objects of type Date.";
39-
40-
internal static string InvalidTypeForDateTimeOffsetConverter = "DateTimeOffsetConverter can only serialize objects of type DateTimeOffset.";
41-
42-
internal static string LocationHeaderNotSetOnRedirect = "Location header not present in redirection response.";
43-
44-
internal static string OverallTimeoutCannotBeSet = "Overall timeout cannot be set after the first request is sent.";
45-
46-
internal static string RequestTimedOut = "The request timed out.";
47-
48-
internal static string RequestUrlMissing = "Request URL is required to send a request.";
49-
50-
internal static string TooManyRedirectsFormatString = "More than {0} redirects encountered while sending the request.";
51-
52-
internal static string TooManyRetriesFormatString = "More than {0} retries encountered while sending the request.";
53-
54-
internal static string UnableToCreateInstanceOfTypeFormatString = "Unable to create an instance of type {0}.";
55-
56-
internal static string UnableToDeserializeDate = "Unable to deserialize the returned Date.";
57-
58-
internal static string UnableToDeserializeDateTimeOffset = "Unable to deserialize the returned DateDateTimeOffset.";
59-
60-
internal static string UnexpectedExceptionOnSend = "An error occurred sending the request.";
61-
62-
internal static string UnexpectedExceptionResponse = "Unexpected exception returned from the service.";
63-
64-
internal static string MaximumValueExceeded = "{0} exceeds the maximum value of {1}.";
65-
66-
internal static string NullParameter = "{0} parameter cannot be null.";
67-
68-
internal static string UnableToDeserializeContent = "Unable to deserialize content.";
16+
internal const string MaximumValueExceeded = "{0} exceeds the maximum value of {1}.";
6917

70-
internal static string InvalidDependsOnRequestId = "Corresponding batch request id not found for the specified dependsOn relation.";
18+
internal const string NullParameter = "{0} parameter cannot be null.";
7119

72-
internal static string ExpiredUploadSession = "Upload session expired. Upload cannot resume";
20+
internal const string UnableToDeserializeContent = "Unable to deserialize content.";
7321

74-
internal static string NoResponseForUpload = "No Response Received for upload.";
22+
internal const string InvalidDependsOnRequestId = "Corresponding batch request id not found for the specified dependsOn relation.";
7523

76-
internal static string NullValue = "{0} cannot be null.";
24+
internal const string ExpiredUploadSession = "Upload session expired. Upload cannot resume";
7725

78-
internal static string UnexpectedMsalException = "Unexpected exception returned from MSAL.";
26+
internal const string NoResponseForUpload = "No Response Received for upload.";
7927

80-
internal static string UnexpectedException = "Unexpected exception occured while authenticating the request.";
28+
internal const string MissingRetryAfterHeader = "Missing retry after header.";
8129

82-
internal static string MissingRetryAfterHeader = "Missing retry after header.";
30+
internal const string PageIteratorRequestError = "Error occured when making a request with the page iterator. See inner exception for more details.";
8331

84-
internal static string PageIteratorRequestError = "Error occured when making a request with the page iterator. See inner exception for more details.";
32+
internal const string BatchRequestError = "Error occured when making the batch request. See inner exception for more details.";
8533

86-
public static string InvalidProxyArgument = "Proxy cannot be set more once. Proxy can only be set on the proxy or defaultHttpHandler argument and not both.";
34+
internal const string InvalidProxyArgument = "Proxy cannot be set more once. Proxy can only be set on the proxy or defaultHttpHandler argument and not both.";
8735
}
8836
}
8937
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
using Microsoft.Kiota.Abstractions.Serialization;
2+
3+
namespace Microsoft.Graph;
4+
5+
/// <summary>
6+
/// Extension helpers for the <see cref="IParseNode"/>
7+
/// </summary>
8+
public static class ParseNodeExtensions
9+
{
10+
internal static string GetErrorMessage(this IParseNode responseParseNode)
11+
{
12+
var errorParseNode = responseParseNode.GetChildNode("error");
13+
// concatenate the error code and message
14+
return $"{errorParseNode?.GetChildNode("code")?.GetStringValue()} : {errorParseNode?.GetChildNode("message")?.GetStringValue()}";
15+
}
16+
}

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ namespace Microsoft.Graph.Core.Requests
77
using System;
88
using System.Collections.Generic;
99
using System.Net.Http;
10+
using System.Text.Json;
1011
using System.Threading;
1112
using System.Threading.Tasks;
1213
using Microsoft.Kiota.Abstractions;
1314
using Microsoft.Kiota.Abstractions.Serialization;
15+
using Microsoft.Kiota.Serialization.Json;
1416

1517
/// <summary>
1618
/// The type BatchRequestBuilder
@@ -58,7 +60,9 @@ public async Task<BatchResponseContent> PostAsync(BatchRequestContent batchReque
5860
var nativeResponseHandler = new NativeResponseHandler();
5961
requestInfo.SetResponseHandler(nativeResponseHandler);
6062
await this.RequestAdapter.SendNoContentAsync(requestInfo, cancellationToken: cancellationToken);
61-
return new BatchResponseContent(nativeResponseHandler.Value as HttpResponseMessage, errorMappings);
63+
var httpResponseMessage = nativeResponseHandler.Value as HttpResponseMessage;
64+
await ThrowIfFailedResponseAsync(httpResponseMessage, cancellationToken);
65+
return new BatchResponseContent(httpResponseMessage, errorMappings);
6266
}
6367

6468
/// <summary>
@@ -99,5 +103,34 @@ public async Task<RequestInformation> ToPostRequestInformationAsync(BatchRequest
99103
requestInfo.Headers.Add("Content-Type", CoreConstants.MimeTypeNames.Application.Json);
100104
return requestInfo;
101105
}
106+
107+
private static async Task ThrowIfFailedResponseAsync(HttpResponseMessage httpResponseMessage, CancellationToken cancellationToken)
108+
{
109+
if (httpResponseMessage.IsSuccessStatusCode) return;
110+
111+
if (httpResponseMessage is { Content.Headers.ContentType.MediaType: string contentTypeMediaType } && contentTypeMediaType.StartsWith(CoreConstants.MimeTypeNames.Application.Json, StringComparison.OrdinalIgnoreCase))
112+
{
113+
#if NET5_0_OR_GREATER
114+
using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false);
115+
#else
116+
using var responseContent = await httpResponseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false);
117+
#endif
118+
using var document = await JsonDocument.ParseAsync(responseContent, cancellationToken: cancellationToken).ConfigureAwait(false);
119+
var parsable = new JsonParseNode(document.RootElement);
120+
throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, new Exception(parsable.GetErrorMessage()));
121+
}
122+
123+
var responseStringContent = string.Empty;
124+
if (httpResponseMessage.Content != null)
125+
{
126+
#if NET5_0_OR_GREATER
127+
responseStringContent = await httpResponseMessage.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
128+
#else
129+
responseStringContent = await httpResponseMessage.Content.ReadAsStringAsync().ConfigureAwait(false);
130+
#endif
131+
}
132+
133+
throw new ServiceException(ErrorConstants.Messages.BatchRequestError, httpResponseMessage.Headers, (int)httpResponseMessage.StatusCode, responseStringContent);
134+
}
102135
}
103136
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ public class BatchResponseContent
3232
public BatchResponseContent(HttpResponseMessage httpResponseMessage, Dictionary<string, ParsableFactory<IParsable>> errorMappings = null)
3333
{
3434
this.batchResponseMessage = httpResponseMessage ?? throw new ArgumentNullException(nameof(httpResponseMessage));
35-
this.apiErrorMappings = errorMappings ?? new();
35+
this.apiErrorMappings = errorMappings ?? new Dictionary<string, ParsableFactory<IParsable>>(StringComparer.OrdinalIgnoreCase) {
36+
{"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.BatchRequestError, new Exception(parsable.GetErrorMessage())) }
37+
};
3638
}
3739

3840
/// <summary>

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

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,7 @@ public static PageIterator<TEntity, TCollectionPage> CreatePageIterator(IRequest
115115
_processPageItemCallback = callback,
116116
_requestConfigurator = requestConfigurator,
117117
_errorMapping = errorMapping ?? new Dictionary<string, ParsableFactory<IParsable>>(StringComparer.OrdinalIgnoreCase) {
118-
{"4XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) },
119-
{"5XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) }
118+
{"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(parsable.GetErrorMessage())) }
120119
},
121120
State = PagingState.NotStarted
122121
};
@@ -172,15 +171,14 @@ public static PageIterator<TEntity, TCollectionPage> CreatePageIterator(IRequest
172171
_asyncProcessPageItemCallback = asyncCallback,
173172
_requestConfigurator = requestConfigurator,
174173
_errorMapping = errorMapping ?? new Dictionary<string, ParsableFactory<IParsable>>(StringComparer.OrdinalIgnoreCase) {
175-
{"4XX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) },
176-
{"5XX", (parsable) =>new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(GetErrorMessageFromParsable(parsable))) },
174+
{"XXX", (parsable) => new ServiceException(ErrorConstants.Messages.PageIteratorRequestError,new Exception(parsable.GetErrorMessage())) }
177175
},
178176
State = PagingState.NotStarted
179177
};
180178
}
181179

182180
/// <summary>
183-
/// Iterate across the content of a a single results page with the callback.
181+
/// Iterate across the content of a single results page with the callback.
184182
/// </summary>
185183
/// <returns>A boolean value that indicates whether the callback cancelled
186184
/// iterating across the page results or whether there are more pages to page.
@@ -393,13 +391,6 @@ private static string ExtractNextLinkFromParsable(TCollectionPage parsableCollec
393391
// the next link property may not be defined in the response schema so we also check its presence in the additional data bag
394392
return parsableCollection.AdditionalData.TryGetValue(CoreConstants.OdataInstanceAnnotations.NextLink, out var nextLink) ? nextLink.ToString() : string.Empty;
395393
}
396-
397-
private static string GetErrorMessageFromParsable(IParseNode responseParseNode)
398-
{
399-
var errorParseNode = responseParseNode.GetChildNode("error");
400-
// concatenate the error code and message
401-
return $"{errorParseNode?.GetChildNode("code")?.GetStringValue()} : {errorParseNode?.GetChildNode("message")?.GetStringValue()}";
402-
}
403394
}
404395

405396
/// <summary>

0 commit comments

Comments
 (0)