From 7f24b6bb5ac7c83bb642464deb02dd58286d62e0 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Mon, 4 Mar 2024 17:30:28 +0100 Subject: [PATCH] Extract error path from context. --- .../src/Execution/Processing/PathHelper.cs | 22 ++++++++++-- .../src/Core/Execution/ExecutionState.cs | 4 +++ .../src/Core/Execution/ExecutorUtils.cs | 10 ++++-- .../src/Core/Execution/Nodes/Resolve.cs | 13 +++---- .../Core/Execution/Nodes/ResolveByKeyBatch.cs | 36 +++++++++++++++---- ...ErrorTests.NestedResolveSubgraphError.snap | 5 +++ ...ts.NestedResolveWithListSubgraphError.snap | 12 +++++++ .../ErrorTests.ResolveByKeySubgraphError.snap | 12 +++++++ ...rorTests.TopLevelResolveSubgraphError.snap | 3 ++ 9 files changed, 100 insertions(+), 17 deletions(-) diff --git a/src/HotChocolate/Core/src/Execution/Processing/PathHelper.cs b/src/HotChocolate/Core/src/Execution/Processing/PathHelper.cs index ce87f8658ab..fa7b051d140 100644 --- a/src/HotChocolate/Core/src/Execution/Processing/PathHelper.cs +++ b/src/HotChocolate/Core/src/Execution/Processing/PathHelper.cs @@ -1,12 +1,13 @@ using System; using System.Buffers; +using System.Text.Json; namespace HotChocolate.Execution.Processing; internal static class PathHelper { - private const int _initialPathLength = 64; - + private const int _initialPathLength = 64; + public static Path CreatePathFromContext(ObjectResult parent) => CreatePath(parent); @@ -18,6 +19,23 @@ public static Path CreatePathFromContext(ISelection selection, ResultData parent _ => throw new NotSupportedException($"{parent.GetType().FullName} is not a supported parent type."), }; + public static Path CombinePath(ObjectResult parent, JsonElement errorSubPath, int skipSubElements) + { + var path = parent.Parent is null ? Path.Root : CreatePath(parent); + + for (var i = skipSubElements; i < errorSubPath.GetArrayLength(); i++) + { + path = errorSubPath[i] switch + { + { ValueKind: JsonValueKind.String, } nameElement => path.Append(nameElement.GetString()!), + { ValueKind: JsonValueKind.Number, } indexElement => path.Append(indexElement.GetInt32()), + _ => throw new InvalidOperationException("The error path contains an unsupported element.") + }; + } + + return path; + } + private static Path CreatePath(ResultData parent, object segmentValue) { var segments = ArrayPool.Shared.Rent(_initialPathLength); diff --git a/src/HotChocolate/Fusion/src/Core/Execution/ExecutionState.cs b/src/HotChocolate/Fusion/src/Core/Execution/ExecutionState.cs index d436e2d385a..1e1c4ac677e 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/ExecutionState.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/ExecutionState.cs @@ -41,11 +41,15 @@ public ExecutionState( /// /// Gets the selection set data that was collected during execution. + /// The selection set data represents the data that we have collected + /// from the subgraphs for the . /// public SelectionData[] SelectionSetData { get; } /// /// Gets the completed selection set result. + /// The selection set result represents the data for the + /// that we deliver to the user. /// public ObjectResult SelectionSetResult { get; } diff --git a/src/HotChocolate/Fusion/src/Core/Execution/ExecutorUtils.cs b/src/HotChocolate/Fusion/src/Core/Execution/ExecutorUtils.cs index 48b97e686ab..29104519e85 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/ExecutorUtils.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/ExecutorUtils.cs @@ -483,6 +483,8 @@ public static void TryInitializeExecutionState(QueryPlan queryPlan, ExecutionSta public static void ExtractErrors( ResultBuilder resultBuilder, JsonElement errors, + ObjectResult selectionSetResult, + int pathDepth, bool addDebugInfo) { if (errors.ValueKind is not JsonValueKind.Array) @@ -492,13 +494,15 @@ public static void ExtractErrors( foreach (var error in errors.EnumerateArray()) { - ExtractError(resultBuilder, error, addDebugInfo); + ExtractError(resultBuilder, error, selectionSetResult, pathDepth, addDebugInfo); } } private static void ExtractError( ResultBuilder resultBuilder, JsonElement error, + ObjectResult selectionSetResult, + int pathDepth, bool addDebugInfo) { if (error.ValueKind is not JsonValueKind.Object) @@ -530,7 +534,9 @@ private static void ExtractError( if (error.TryGetProperty("path", out var remotePath) && remotePath.ValueKind is JsonValueKind.Array) { - // TODO : rewrite remote path if possible! + var path = PathHelper.CombinePath(selectionSetResult, remotePath, pathDepth); + errorBuilder.SetPath(path); + if (addDebugInfo) { errorBuilder.SetExtension("remotePath", remotePath); diff --git a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs index 8a83a40d64f..57e7c06f2f2 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs @@ -17,7 +17,6 @@ namespace HotChocolate.Fusion.Execution.Nodes; /// internal sealed class Resolve(int id, Config config) : ResolverNodeBase(id, config) { - /// /// Gets the kind of this node. /// @@ -69,7 +68,7 @@ protected override async Task OnExecuteAsync( ProcessResponses(context, executionState, requests, responses, SubgraphName); } } - catch(Exception ex) + catch (Exception ex) { var error = context.OperationContext.ErrorHandler.CreateUnexpectedError(ex); context.Result.AddError(error.Build()); @@ -129,20 +128,22 @@ private void ProcessResponses( ref var request = ref MemoryMarshal.GetArrayDataReference(requests); ref var response = ref MemoryMarshal.GetArrayDataReference(responses); ref var end = ref Unsafe.Add(ref state, executionStates.Count); + var pathLength = Path.Length; while (Unsafe.IsAddressLessThan(ref state, ref end)) { var data = UnwrapResult(response); var selectionSet = state.SelectionSet; - var selectionResults = state.SelectionSetData; + var selectionSetData = state.SelectionSetData; + var selectionSetResult = state.SelectionSetResult; var exportKeys = state.Requires; var variableValues = state.VariableValues; - ExtractErrors(context.Result, response.Errors, context.ShowDebugInfo); + ExtractErrors(context.Result, response.Errors, selectionSetResult, pathLength, context.ShowDebugInfo); // we extract the selection data from the request and add it to the // workItem results. - ExtractSelectionResults(SelectionSet, subgraphName, data, selectionResults); + ExtractSelectionResults(SelectionSet, subgraphName, data, selectionSetData); // next we need to extract any variables that we need for followup requests. ExtractVariables(data, context.QueryPlan, selectionSet, exportKeys, variableValues); @@ -152,4 +153,4 @@ private void ProcessResponses( response = ref Unsafe.Add(ref response, 1)!; } } -} +} \ No newline at end of file diff --git a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs index 2e759182eb9..4b4429efc35 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs @@ -2,6 +2,7 @@ using System.Runtime.InteropServices; using System.Text; using System.Text.Json; +using HotChocolate.Execution.Processing; using HotChocolate.Fusion.Clients; using HotChocolate.Language; using static HotChocolate.Fusion.Execution.ExecutorUtils; @@ -87,7 +88,7 @@ protected override async Task OnExecuteAsync( ProcessResult(context, response, batchExecutionState); } } - catch(Exception ex) + catch (Exception ex) { var error = context.OperationContext.ErrorHandler.CreateUnexpectedError(ex); context.Result.AddError(error.Build()); @@ -134,17 +135,28 @@ private void ProcessResult( GraphQLResponse response, BatchExecutionState[] batchExecutionState) { - ExtractErrors(context.Result, response.Errors, context.ShowDebugInfo); var result = UnwrapResult(response, Requires); - ref var batchState = ref MemoryMarshal.GetArrayDataReference(batchExecutionState); ref var end = ref Unsafe.Add(ref batchState, batchExecutionState.Length); + var pathLength = Path.Length; + var first = true; while (Unsafe.IsAddressLessThan(ref batchState, ref end)) { if (result.TryGetValue(batchState.Key, out var data)) { - ExtractSelectionResults(SelectionSet, SubgraphName, data, batchState.SelectionResults); + if (first) + { + ExtractErrors( + context.Result, + response.Errors, + batchState.SelectionSetResult, + pathLength, + context.ShowDebugInfo); + first = false; + } + + ExtractSelectionResults(SelectionSet, SubgraphName, data, batchState.SelectionSetData); ExtractVariables(data, context.QueryPlan, SelectionSet, batchState.Requires, batchState.VariableValues); } @@ -237,6 +249,7 @@ private Dictionary UnwrapResult( if (exportKeys.Count == 1) { var key = exportKeys[0]; + foreach (var element in data.EnumerateArray()) { if (element.TryGetProperty(key, out var keyValue)) @@ -391,8 +404,17 @@ private readonly struct BatchExecutionState(string batchKey, ExecutionState exec public IReadOnlyList Requires { get; } = executionState.Requires; /// - /// Gets the selection set data. + /// Gets the completed selection set result. + /// The selection set result represents the data for the + /// that we deliver to the user. + /// + public ObjectResult SelectionSetResult { get; } = executionState.SelectionSetResult; + + /// + /// Gets the selection set data that was collected during execution. + /// The selection set data represents the data that we have collected + /// from the subgraphs for the . /// - public SelectionData[] SelectionResults { get; } = executionState.SelectionSetData; + public SelectionData[] SelectionSetData { get; } = executionState.SelectionSetData; } -} +} \ No newline at end of file diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveSubgraphError.snap b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveSubgraphError.snap index 68a71ad668c..02feb9b2364 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveSubgraphError.snap +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveSubgraphError.snap @@ -80,6 +80,11 @@ Result "column": 101 } ], + "path": [ + "reviewById", + "author", + "errorField" + ], "extensions": { "remotePath": [ "userById", diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveWithListSubgraphError.snap b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveWithListSubgraphError.snap index 06e6f94e41c..e8c8ffa7e1b 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveWithListSubgraphError.snap +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.NestedResolveWithListSubgraphError.snap @@ -80,6 +80,12 @@ Result "column": 84 } ], + "path": [ + "userById", + "reviews", + 1, + "errorField" + ], "extensions": { "remotePath": [ "userById", @@ -96,6 +102,12 @@ Result "column": 84 } ], + "path": [ + "userById", + "reviews", + 0, + "errorField" + ], "extensions": { "remotePath": [ "userById", diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.ResolveByKeySubgraphError.snap b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.ResolveByKeySubgraphError.snap index 1fe6af72c9e..fee0fbbcc51 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.ResolveByKeySubgraphError.snap +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.ResolveByKeySubgraphError.snap @@ -80,6 +80,12 @@ Result "column": 94 } ], + "path": [ + "reviews", + 0, + "author", + "errorField" + ], "extensions": { "remotePath": [ "usersById",1, @@ -95,6 +101,12 @@ Result "column": 94 } ], + "path": [ + "reviews", + 0, + "author", + "errorField" + ], "extensions": { "remotePath": [ "usersById",0, diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.TopLevelResolveSubgraphError.snap b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.TopLevelResolveSubgraphError.snap index 03bb1d186cb..545d8aa611b 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.TopLevelResolveSubgraphError.snap +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ErrorTests.TopLevelResolveSubgraphError.snap @@ -51,6 +51,9 @@ Result "column": 68 } ], + "path": [ + "errorField" + ], "extensions": { "remotePath": [ "errorField"