From e6b31a007a8f634b6013045a37251a7ad8d74975 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Wed, 13 Sep 2023 09:26:03 +0200 Subject: [PATCH] Fixed Error Behavior in Query Plan --- .../src/Core/Execution/Nodes/Resolve.cs | 12 ++++- .../Core/Execution/Nodes/ResolveByKeyBatch.cs | 12 ++++- .../FusionGraphConfigurationReader.cs | 10 +++- .../Fusion/test/Core.Tests/ErrorTests.cs | 51 ++++++++++++++++++- ...s.Internal_Server_Error_On_Root_Field.snap | 7 +++ ...Projections.Spatial.SqlServer.Tests.csproj | 2 +- .../packages.lock.json | 40 ++++++++++++--- 7 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.Internal_Server_Error_On_Root_Field.snap diff --git a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs index 88437886519..8a83a40d64f 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs @@ -40,7 +40,12 @@ protected override async Task OnExecuteAsync( RequestState state, CancellationToken cancellationToken) { - if (state.TryGetState(SelectionSet, out var executionState)) + if (!state.TryGetState(SelectionSet, out var executionState)) + { + return; + } + + try { var requests = new SubgraphGraphQLRequest[executionState.Count]; @@ -64,6 +69,11 @@ protected override async Task OnExecuteAsync( ProcessResponses(context, executionState, requests, responses, SubgraphName); } } + catch(Exception ex) + { + var error = context.OperationContext.ErrorHandler.CreateUnexpectedError(ex); + context.Result.AddError(error.Build()); + } } /// diff --git a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs index 5d8a01dd6e7..bb2b74cde66 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs @@ -57,7 +57,12 @@ protected override async Task OnExecuteAsync( RequestState state, CancellationToken cancellationToken) { - if (state.TryGetState(SelectionSet, out var executionState)) + if (!state.TryGetState(SelectionSet, out var executionState)) + { + return; + } + + try { InitializeRequests(context, executionState); @@ -82,6 +87,11 @@ protected override async Task OnExecuteAsync( ProcessResult(context, response, batchExecutionState); } } + catch(Exception ex) + { + var error = context.OperationContext.ErrorHandler.CreateUnexpectedError(ex); + context.Result.AddError(error.Build()); + } } /// diff --git a/src/HotChocolate/Fusion/src/Core/Metadata/FusionGraphConfigurationReader.cs b/src/HotChocolate/Fusion/src/Core/Metadata/FusionGraphConfigurationReader.cs index 5694e92fbfb..95e49ccb045 100644 --- a/src/HotChocolate/Fusion/src/Core/Metadata/FusionGraphConfigurationReader.cs +++ b/src/HotChocolate/Fusion/src/Core/Metadata/FusionGraphConfigurationReader.cs @@ -318,6 +318,8 @@ private void ReadEntityConfig( break; } } + + _subgraphNames.Add(subgraph); if(!_subgraphInfos.TryGetValue(subgraph, out var subgraphInfo)) { @@ -402,6 +404,8 @@ private ArgumentVariableDefinition ReadArgumentVariableDefinition( break; } } + + _subgraphNames.Add(schemaName); return new ArgumentVariableDefinition(name, schemaName, type, argumentName); } @@ -435,6 +439,8 @@ private FieldVariableDefinition ReadFieldVariableDefinition( } } + _subgraphNames.Add(schemaName); + return new FieldVariableDefinition(name, schemaName, select); } @@ -497,6 +503,8 @@ private ResolverDefinition ReadResolverDefinition( break; } } + + _subgraphNames.Add(subgraph); FragmentSpreadNode? placeholder = null; _assert.Clear(); @@ -538,7 +546,7 @@ static void OptionalArgs(HashSet assert) } } - private Dictionary? ReadResolverArgumentDefinitions( + private static Dictionary? ReadResolverArgumentDefinitions( IValueNode argumentDefinitions) { if (argumentDefinitions is NullValueNode) diff --git a/src/HotChocolate/Fusion/test/Core.Tests/ErrorTests.cs b/src/HotChocolate/Fusion/test/Core.Tests/ErrorTests.cs index 722df686abc..d73065f54d1 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/ErrorTests.cs +++ b/src/HotChocolate/Fusion/test/Core.Tests/ErrorTests.cs @@ -4,6 +4,7 @@ using HotChocolate.Fusion.Composition; using HotChocolate.Fusion.Composition.Features; using HotChocolate.Fusion.Shared; +using HotChocolate.Language; using HotChocolate.Skimmed.Serialization; using HotChocolate.Utilities; using Microsoft.Extensions.DependencyInjection; @@ -173,6 +174,54 @@ query ReformatIds { await snapshot.MatchAsync(); } + [Fact] + public async Task Internal_Server_Error_On_Root_Field() + { + // arrange + using var demoProject = await DemoProject.CreateAsync(); + + // act + var executor = await new ServiceCollection() + .AddSingleton(new ErrorFactory(demoProject.HttpClientFactory, "a")) + .AddSingleton(demoProject.WebSocketConnectionFactory) + .AddFusionGatewayServer() + .ConfigureFromDocument( + Parse( + """ + schema + @fusion(version: 1) + @transport(subgraph: "a", group: "a", location: "http:\/\/localhost\/graphql", kind: "HTTP") { + query: Query + mutation: Mutation + } + + type Query { + a: Boolean! + @resolver(subgraph: "a", select: "{ a }") + } + """)) + .CoreBuilder + .ModifyRequestOptions(o => o.IncludeExceptionDetails = true) + .BuildRequestExecutorAsync(); + + var request = Parse( + """ + query A { + a + } + """); + + // act + var result = await executor.ExecuteAsync( + QueryRequestBuilder + .New() + .SetQuery(request) + .Create()); + + // assert + result.MatchSnapshot(); + } + private class ErrorFactory : IHttpClientFactory { private readonly IHttpClientFactory _innerFactory; @@ -203,4 +252,4 @@ protected override Task SendAsync( => Task.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)); } } -} +} \ No newline at end of file diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.Internal_Server_Error_On_Root_Field.snap b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.Internal_Server_Error_On_Root_Field.snap new file mode 100644 index 00000000000..57b92a2afbd --- /dev/null +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.Internal_Server_Error_On_Root_Field.snap @@ -0,0 +1,7 @@ +{ + "errors": [ + { + "message": "Internal Execution Error" + } + ] +} \ No newline at end of file diff --git a/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/HotChocolate.Data.Projections.Spatial.SqlServer.Tests.csproj b/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/HotChocolate.Data.Projections.Spatial.SqlServer.Tests.csproj index e82e360ac4a..93339d97f6f 100644 --- a/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/HotChocolate.Data.Projections.Spatial.SqlServer.Tests.csproj +++ b/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/HotChocolate.Data.Projections.Spatial.SqlServer.Tests.csproj @@ -27,7 +27,7 @@ - + diff --git a/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/packages.lock.json b/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/packages.lock.json index c2774b1f2a2..86b3c1d57fc 100644 --- a/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/packages.lock.json +++ b/src/HotChocolate/Spatial/test/Data.Projections.SqlServer.Tests/packages.lock.json @@ -3472,6 +3472,16 @@ "System.Threading.Tasks.Extensions": "4.5.4" } }, + "Npgsql.EntityFrameworkCore.PostgreSQL.NetTopologySuite": { + "type": "Direct", + "requested": "[8.0.0-preview.7, )", + "resolved": "8.0.0-preview.7", + "contentHash": "sKFxUIzjZgRQIQEzBR99hBLRgclqgDd6VbgrAFdTLiscBwyDLJggQjC/K1XZhy9E1/ustPSWPapCfcVeFzXdjw==", + "dependencies": { + "Npgsql.EntityFrameworkCore.PostgreSQL": "8.0.0-preview.7", + "Npgsql.NetTopologySuite": "8.0.0-preview.4" + } + }, "Squadron.PostgreSql": { "type": "Direct", "requested": "[0.15.0, )", @@ -3834,10 +3844,18 @@ }, "NetTopologySuite": { "type": "Transitive", - "resolved": "2.0.0", - "contentHash": "3ajBClEI9wx2/DjjGmV52sHW1m52vLg8sdz1pJbTf5ySj1X90qehQs3v1DRwGo0F8UKj/Z2SjNhRN/6LroAkqg==", + "resolved": "2.5.0", + "contentHash": "5/+2O2ADomEdUn09mlSigACdqvAf0m/pVPGtIPEPQWnyrVykYY0NlfXLIdkMgi41kvH9kNrPqYaFBTZtHYH7Xw==", "dependencies": { - "System.Memory": "4.5.3" + "System.Memory": "4.5.4" + } + }, + "NetTopologySuite.IO.PostGis": { + "type": "Transitive", + "resolved": "2.1.0", + "contentHash": "3W8XTFz8iP6GQ5jDXK1/LANHiU+988k1kmmuPWNKcJLpmSg6CvFpbTpz+s4+LBzkAp64wHGOldSlkSuzYfrIKA==", + "dependencies": { + "NetTopologySuite": "[2.0.0, 3.0.0-A)" } }, "Newtonsoft.Json": { @@ -3847,10 +3865,20 @@ }, "Npgsql": { "type": "Transitive", - "resolved": "4.1.1", - "contentHash": "VvYSduwCVCy1erIntV3jVbBduAL8+hqobg8xf5xPmkqzGJ5j9Wx4y/eXz2+BLmFkozJEjTzxz8/X5cOyJ4P9Ig==", + "resolved": "8.0.0-preview.4", + "contentHash": "wvbiipddbRgaiYOij4fQYqnfDlxKz1XlpCNY7mWErF+CPmg+ujfs9u1ovx40TsV0Q7GKlgdyr6dugV15HvZ2Pw==", "dependencies": { - "System.Runtime.CompilerServices.Unsafe": "4.6.0" + "Microsoft.Extensions.Logging.Abstractions": "6.0.0" + } + }, + "Npgsql.NetTopologySuite": { + "type": "Transitive", + "resolved": "8.0.0-preview.4", + "contentHash": "rgVwnoFNA0bOT7EudZEZ4zRXQvETmomFermJJhYSPtY63LbjY84YNkvA7aC75a4oxF+997hbcNwNHKpp3TzoUw==", + "dependencies": { + "NetTopologySuite": "2.5.0", + "NetTopologySuite.IO.PostGIS": "2.1.0", + "Npgsql": "8.0.0-preview.4" } }, "NuGet.Frameworks": {