From c986d0d44f066af95eb4577f037319c237740de5 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 8 Nov 2023 13:47:00 +0100 Subject: [PATCH] Fix error handling. --- .../GraphQL/CachingGraphQLResolver.cs | 4 +- .../Contents/GraphQL/Types/ErrorProvider.cs | 35 -------------- .../Contents/GraphQL/Types/ErrorVisitor.cs | 7 +++ backend/src/Squidex/Config/Web/WebServices.cs | 12 ----- .../Contents/GraphQL/GraphQLMutationTests.cs | 48 +++++++++++++++++++ .../GraphQL/GraphQLSubscriptionTests.cs | 16 +++++++ 6 files changed, 72 insertions(+), 50 deletions(-) delete mode 100644 backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorProvider.cs diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs index 9758948a7..e66c0325d 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/CachingGraphQLResolver.cs @@ -55,9 +55,7 @@ public sealed class CachingGraphQLResolver : IConfigureExecution options.Schema = await GetSchemaAsync(context.App); options.HandleError(serviceProvider); - var a = await next(options); - - return a; + return await next(options); } public async Task GetSchemaAsync(IAppEntity app) diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorProvider.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorProvider.cs deleted file mode 100644 index 2756e4775..000000000 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorProvider.cs +++ /dev/null @@ -1,35 +0,0 @@ -// ========================================================================== -// Squidex Headless CMS -// ========================================================================== -// Copyright (c) Squidex UG (haftungsbeschraenkt) -// All rights reserved. Licensed under the MIT license. -// ========================================================================== - -using GraphQL; -using GraphQL.Execution; -using Squidex.Infrastructure; -using Squidex.Infrastructure.Validation; - -namespace Squidex.Domain.Apps.Entities.Contents.GraphQL.Types; - -public sealed class ErrorProvider : ErrorInfoProvider -{ - public override ErrorInfo GetInfo(ExecutionError executionError) - { - var actual = base.GetInfo(executionError); - - if (executionError.InnerException is ValidationException or DomainException) - { - if (!string.IsNullOrWhiteSpace(actual.Message)) - { - actual.Message = $"{actual.Message} - {executionError.InnerException.Message}"; - } - else - { - actual.Message = executionError.InnerException.Message; - } - } - - return actual; - } -} diff --git a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorVisitor.cs b/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorVisitor.cs index 0c2ff6efe..f5c5c3f37 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorVisitor.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Contents/GraphQL/Types/ErrorVisitor.cs @@ -33,6 +33,13 @@ internal static class ErrorVisitor log.LogError(context.OriginalException, "Failed to resolve execute query."); } + if (context.OriginalException is ValidationException or DomainException) + { + var message = context.OriginalException.Message; + + context.ErrorMessage = context.OriginalException.Message; + } + return Task.CompletedTask; }; } diff --git a/backend/src/Squidex/Config/Web/WebServices.cs b/backend/src/Squidex/Config/Web/WebServices.cs index d00d3a606..3bfc11d59 100644 --- a/backend/src/Squidex/Config/Web/WebServices.cs +++ b/backend/src/Squidex/Config/Web/WebServices.cs @@ -109,21 +109,9 @@ public static class WebServices services.AddGraphQL(builder => { builder.UseApolloTracing(); - builder.AddErrorInfoProvider(); builder.AddSchema(); builder.AddSystemTextJson(); builder.AddDataLoader(); - builder.ConfigureExecutionOptions(options => - { - var logger = options.RequestServices!.GetRequiredService>(); - - options.UnhandledExceptionDelegate = ctx => - { - logger.LogError(ctx.Exception, "GraphQL error in field {field}", ctx.FieldContext?.FieldAst?.Name); - - return Task.CompletedTask; - }; - }); }); services.AddSingletonAs() diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLMutationTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLMutationTests.cs index e96fee63c..c68ea30ae 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLMutationTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLMutationTests.cs @@ -60,6 +60,14 @@ public class GraphQLMutationTests : GraphQLTestBase path = new[] { "createMySchemaContent" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } }, @@ -200,6 +208,14 @@ public class GraphQLMutationTests : GraphQLTestBase path = new[] { "updateMySchemaContent" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } }, @@ -291,6 +307,14 @@ public class GraphQLMutationTests : GraphQLTestBase path = new[] { "upsertMySchemaContent" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } }, @@ -387,6 +411,14 @@ public class GraphQLMutationTests : GraphQLTestBase path = new[] { "patchMySchemaContent" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } }, @@ -482,6 +514,14 @@ public class GraphQLMutationTests : GraphQLTestBase path = new[] { "changeMySchemaContent" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } }, @@ -660,6 +700,14 @@ public class GraphQLMutationTests : GraphQLTestBase path = new[] { "deleteMySchemaContent" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } }, diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLSubscriptionTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLSubscriptionTests.cs index 3fec58dbe..baa7fec4e 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLSubscriptionTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Contents/GraphQL/GraphQLSubscriptionTests.cs @@ -95,6 +95,14 @@ public class GraphQLSubscriptionTests : GraphQLTestBase path = new[] { "assetChanges" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } }, @@ -188,6 +196,14 @@ public class GraphQLSubscriptionTests : GraphQLTestBase path = new[] { "contentChanges" + }, + extensions = new + { + code = "DOMAIN_FORBIDDEN", + codes = new[] + { + "DOMAIN_FORBIDDEN" + } } } },