Skip to content

Commit

Permalink
Merge pull request #1022 from clement911/master
Browse files Browse the repository at this point in the history
Added ShopifyHttpException.RequestInfo, which is useful for logging/d…
  • Loading branch information
clement911 committed Feb 19, 2024
2 parents 924fbab + 1956d1f commit d8568fb
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 67 deletions.
4 changes: 2 additions & 2 deletions ShopifySharp.Tests/Infrastructure/ResponseClassifierTests.cs
Expand Up @@ -131,7 +131,7 @@ public void IsRetriableException_AllowsThreeRetriesForNonRateLimitedExceptions(i

private ShopifyRateLimitException CreateRateLimitException(ShopifyRateLimitReason reason)
{
return new ShopifyRateLimitException((HttpStatusCode)429,
return new ShopifyRateLimitException(string.Empty, (HttpStatusCode)429,
[],
"some-exception-message",
"some-raw-response-body",
Expand All @@ -142,7 +142,7 @@ private ShopifyRateLimitException CreateRateLimitException(ShopifyRateLimitReaso

private ShopifyHttpException CreateHttpException(int statusCode)
{
return new ShopifyHttpException((HttpStatusCode)statusCode,
return new ShopifyHttpException(string.Empty, (HttpStatusCode)statusCode,
[],
"some-exception-message",
"some-raw-response-body",
Expand Down
92 changes: 46 additions & 46 deletions ShopifySharp.Tests/ShopifyException_Tests.cs
Expand Up @@ -51,7 +51,7 @@ public void Throws_On_OAuth_Code_Used()

try
{
ShopifyService.CheckResponseExceptions(res, rawBody);
ShopifyService.CheckResponseExceptions(string.Empty, res, rawBody);
}
catch (ShopifyException e)
{
Expand Down Expand Up @@ -82,21 +82,21 @@ public async Task Throws_On_Error_String()
var req = client.SendAsync(msg);
response = await req;
rawBody = await response.Content.ReadAsStringAsync();
}

try
{
ShopifyService.CheckResponseExceptions(response, rawBody);
}
catch (ShopifyRateLimitException)
{
// Ignore this exception and retry the request.
// RateLimitExceptions may happen when all Exception tests are running and
// execution policies are retrying.
}
catch (ShopifyException e)
{
ex = e;
try
{
ShopifyService.CheckResponseExceptions(msg.ToString(), response, rawBody);
}
catch (ShopifyRateLimitException)
{
// Ignore this exception and retry the request.
// RateLimitExceptions may happen when all Exception tests are running and
// execution policies are retrying.
}
catch (ShopifyException e)
{
ex = e;
}
}
}
}
Expand Down Expand Up @@ -124,21 +124,21 @@ public async Task Throws_On_Error_Object()
var req = client.SendAsync(msg);
response = await req;
rawBody = await response.Content.ReadAsStringAsync();
}

try
{
ShopifyService.CheckResponseExceptions(response, rawBody);
}
catch (ShopifyRateLimitException)
{
// Ignore this exception and retry the request.
// RateLimitExceptions may happen when all Exception tests are running and
// execution policies are retrying.
}
catch (ShopifyException e)
{
ex = e;
try
{
ShopifyService.CheckResponseExceptions(msg.ToString(), response, rawBody);
}
catch (ShopifyRateLimitException)
{
// Ignore this exception and retry the request.
// RateLimitExceptions may happen when all Exception tests are running and
// execution policies are retrying.
}
catch (ShopifyException e)
{
ex = e;
}
}
}
}
Expand All @@ -148,7 +148,7 @@ public async Task Throws_On_Error_Object()
Assert.Equal("(400 Bad Request) order: Required parameter missing or invalid", ex.Message);

var error = ex.Errors.First();

Assert.Equal("order: Required parameter missing or invalid", error);
}

Expand Down Expand Up @@ -197,26 +197,26 @@ public async Task Throws_On_Error_Arrays()
while (ex == null)
{
// This request will return a response which looks like { errors: { "order" : [ "some error message" ] } }
using (var msg = PrepareRequest(HttpMethod.Post, "orders.json", new JsonContent(new {order })))
using (var msg = PrepareRequest(HttpMethod.Post, "orders.json", new JsonContent(new { order })))
{
var req = client.SendAsync(msg);
response = await req;
rawBody = await response.Content.ReadAsStringAsync();
}

try
{
ShopifyService.CheckResponseExceptions(response, rawBody);
}
catch (ShopifyRateLimitException)
{
// Ignore this exception and retry the request.
// RateLimitExceptions may happen when all Exception tests are running and
// execution policies are retrying.
}
catch (ShopifyException e)
{
ex = e;
try
{
ShopifyService.CheckResponseExceptions(msg.ToString(), response, rawBody);
}
catch (ShopifyRateLimitException)
{
// Ignore this exception and retry the request.
// RateLimitExceptions may happen when all Exception tests are running and
// execution policies are retrying.
}
catch (ShopifyException e)
{
ex = e;
}
}
}
}
Expand All @@ -227,7 +227,7 @@ public async Task Throws_On_Error_Arrays()
Assert.Equal("(422 Unprocessable Entity) order: Tax lines must be associated with either order or line item but not both", ex.Message);

var error = ex.Errors.First();

Assert.Equal("order: Tax lines must be associated with either order or line item but not both", error);
}

Expand Down
16 changes: 8 additions & 8 deletions ShopifySharp.Tests/ShopifyService_Tests.cs
Expand Up @@ -54,7 +54,7 @@ public void Returns_Message_Saying_Json_Could_Not_Be_Parsed()

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand All @@ -76,7 +76,7 @@ public void Returns_Message_Saying_There_Was_No_Json_To_Parse()

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand All @@ -98,7 +98,7 @@ public void Exception_Contains_Message_From_Error_Type_One()

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand All @@ -120,7 +120,7 @@ public void Exception_Contains_Message_From_Error_Type_Two()

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand All @@ -142,7 +142,7 @@ public void Exception_Contains_Message_From_Error_Type_Three()

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand All @@ -164,7 +164,7 @@ public void Exception_Contains_Message_From_Error_Type_Three_With_Multiple_Messa

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand All @@ -187,7 +187,7 @@ public void Exception_Contains_Message_From_Error_Type_Four()

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand All @@ -209,7 +209,7 @@ public void Exception_Contains_Message_From_Error_Type_Five()

try
{
ShopifyService.CheckResponseExceptions(res, json);
ShopifyService.CheckResponseExceptions(string.Empty, res, json);
}
catch (ShopifyException e)
{
Expand Down
4 changes: 4 additions & 0 deletions ShopifySharp/Infrastructure/RequestResult.cs
Expand Up @@ -6,6 +6,8 @@ namespace ShopifySharp
{
public class RequestResult<T>
{
public string RequestInfo { get; }

[Obsolete("This property is obsolete and will be removed in a future version of ShopifySharp. If you need to use the response headers, please use the " + nameof(ResponseHeaders) + " property instead.")]
public HttpResponseMessage Response { get; }

Expand All @@ -31,12 +33,14 @@ public RequestResult(HttpResponseMessage response, T result, string rawResult, s
}

public RequestResult(
string requestInfo,
HttpResponseMessage httpResponseMessage,
HttpResponseHeaders httpResponseHeaders,
T result,
string rawResult,
string rawLinkHeaderValue)
{
RequestInfo = requestInfo;
Response = httpResponseMessage;

Check warning on line 44 in ShopifySharp/Infrastructure/RequestResult.cs

View workflow job for this annotation

GitHub Actions / Build

'RequestResult<T>.Response' is obsolete: 'This property is obsolete and will be removed in a future version of ShopifySharp. If you need to use the response headers, please use the ResponseHeaders property instead.'

Check warning on line 44 in ShopifySharp/Infrastructure/RequestResult.cs

View workflow job for this annotation

GitHub Actions / Unit tests

'RequestResult<T>.Response' is obsolete: 'This property is obsolete and will be removed in a future version of ShopifySharp. If you need to use the response headers, please use the ResponseHeaders property instead.'
ResponseHeaders = httpResponseHeaders;
Result = result;
Expand Down
3 changes: 3 additions & 0 deletions ShopifySharp/Infrastructure/ShopifyHttpException.cs
Expand Up @@ -5,6 +5,7 @@
namespace ShopifySharp.Infrastructure;

public class ShopifyHttpException(
string requestInfo,
HttpStatusCode statusCode,
ICollection<string> errors,
string message,
Expand All @@ -23,4 +24,6 @@ public class ShopifyHttpException(

/// The X-Request-Id header returned by Shopify. This can be used when working with the Shopify support team to identify the failed request.
public new readonly string? RequestId = requestId;

public readonly string RequestInfo = requestInfo;
}
4 changes: 3 additions & 1 deletion ShopifySharp/Infrastructure/ShopifyRateLimitException.cs
@@ -1,20 +1,22 @@
#nullable enable
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using ShopifySharp.Infrastructure;

namespace ShopifySharp;

/// An exception thrown when an API call has reached Shopify's rate limit.
public class ShopifyRateLimitException(
string requestInfo,
HttpStatusCode statusCode,
ICollection<string> errors,
string message,
string rawResponseBody,
string? requestId,
ShopifyRateLimitReason reason,
int? retryAfterSeconds)
: ShopifyHttpException(statusCode, errors, message, rawResponseBody, requestId)
: ShopifyHttpException(requestInfo, statusCode, errors, message, rawResponseBody, requestId)
{
public readonly int? RetryAfterSeconds = retryAfterSeconds;

Expand Down
2 changes: 1 addition & 1 deletion ShopifySharp/Services/Graph/GraphService.cs
Expand Up @@ -137,7 +137,7 @@ protected virtual void CheckForErrors<T>(RequestResult<T> requestResult)
var message = errorList.FirstOrDefault() ?? "Unable to parse Shopify's error response, please inspect exception's RawBody property and report this issue to the ShopifySharp maintainers.";
var requestId = ParseRequestIdResponseHeader(requestResult.ResponseHeaders);

throw new ShopifyHttpException(HttpStatusCode.OK, errorList, message, requestResult.RawResult, requestId);
throw new ShopifyHttpException(requestResult.RequestInfo, HttpStatusCode.OK, errorList, message, requestResult.RawResult, requestId);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ShopifySharp/Services/Partner/PartnerService.cs
Expand Up @@ -73,7 +73,7 @@ private static void CheckForErrors(RequestResult<JToken> requestResult)

var message = errorList.FirstOrDefault() ?? "Unable to parse Shopify's error response, please inspect exception's RawBody property and report this issue to the ShopifySharp maintainers.";

throw new ShopifyHttpException(HttpStatusCode.OK, errorList, message, requestResult.RawResult, requestId);
throw new ShopifyHttpException(requestResult.RequestInfo, HttpStatusCode.OK, errorList, message, requestResult.RawResult, requestId);
}
}
}
12 changes: 6 additions & 6 deletions ShopifySharp/Services/ShopifyService.cs
Expand Up @@ -213,11 +213,11 @@ private string ReadLinkHeader(HttpResponseHeaders responseHeaders)
#endif
//Check for and throw exception when necessary.
CheckResponseExceptions(response, rawResult);
CheckResponseExceptions(baseRequestMessage.ToString(), response, rawResult);
var result = method == HttpMethod.Delete ? default : Serializer.Deserialize<T>(rawResult, rootElement, dateParseHandlingOverride);
return new RequestResult<T>(response, response.Headers, result, rawResult, ReadLinkHeader(response.Headers));
return new RequestResult<T>(baseRequestMessage.ToString(), response, response.Headers, result, rawResult, ReadLinkHeader(response.Headers));
}, cancellationToken, graphqlQueryCost);

return policyResult;
Expand Down Expand Up @@ -358,7 +358,7 @@ CancellationToken cancellationToken
/// </summary>
/// <param name="response">The response.</param>
/// <param name="rawResponse">The response body returned by Shopify.</param>
public static void CheckResponseExceptions(HttpResponseMessage response, string rawResponse)
public static void CheckResponseExceptions(string requestInfo, HttpResponseMessage response, string rawResponse)
{
// TODO: make this method protected virtual so inheriting members can override it (e.g. the PartnerService which is doing its own custom error checking right now)

Expand Down Expand Up @@ -406,7 +406,7 @@ public static void CheckResponseExceptions(HttpResponseMessage response, string
retryAfterSeconds = retryValue;
}

throw new ShopifyRateLimitException(code, errors.ToList(), rateExceptionMessage, rawResponse, requestId, reason, retryAfterSeconds);
throw new ShopifyRateLimitException(requestInfo, code, errors.ToList(), rateExceptionMessage, rawResponse, requestId, reason, retryAfterSeconds);
}

var contentType = response.Content.Headers.GetValues("Content-Type").FirstOrDefault();
Expand Down Expand Up @@ -446,7 +446,7 @@ public static void CheckResponseExceptions(HttpResponseMessage response, string
errors = [];
}

throw new ShopifyHttpException(code, errors.ToList(), exceptionMessage, rawResponse, requestId);
throw new ShopifyHttpException(requestInfo, code, errors.ToList(), exceptionMessage, rawResponse, requestId);
}

var message = $"({statusMessage}) Shopify returned {statusMessage}, but there was no JSON to parse into an error message.";
Expand All @@ -455,7 +455,7 @@ public static void CheckResponseExceptions(HttpResponseMessage response, string
message
};

throw new ShopifyHttpException(code, customErrors, message, rawResponse, requestId);
throw new ShopifyHttpException(requestInfo, code, customErrors, message, rawResponse, requestId);
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions ShopifySharp/Utilities/ShopifyOauthUtility.cs
Expand Up @@ -242,7 +242,7 @@ string clientSecret
using var response = await client.SendAsync(request);
var rawDataString = await response.Content.ReadAsStringAsync();

ShopifyService.CheckResponseExceptions(response, rawDataString);
ShopifyService.CheckResponseExceptions(request.ToString(), response, rawDataString);

var json = JToken.Parse(rawDataString);
return new AuthorizationResult(json.Value<string>("access_token"), json.Value<string>("scope")?.Split(','));

Check warning on line 248 in ShopifySharp/Utilities/ShopifyOauthUtility.cs

View workflow job for this annotation

GitHub Actions / Build

Possible null reference argument for parameter 'accessToken' in 'AuthorizationResult.AuthorizationResult(string accessToken, string[]? grantedScopes)'.

Check warning on line 248 in ShopifySharp/Utilities/ShopifyOauthUtility.cs

View workflow job for this annotation

GitHub Actions / Unit tests

Possible null reference argument for parameter 'accessToken' in 'AuthorizationResult.AuthorizationResult(string accessToken, string[]? grantedScopes)'.
Expand Down Expand Up @@ -285,7 +285,7 @@ string existingStoreAccessToken
using var response = await client.SendAsync(request);
var rawDataString = await response.Content.ReadAsStringAsync();

ShopifyService.CheckResponseExceptions(response, rawDataString);
ShopifyService.CheckResponseExceptions(request.ToString(), response, rawDataString);

var json = JToken.Parse(rawDataString);
// TODO: throw a ShopifyJsonParseException if value is null. Exception should have a RawBody property.
Expand Down

0 comments on commit d8568fb

Please sign in to comment.