From 97abafd327b33128d4faef757cae293ee00d0ee6 Mon Sep 17 00:00:00 2001 From: Brandon Bernard Date: Mon, 9 Oct 2023 21:22:51 -0500 Subject: [PATCH] Fix issues in Error Handling not identifying GraphQL Server errors correctly in all cases, and therefore not propagating the full details returned by the Server. We now check all response contents (if available) if it looks like Json and then interrogate it for the existence of an errors property and parse all details if possible. Also fixed issue not processing the error path variables correctly (int vs long return values); we now handle all possible numeric types. --- .../Flurl/FlurlGraphQLException.cs | 37 +++++++++++++------ .../Flurl/FlurlGraphQLRequest.cs | 17 +++++---- .../FlurlGraphQL.Querying.csproj | 6 +-- .../NewtonsoftJsonExtensions.cs | 2 +- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/FlurlGraphQL.Querying/Flurl/FlurlGraphQLException.cs b/FlurlGraphQL.Querying/Flurl/FlurlGraphQLException.cs index 4dc25b2..9278f80 100644 --- a/FlurlGraphQL.Querying/Flurl/FlurlGraphQLException.cs +++ b/FlurlGraphQL.Querying/Flurl/FlurlGraphQLException.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Net; using System.Text; @@ -11,13 +12,17 @@ public class FlurlGraphQLException : Exception { private readonly string _errorMessage = null; - public FlurlGraphQLException(string message, string graphqlQuery, object graphQLResponsePayload = null, HttpStatusCode httpStatusCode = HttpStatusCode.BadRequest, Exception innerException = null) - : base(message, innerException) + public FlurlGraphQLException( + string message, + string graphqlQuery, + object graphQLResponsePayload = null, + HttpStatusCode httpStatusCode = HttpStatusCode.BadRequest, + Exception innerException = null + ) : base(message, innerException) { Query = graphqlQuery; HttpStatusCode = httpStatusCode; - //TODO: Verify if string is the optimal way to handle this vs Json JObject, etc. switch (graphQLResponsePayload) { case string jsonString: ErrorResponseContent = jsonString; break; @@ -27,13 +32,18 @@ public FlurlGraphQLException(string message, string graphqlQuery, object graphQL //Because this may be a result from a non-200-OK request response we attempt to inspect the response payload and possibly parse out // error details that may be available in the Error Response Content (but not already parsed and available (e.g. when GraphQL responds with 400-BadRequest). - GraphQLErrors = ParseGraphQLErrorsFromPayload(ErrorResponseContent); + GraphQLErrors = ParseGraphQLErrorsFromPayloadSafely(ErrorResponseContent); _errorMessage = BuildErrorMessage(message, GraphQLErrors, innerException); } - public FlurlGraphQLException(IReadOnlyList graphqlErrors, string graphqlQuery, string graphqlResponsePayloadContent = null, HttpStatusCode httpStatusCode = HttpStatusCode.BadRequest, Exception innerException = null) - : base(string.Empty, innerException) + public FlurlGraphQLException( + IReadOnlyList graphqlErrors, + string graphqlQuery, + string graphqlResponsePayloadContent = null, + HttpStatusCode httpStatusCode = HttpStatusCode.BadRequest, + Exception innerException = null + ) : base(string.Empty, innerException) { GraphQLErrors = graphqlErrors; Query = graphqlQuery; @@ -62,7 +72,7 @@ protected string GetMessageInternal() : _errorMessage; } - protected static IReadOnlyList ParseGraphQLErrorsFromPayload(string errorResponseContent) + protected static IReadOnlyList ParseGraphQLErrorsFromPayloadSafely(string errorResponseContent) { if (errorResponseContent.TryParseJObject(out var errorJson)) { @@ -83,8 +93,8 @@ protected static string BuildErrorMessage(string message, IReadOnlyList { - var locations = string.Join("; ", e.Locations.Select(l => $"At={l.Line},{l.Column}")); - var locationText = !string.IsNullOrEmpty(locations) ? $" [{locations}]" : null; + var concatenatedLocations = string.Join("; ", e.Locations?.Select(l => $"At={l.Line},{l.Column}") ?? Enumerable.Empty()); + var locationText = !string.IsNullOrEmpty(concatenatedLocations) ? $" [{concatenatedLocations}]" : null; var path = BuildGraphQLPath(e); var pathText = path != null ? $" [For={path}]" : null; @@ -112,9 +122,9 @@ protected static string BuildGraphQLPath(GraphQLError graphqlError) bool isFirst = true; foreach (var p in graphqlError.Path) { - if (p is int pathIndex) + if (IsNumeric(p)) { - stringBuilder.Append("[").Append(pathIndex).Append("]"); + stringBuilder.Append("[").Append(p).Append("]"); } else if (p is string pathString) { @@ -127,5 +137,10 @@ protected static string BuildGraphQLPath(GraphQLError graphqlError) return stringBuilder.ToString(); } + + protected static bool IsNumeric(object obj) => + obj is sbyte || obj is byte || obj is short || obj is ushort + || obj is int || obj is uint || obj is long || obj is ulong + || obj is float || obj is double || obj is decimal; } } diff --git a/FlurlGraphQL.Querying/Flurl/FlurlGraphQLRequest.cs b/FlurlGraphQL.Querying/Flurl/FlurlGraphQLRequest.cs index a17acf5..6ca8539 100644 --- a/FlurlGraphQL.Querying/Flurl/FlurlGraphQLRequest.cs +++ b/FlurlGraphQL.Querying/Flurl/FlurlGraphQLRequest.cs @@ -356,17 +356,18 @@ protected async Task ExecuteRequestWithExceptionHandling(F } catch (FlurlHttpException httpException) { - var httpStatusCode = (HttpStatusCode?)httpException?.StatusCode; + var responseHttpStatusCode = (HttpStatusCode?)httpException?.StatusCode; var errorContent = await httpException.GetResponseStringSafelyAsync().ConfigureAwait(false); - if (httpStatusCode is HttpStatusCode.BadRequest) - throw new FlurlGraphQLException( - $"[{(int)HttpStatusCode.BadRequest}-{HttpStatusCode.BadRequest}] The GraphQL server returned a bad request response for the query." - + " This is likely caused by a malformed, or non-parsable query; validate the query syntax, operation name, arguments, etc." - + " to ensure that the query is valid.", this.GraphQLQuery, errorContent, httpStatusCode.Value, httpException - ); - else + if (!errorContent.IsDuckTypedJson()) throw; + + var httpStatusCode = responseHttpStatusCode ?? HttpStatusCode.BadRequest; + throw new FlurlGraphQLException( + $"[{(int)httpStatusCode}-{httpStatusCode}] The GraphQL server returned an error response for the query." + + " This is likely caused by a malformed/non-parsable query, or a Schema validation issue; please validate the query syntax, operation name, and arguments" + + " to ensure that the query is valid.", this.GraphQLQuery, errorContent, httpStatusCode, httpException + ); } } diff --git a/FlurlGraphQL.Querying/FlurlGraphQL.Querying.csproj b/FlurlGraphQL.Querying/FlurlGraphQL.Querying.csproj index 26ec3e5..961611e 100644 --- a/FlurlGraphQL.Querying/FlurlGraphQL.Querying.csproj +++ b/FlurlGraphQL.Querying/FlurlGraphQL.Querying.csproj @@ -2,9 +2,9 @@ netstandard2.0;netstandard2.1 - 1.3.1 - 1.3.1 - 1.3.1 + 1.3.2 + 1.3.2 + 1.3.2 BBernard / CajunCoding CajunCoding GraphQL client extensions for Flurl.Http -- lightweight, simplified, asynchronous, fluent GraphQL client API extensions for the amazing Flurl Http library! diff --git a/FlurlGraphQL.Querying/NewtonsoftJson/NewtonsoftJsonExtensions.cs b/FlurlGraphQL.Querying/NewtonsoftJson/NewtonsoftJsonExtensions.cs index af2ba6b..db6e578 100644 --- a/FlurlGraphQL.Querying/NewtonsoftJson/NewtonsoftJsonExtensions.cs +++ b/FlurlGraphQL.Querying/NewtonsoftJson/NewtonsoftJsonExtensions.cs @@ -7,7 +7,7 @@ namespace FlurlGraphQL.Querying { internal static class NewtonsoftJsonExtensions { - public static bool TryParseJObject(this string jsonText, out JToken json) + public static bool TryParseJObject(this string jsonText, out JObject json) { try {