Refactor MaybeThrowGraphqlException to throw AggregateException with …

…all GraphQL errors (#169)

* first pass

* Updated tests

* Removed path from message

src/Speckle.Sdk.Dependencies/GraphQLRetry.cs

@@ -5,15 +5,15 @@ namespace Speckle.Sdk.Dependencies;
 
 public static class GraphQLRetry
 {
-  public static async Task<T> ExecuteAsync<T, TException>(
+  public static async Task<T> ExecuteAsync<T, TInnerException>(
     Func<Task<T>> func,
     Action<Exception, TimeSpan>? onRetry = null
   )
-    where TException : Exception
+    where TInnerException : Exception
   {
     var delay = Backoff.DecorrelatedJitterBackoffV2(TimeSpan.FromSeconds(1), 5);
     var graphqlRetry = Policy
-      .Handle<TException>()
+      .HandleInner<TInnerException>()
       .WaitAndRetryAsync(
         delay,
         (ex, timeout, _) =>

src/Speckle.Sdk/Api/Exceptions.cs

@@ -1,53 +1,17 @@
-using GraphQL;
+using Speckle.Sdk.Api.GraphQL;
 
 namespace Speckle.Sdk.Api;
 
 /// <summary>
-/// Base class for GraphQL API exceptions
+/// The base class for all GraphQL errors (these are errors in the graphql response)
+/// Some specific codes are maped to subtypes <see cref="GraphQLErrorHandler"/>
+/// <seealso cref="SpeckleGraphQLForbiddenException"/>
+/// <seealso cref="SpeckleGraphQLInternalErrorException"/>
+/// <seealso cref="SpeckleGraphQLBadInputException"/>
+/// <seealso cref="SpeckleGraphQLInvalidQueryException"/>
 /// </summary>
-public class SpeckleGraphQLException<T> : SpeckleGraphQLException
-{
-  public new GraphQLResponse<T>? Response => (GraphQLResponse<T>?)base.Response;
-
-  public SpeckleGraphQLException(
-    string message,
-    GraphQLRequest request,
-    GraphQLResponse<T>? response,
-    Exception? innerException = null
-  )
-    : base(message, request, response, innerException) { }
-
-  public SpeckleGraphQLException() { }
-
-  public SpeckleGraphQLException(string? message)
-    : base(message) { }
-
-  public SpeckleGraphQLException(string? message, Exception? innerException)
-    : base(message, innerException) { }
-}
-
 public class SpeckleGraphQLException : SpeckleException
 {
-  private readonly GraphQLRequest _request;
-  public IGraphQLResponse? Response { get; }
-
-  public IEnumerable<string> ErrorMessages =>
-    Response?.Errors != null ? Response.Errors.Select(e => e.Message) : Enumerable.Empty<string>();
-
-  public IDictionary<string, object>? Extensions => Response?.Extensions;
-
-  public SpeckleGraphQLException(
-    string? message,
-    GraphQLRequest request,
-    IGraphQLResponse? response,
-    Exception? innerException = null
-  )
-    : base(message, innerException)
-  {
-    _request = request;
-    Response = response;
-  }
-
   public SpeckleGraphQLException() { }
 
   public SpeckleGraphQLException(string? message)
@@ -58,19 +22,12 @@ public SpeckleGraphQLException(string? message, Exception? innerException)
 }
 
 /// <summary>
-/// Represents a "FORBIDDEN" on "UNAUTHORIZED" GraphQL error as an exception.
+/// Represents a "FORBIDDEN" or "UNAUTHORIZED" GraphQL error as an exception.
 /// https://www.apollographql.com/docs/apollo-server/v2/data/errors/#unauthenticated
 /// https://www.apollographql.com/docs/apollo-server/v2/data/errors/#forbidden
 /// </summary>
-public class SpeckleGraphQLForbiddenException : SpeckleGraphQLException
+public sealed class SpeckleGraphQLForbiddenException : SpeckleGraphQLException
 {
-  public SpeckleGraphQLForbiddenException(
-    GraphQLRequest request,
-    IGraphQLResponse response,
-    Exception? innerException = null
-  )
-    : base("Your request was forbidden", request, response, innerException) { }
-
   public SpeckleGraphQLForbiddenException() { }
 
   public SpeckleGraphQLForbiddenException(string? message)
@@ -80,15 +37,12 @@ public SpeckleGraphQLForbiddenException(string? message, Exception? innerExcepti
     : base(message, innerException) { }
 }
 
-public class SpeckleGraphQLInternalErrorException : SpeckleGraphQLException
+/// <summary>
+/// Represents a "INTERNAL_SERVER_ERROR" GraphQL error as an exception.
+/// https://www.apollographql.com/docs/apollo-server/v2/data/errors#internal_server_error
+/// </summary>
+public sealed class SpeckleGraphQLInternalErrorException : SpeckleGraphQLException
 {
-  public SpeckleGraphQLInternalErrorException(
-    GraphQLRequest request,
-    IGraphQLResponse response,
-    Exception? innerException = null
-  )
-    : base("Your request failed on the server side", request, response, innerException) { }
-
   public SpeckleGraphQLInternalErrorException() { }
 
   public SpeckleGraphQLInternalErrorException(string? message)
@@ -98,15 +52,11 @@ public SpeckleGraphQLInternalErrorException(string? message, Exception? innerExc
     : base(message, innerException) { }
 }
 
-public class SpeckleGraphQLStreamNotFoundException : SpeckleGraphQLException
+/// <summary>
+/// Represents the custom "STREAM_NOT_FOUND" GraphQL error as an exception.
+/// </summary>
+public sealed class SpeckleGraphQLStreamNotFoundException : SpeckleGraphQLException
 {
-  public SpeckleGraphQLStreamNotFoundException(
-    GraphQLRequest request,
-    IGraphQLResponse response,
-    Exception? innerException = null
-  )
-    : base("Stream not found", request, response, innerException) { }
-
   public SpeckleGraphQLStreamNotFoundException() { }
 
   public SpeckleGraphQLStreamNotFoundException(string? message)
@@ -115,3 +65,34 @@ public SpeckleGraphQLStreamNotFoundException(string? message)
   public SpeckleGraphQLStreamNotFoundException(string? message, Exception? innerException)
     : base(message, innerException) { }
 }
+
+/// <summary>
+/// Represents a "BAD_USER_INPUT" GraphQL error as an exception.
+/// https://www.apollographql.com/docs/apollo-server/v2/data/errors#bad_user_input
+/// </summary>
+public sealed class SpeckleGraphQLBadInputException : SpeckleGraphQLException
+{
+  public SpeckleGraphQLBadInputException() { }
+
+  public SpeckleGraphQLBadInputException(string? message)
+    : base(message) { }
+
+  public SpeckleGraphQLBadInputException(string? message, Exception? innerException)
+    : base(message, innerException) { }
+}
+
+/// <summary>
+/// Represents a "GRAPHQL_PARSE_FAILED" or "GRAPHQL_VALIDATION_FAILED" GraphQL error as an exception.
+/// https://www.apollographql.com/docs/apollo-server/v2/data/errors#graphql_parse_failed
+/// https://www.apollographql.com/docs/apollo-server/v2/data/errors#graphql_validation_failed
+/// </summary>
+public sealed class SpeckleGraphQLInvalidQueryException : SpeckleGraphQLException
+{
+  public SpeckleGraphQLInvalidQueryException() { }
+
+  public SpeckleGraphQLInvalidQueryException(string? message)
+    : base(message) { }
+
+  public SpeckleGraphQLInvalidQueryException(string? message, Exception? innerException)
+    : base(message, innerException) { }
+}

src/Speckle.Sdk/Api/GraphQL/Client.cs

@@ -117,7 +117,7 @@ public async Task<T> ExecuteGraphQLRequest<T>(GraphQLRequest request, Cancellati
           GraphQLResponse<T> result = await GQLClient
             .SendMutationAsync<T>(request, cancellationToken)
             .ConfigureAwait(false);
-          MaybeThrowFromGraphQLErrors(request, result);
+          result.EnsureGraphQLSuccess();
           return result.Data;
         })
         .ConfigureAwait(false);
@@ -132,129 +132,48 @@ public async Task<T> ExecuteGraphQLRequest<T>(GraphQLRequest request, Cancellati
     }
   }
 
-  internal void MaybeThrowFromGraphQLErrors<T>(GraphQLRequest request, GraphQLResponse<T> response)
-  {
-    // The errors reflect the Apollo server v2 API, which is deprecated. It is bound to change,
-    // once we migrate to a newer version.
-    var errors = response.Errors;
-    if (errors != null && errors.Length != 0)
-    {
-      if (
-        errors.Any(e =>
-          e.Extensions != null
-          && (
-            e.Extensions.Contains(new KeyValuePair<string, object>("code", "FORBIDDEN"))
-            || e.Extensions.Contains(new KeyValuePair<string, object>("code", "UNAUTHENTICATED"))
-          )
-        )
-      )
-      {
-        throw new SpeckleGraphQLForbiddenException(request, response);
-      }
-
-      if (
-        errors.Any(e =>
-          e.Extensions != null && e.Extensions.Contains(new KeyValuePair<string, object>("code", "STREAM_NOT_FOUND"))
-        )
-      )
-      {
-        throw new SpeckleGraphQLStreamNotFoundException(request, response);
-      }
-
-      if (
-        errors.Any(e =>
-          e.Extensions != null
-          && e.Extensions.Contains(new KeyValuePair<string, object>("code", "INTERNAL_SERVER_ERROR"))
-        )
-      )
-      {
-        throw new SpeckleGraphQLInternalErrorException(request, response);
-      }
-
-      throw new SpeckleGraphQLException<T>("Request failed with errors", request, response);
-    }
-  }
-
   IDisposable ISpeckleGraphQLClient.SubscribeTo<T>(GraphQLRequest request, Action<object, T> callback) =>
     SubscribeTo(request, callback);
 
   /// <inheritdoc cref="ISpeckleGraphQLClient.SubscribeTo{T}"/>
   private IDisposable SubscribeTo<T>(GraphQLRequest request, Action<object, T> callback)
   {
-    //using (LogContext.Push(CreateEnrichers<T>(request)))
+    try
     {
-      try
-      {
-        var res = GQLClient.CreateSubscriptionStream<T>(request);
-        return res.Subscribe(
-          response =>
+      var res = GQLClient.CreateSubscriptionStream<T>(request);
+      return res.Subscribe(
+        response =>
+        {
+          try
           {
-            try
-            {
-              MaybeThrowFromGraphQLErrors(request, response);
+            response.EnsureGraphQLSuccess();
 
-              if (response.Data != null)
-              {
-                callback(this, response.Data);
-              }
-              else
-              {
-                // Serilog.Log.ForContext("graphqlResponse", response)
-                _logger.LogError(
-                  "Cannot execute graphql callback for {resultType}, the response has no data.",
-                  typeof(T).Name
-                );
-              }
-            }
-            // we catch forbidden to rethrow, making sure its not logged.
-            catch (SpeckleGraphQLForbiddenException)
-            {
-              throw;
-            }
-            // anything else related to graphql gets logged
-            catch (SpeckleGraphQLException<T> gqlException)
-            {
-              /* Speckle.Sdk.Logging..ForContext("graphqlResponse", gqlException.Response)
-                 .ForContext("graphqlExtensions", gqlException.Extensions)
-                 .ForContext("graphqlErrorMessages", gqlException.ErrorMessages.ToList())*/
-              _logger.LogWarning(
-                gqlException,
-                "Execution of the graphql request to get {resultType} failed with {graphqlExceptionType} {exceptionMessage}.",
-                typeof(T).Name,
-                gqlException.GetType().Name,
-                gqlException.Message
-              );
-              throw;
-            }
-            // we're not handling the bare Exception type here,
-            // since we have a response object on the callback, we know the Exceptions
-            // can only be thrown from the MaybeThrowFromGraphQLErrors which wraps
-            // every exception into SpeckleGraphQLException
-          },
-          ex =>
+            callback(this, response.Data);
+          }
+          catch (AggregateException ex)
           {
-            // we're logging this as an error for now, to keep track of failures
-            // so far we've swallowed these errors
-            _logger.LogError(
-              ex,
-              "Subscription for {resultType} terminated unexpectedly with {exceptionMessage}",
-              typeof(T).Name,
-              ex.Message
-            );
-            // we could be throwing like this:
-            // throw ex;
+            _logger.LogWarning(ex, "Subscription for {type} got a response with errors", typeof(T).Name);
+            throw;
           }
-        );
-      }
-      catch (Exception ex) when (!ex.IsFatal())
-      {
-        throw new SpeckleGraphQLException<T>(
-          "The graphql request failed without a graphql response",
-          request,
-          null,
-          ex
-        );
-      }
+        },
+        ex =>
+        {
+          // we're logging this as an error for now, to keep track of failures
+          // so far we've swallowed these errors
+          _logger.LogError(
+            ex,
+            "Subscription for {resultType} terminated unexpectedly with {exceptionMessage}",
+            typeof(T).Name,
+            ex.Message
+          );
+          // we could be throwing like this:
+          // throw ex;
+        }
+      );
+    }
+    catch (Exception ex) when (!ex.IsFatal() && ex is not ObjectDisposedException)
+    {
+      throw new SpeckleGraphQLException($"Subscription for {typeof(T)} failed to start", ex);
     }
   }
 

src/Speckle.Sdk/Api/GraphQL/GraphQLErrorHandler.cs

@@ -0,0 +1,49 @@
+using System.Diagnostics.Contracts;
+using GraphQL;
+
+namespace Speckle.Sdk.Api.GraphQL;
+
+internal static class GraphQLErrorHandler
+{
+  /// <exception cref="AggregateException"><inheritdoc cref="EnsureGraphQLSuccess(IReadOnlyCollection{GraphQLError}?)"/></exception>
+  public static void EnsureGraphQLSuccess(this IGraphQLResponse response) => EnsureGraphQLSuccess(response.Errors);
+
+  /// <exception cref="AggregateException">Containing a <see cref="SpeckleGraphQLException"/> (or subclass of) for each graphql Error</exception>
+  public static void EnsureGraphQLSuccess(IReadOnlyCollection<GraphQLError>? errors)
+  {
+    // The errors reflect the Apollo server v2 API, which is deprecated. It is bound to change,
+    // once we migrate to a newer version.
+    if (errors == null || errors.Count == 0)
+    {
+      return;
+    }
+
+    List<SpeckleGraphQLException> exceptions = new(errors.Count);
+    foreach (var error in errors)
+    {
+      object? code = null;
+      _ = error.Extensions?.TryGetValue("code", out code);
+
+      var message = FormatErrorMessage(error, code);
+      var ex = code switch
+      {
+        "GRAPHQL_PARSE_FAILED" or "GRAPHQL_VALIDATION_FAILED" => new SpeckleGraphQLInvalidQueryException(message),
+        "FORBIDDEN" or "UNAUTHENTICATED" => new SpeckleGraphQLForbiddenException(message),
+        "STREAM_NOT_FOUND" => new SpeckleGraphQLStreamNotFoundException(message),
+        "BAD_USER_INPUT" => new SpeckleGraphQLBadInputException(message),
+        "INTERNAL_SERVER_ERROR" => new SpeckleGraphQLInternalErrorException(message),
+        _ => new SpeckleGraphQLException(message),
+      };
+      exceptions.Add(ex);
+    }
+
+    throw new AggregateException("Request failed with GraphQL errors, see inner exceptions", exceptions);
+  }
+
+  [Pure]
+  private static string FormatErrorMessage(GraphQLError error, object? code)
+  {
+    code ??= "ERROR";
+    return $"{code}: {error.Message}";
+  }
+}