Skip to content

Commit

Permalink
Fix issues in Error Handling not identifying GraphQL Server errors co…
Browse files Browse the repository at this point in the history
…rrectly 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.
  • Loading branch information
cajuncoding committed Oct 10, 2023
1 parent df3cae6 commit 97abafd
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 23 deletions.
37 changes: 26 additions & 11 deletions FlurlGraphQL.Querying/Flurl/FlurlGraphQLException.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net;
using System.Text;
Expand All @@ -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;
Expand All @@ -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<GraphQLError> graphqlErrors, string graphqlQuery, string graphqlResponsePayloadContent = null, HttpStatusCode httpStatusCode = HttpStatusCode.BadRequest, Exception innerException = null)
: base(string.Empty, innerException)
public FlurlGraphQLException(
IReadOnlyList<GraphQLError> graphqlErrors,
string graphqlQuery,
string graphqlResponsePayloadContent = null,
HttpStatusCode httpStatusCode = HttpStatusCode.BadRequest,
Exception innerException = null
) : base(string.Empty, innerException)
{
GraphQLErrors = graphqlErrors;
Query = graphqlQuery;
Expand Down Expand Up @@ -62,7 +72,7 @@ protected string GetMessageInternal()
: _errorMessage;
}

protected static IReadOnlyList<GraphQLError> ParseGraphQLErrorsFromPayload(string errorResponseContent)
protected static IReadOnlyList<GraphQLError> ParseGraphQLErrorsFromPayloadSafely(string errorResponseContent)
{
if (errorResponseContent.TryParseJObject(out var errorJson))
{
Expand All @@ -83,8 +93,8 @@ protected static string BuildErrorMessage(string message, IReadOnlyList<GraphQLE

var errorMessages = graphqlErrors.Select(e =>
{
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<string>());
var locationText = !string.IsNullOrEmpty(concatenatedLocations) ? $" [{concatenatedLocations}]" : null;
var path = BuildGraphQLPath(e);
var pathText = path != null ? $" [For={path}]" : null;
Expand Down Expand Up @@ -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)
{
Expand All @@ -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;
}
}
17 changes: 9 additions & 8 deletions FlurlGraphQL.Querying/Flurl/FlurlGraphQLRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -356,17 +356,18 @@ protected async Task<FlurlGraphQLResponse> 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
);
}
}

Expand Down
6 changes: 3 additions & 3 deletions FlurlGraphQL.Querying/FlurlGraphQL.Querying.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

<PropertyGroup>
<TargetFrameworks>netstandard2.0;netstandard2.1</TargetFrameworks>
<Version>1.3.1</Version>
<AssemblyVersion>1.3.1</AssemblyVersion>
<FileVersion>1.3.1</FileVersion>
<Version>1.3.2</Version>
<AssemblyVersion>1.3.2</AssemblyVersion>
<FileVersion>1.3.2</FileVersion>
<Authors>BBernard / CajunCoding</Authors>
<Company>CajunCoding</Company>
<Description>GraphQL client extensions for Flurl.Http -- lightweight, simplified, asynchronous, fluent GraphQL client API extensions for the amazing Flurl Http library!</Description>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down

0 comments on commit 97abafd

Please sign in to comment.