From a728bb279f28863f47793b90174f7ab3f9c66863 Mon Sep 17 00:00:00 2001 From: tobias-tengler <45513122+tobias-tengler@users.noreply.github.com> Date: Fri, 27 Sep 2024 11:48:25 +0200 Subject: [PATCH] Improve ResolveByKey --- .../Fusion/src/Core/Execution/ErrorTrie.cs | 4 +- .../src/Core/Execution/ExecutionUtils.cs | 16 +++ .../Core/Execution/Nodes/ResolveByKeyBatch.cs | 118 +++++++++++++++--- .../src/Core/Execution/SelectionData.cs | 2 + ...onNull_Second_Service_Errors_EntryField.md | 36 ++---- ...llable_Second_Service_Errors_EntryField.md | 36 ++---- ...llable_Second_Service_Errors_EntryField.md | 35 +++++- ...Nullable_Second_Service_Errors_SubField.md | 2 +- ...Nullable_Second_Service_Errors_SubField.md | 115 ----------------- 9 files changed, 166 insertions(+), 198 deletions(-) delete mode 100644 src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/__EXPECTED__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md diff --git a/src/HotChocolate/Fusion/src/Core/Execution/ErrorTrie.cs b/src/HotChocolate/Fusion/src/Core/Execution/ErrorTrie.cs index 5812e33b8e0..96d6f9eca50 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/ErrorTrie.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/ErrorTrie.cs @@ -41,7 +41,7 @@ public static ErrorTrie FromErrors(List errors) if (i == lastPathIndex) { - currentTrie.PushError(error); + currentTrie.AddError(error); } } } @@ -49,7 +49,7 @@ public static ErrorTrie FromErrors(List errors) return root; } - private void PushError(IError error) + public void AddError(IError error) { if (Errors is null) { diff --git a/src/HotChocolate/Fusion/src/Core/Execution/ExecutionUtils.cs b/src/HotChocolate/Fusion/src/Core/Execution/ExecutionUtils.cs index cd302cdd4a6..5b41ad22e30 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/ExecutionUtils.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/ExecutionUtils.cs @@ -600,6 +600,22 @@ public static void TryInitializeExecutionState(QueryPlan queryPlan, ExecutionSta executionState.IsInitialized = true; } + public static IError CreateTransportError( + Exception transportException, + IErrorHandler errorHandler, + string subgraphName, + bool addDebugInfo) + { + var errorBuilder = errorHandler.CreateUnexpectedError(transportException); + + if (addDebugInfo) + { + errorBuilder.SetExtension("subgraphName", subgraphName); + } + + return errorHandler.Handle(errorBuilder.Build()); + } + public static List CreateTransportErrors( Exception transportException, IErrorHandler errorHandler, diff --git a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs index 03b764e3ffc..4ee369fd2ea 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/Nodes/ResolveByKeyBatch.cs @@ -1,9 +1,11 @@ +using System.Net; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; using System.Text.Json; using HotChocolate.Execution.Processing; using HotChocolate.Fusion.Clients; +using HotChocolate.Fusion.Planning; using HotChocolate.Language; using static HotChocolate.Fusion.Execution.ExecutionUtils; @@ -149,53 +151,131 @@ private void ProcessResult( ref var batchState = ref MemoryMarshal.GetArrayDataReference(batchExecutionState); ref var end = ref Unsafe.Add(ref batchState, batchExecutionState.Length); - var errors = response.TransportException is not null - ? CreateTransportErrors( - response.TransportException, - context.ErrorHandler, - batchState.SelectionSetResult, - RootSelections, - subgraphName, - context.ShowDebugInfo) - : ExtractErrors( + var errors = ExtractErrors( context.ErrorHandler, response.Errors, subgraphName, context.ShowDebugInfo); - ErrorTrie? unwrappedErrorTrie = null; + ErrorTrie? subgraphErrorTrie = null; + ErrorTrie? unwrappedSubgraphErrorTrie = null; if (errors is not null) { ApplyErrorsWithoutPathToResult(context.Result, errors); - var errorTrie = ErrorTrie.FromErrors(errors); - unwrappedErrorTrie = UnwrapErrors(errorTrie); + subgraphErrorTrie = ErrorTrie.FromErrors(errors); + unwrappedSubgraphErrorTrie = UnwrapErrors(subgraphErrorTrie); + } + + IError? transportError = null; + if (response.TransportException is not null) + { + transportError = CreateTransportError( + response.TransportException, + context.ErrorHandler, + subgraphName, + context.ShowDebugInfo); } while (Unsafe.IsAddressLessThan(ref batchState, ref end)) { + ErrorTrie? errorTrie = null; + if (result.TryGetValue(batchState.Key, out var listResult)) { var data = listResult.Data; - if (unwrappedErrorTrie is not null && unwrappedErrorTrie.TryGetValue(listResult.Index, out var errorTrieAtIndex)) + if (unwrappedSubgraphErrorTrie is not null + && unwrappedSubgraphErrorTrie.TryGetValue(listResult.Index, out var errorTrieAtIndex)) { - var errorTrie = ExtractErrors(SelectionSet, errorTrieAtIndex); - - if (errorTrie is not null) - { - batchState.SetErrorTrie(errorTrie); - } + errorTrie = ExtractErrors(SelectionSet, errorTrieAtIndex); } ExtractSelectionResults(SelectionSet, SubgraphName, data, batchState.SelectionSetData); ExtractVariables(data, context.QueryPlan, SelectionSet, batchState.Requires, batchState.VariableValues); } + else if (subgraphErrorTrie is not null) + { + errorTrie = GetErrorTrieForChildrenFromErrorsOnPath(subgraphErrorTrie, RootSelections, Path); + } + else if (transportError is not null) + { + errorTrie = GetErrorTrieForChildren(transportError, RootSelections); + } + + if (errorTrie is not null) + { + batchState.SetErrorTrie(errorTrie); + } batchState = ref Unsafe.Add(ref batchState, 1)!; } } + private static ErrorTrie GetErrorTrieForChildren(IError error, List rootSelections) + { + var childErrorTrie = new ErrorTrie(); + + foreach(var rootSelection in rootSelections) + { + var errorTrieForSubfield = new ErrorTrie(); + errorTrieForSubfield.AddError(error); + + childErrorTrie.Add(rootSelection.Selection.ResponseName, errorTrieForSubfield); + } + + return childErrorTrie; + } + + private static ErrorTrie? GetErrorTrieForChildrenFromErrorsOnPath( + ErrorTrie subgraphErrorTrie, + List rootSelections, + string[] path) + { + var firstErrorOnPath = GetFirstErrorOnPath(subgraphErrorTrie, path); + + if (firstErrorOnPath is null) + { + return null; + } + + var errorTrieOfParentField = new ErrorTrie(); + + foreach(var rootSelection in rootSelections) + { + var errorTrieOfSubfield = new ErrorTrie(); + errorTrieOfSubfield.AddError(firstErrorOnPath); + + errorTrieOfParentField.Add(rootSelection.Selection.ResponseName, errorTrieOfSubfield); + } + + return errorTrieOfParentField; + } + + private static IError? GetFirstErrorOnPath(ErrorTrie errorTrie, string[] path) + { + foreach (var segment in path) + { + if (errorTrie.TryGetValue(segment, out var childErrorTrie)) + { + errorTrie = childErrorTrie; + + var firstError = errorTrie.Errors?.FirstOrDefault(); + + if (firstError is not null) + { + return firstError; + } + } + else + { + return null; + } + } + + return null; + } + private static Dictionary BuildVariables( BatchExecutionState[] batchExecutionState, Dictionary argumentTypes) diff --git a/src/HotChocolate/Fusion/src/Core/Execution/SelectionData.cs b/src/HotChocolate/Fusion/src/Core/Execution/SelectionData.cs index 19665e7452b..2f69a58b6b9 100644 --- a/src/HotChocolate/Fusion/src/Core/Execution/SelectionData.cs +++ b/src/HotChocolate/Fusion/src/Core/Execution/SelectionData.cs @@ -25,6 +25,8 @@ private SelectionData(JsonResult[] multiple) public JsonResult[]? Multiple { get; } + public IError[] Errors { get; } + public QualifiedTypeName GetTypeName() { var result = Multiple is null ? Single : Multiple[0]; diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_NonNull_Second_Service_Errors_EntryField.md b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_NonNull_Second_Service_Errors_EntryField.md index 19ebc651ec7..bf13aaf248a 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_NonNull_Second_Service_Errors_EntryField.md +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_NonNull_Second_Service_Errors_EntryField.md @@ -7,19 +7,6 @@ "errors": [ { "message": "Unexpected Execution Error", - "locations": [ - { - "line": 2, - "column": 3 - } - ], - "path": [ - "products", - 0 - ] - }, - { - "message": "Cannot return null for non-nullable field.", "locations": [ { "line": 5, @@ -28,15 +15,12 @@ ], "path": [ "products", - 2, + 0, "price" - ], - "extensions": { - "code": "HC0018" - } + ] }, { - "message": "Cannot return null for non-nullable field.", + "message": "Unexpected Execution Error", "locations": [ { "line": 5, @@ -47,13 +31,10 @@ "products", 1, "price" - ], - "extensions": { - "code": "HC0018" - } + ] }, { - "message": "Cannot return null for non-nullable field.", + "message": "Unexpected Execution Error", "locations": [ { "line": 5, @@ -62,12 +43,9 @@ ], "path": [ "products", - 0, + 2, "price" - ], - "extensions": { - "code": "HC0018" - } + ] } ], "data": { diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_Nullable_Second_Service_Errors_EntryField.md b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_Nullable_Second_Service_Errors_EntryField.md index 5d507195665..37369e8312d 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_Nullable_Second_Service_Errors_EntryField.md +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_NonNull_ListItem_Nullable_Second_Service_Errors_EntryField.md @@ -7,19 +7,6 @@ "errors": [ { "message": "Unexpected Execution Error", - "locations": [ - { - "line": 2, - "column": 3 - } - ], - "path": [ - "products", - 0 - ] - }, - { - "message": "Cannot return null for non-nullable field.", "locations": [ { "line": 5, @@ -28,15 +15,12 @@ ], "path": [ "products", - 2, + 0, "price" - ], - "extensions": { - "code": "HC0018" - } + ] }, { - "message": "Cannot return null for non-nullable field.", + "message": "Unexpected Execution Error", "locations": [ { "line": 5, @@ -47,13 +31,10 @@ "products", 1, "price" - ], - "extensions": { - "code": "HC0018" - } + ] }, { - "message": "Cannot return null for non-nullable field.", + "message": "Unexpected Execution Error", "locations": [ { "line": 5, @@ -62,12 +43,9 @@ ], "path": [ "products", - 0, + 2, "price" - ], - "extensions": { - "code": "HC0018" - } + ] } ], "data": { diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_EntryField.md b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_EntryField.md index a6eca9872ee..b3484ca3bc3 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_EntryField.md +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_EntryField.md @@ -9,13 +9,42 @@ "message": "Unexpected Execution Error", "locations": [ { - "line": 2, - "column": 3 + "line": 5, + "column": 5 } ], "path": [ "products", - 0 + 0, + "price" + ] + }, + { + "message": "Unexpected Execution Error", + "locations": [ + { + "line": 5, + "column": 5 + } + ], + "path": [ + "products", + 1, + "price" + ] + }, + { + "message": "Unexpected Execution Error", + "locations": [ + { + "line": 5, + "column": 5 + } + ], + "path": [ + "products", + 2, + "price" ] } ], diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md index 8f930cc9634..26e3f600651 100644 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md +++ b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md @@ -15,7 +15,7 @@ ], "path": [ "products", - 0, + 1, "price" ] } diff --git a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/__EXPECTED__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md b/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/__EXPECTED__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md deleted file mode 100644 index 26e3f600651..00000000000 --- a/src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/__EXPECTED__/SubgraphErrorTests.ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField.md +++ /dev/null @@ -1,115 +0,0 @@ -# ResolveByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_SubField - -## Result - -```json -{ - "errors": [ - { - "message": "Unexpected Execution Error", - "locations": [ - { - "line": 5, - "column": 5 - } - ], - "path": [ - "products", - 1, - "price" - ] - } - ], - "data": { - "products": [ - { - "id": "1", - "name": "string", - "price": 123 - }, - { - "id": "2", - "name": "string", - "price": null - }, - { - "id": "3", - "name": "string", - "price": 123 - } - ] - } -} -``` - -## Request - -```graphql -{ - products { - id - name - price - } -} -``` - -## QueryPlan Hash - -```text -C991588ECF525B8EF311F2923FD2CEE9D7BE5B3A -``` - -## QueryPlan - -```json -{ - "document": "{ products { id name price } }", - "rootNode": { - "type": "Sequence", - "nodes": [ - { - "type": "Resolve", - "subgraph": "Subgraph_1", - "document": "query fetch_products_1 { products { id name __fusion_exports__1: id } }", - "selectionSetId": 0, - "provides": [ - { - "variable": "__fusion_exports__1" - } - ] - }, - { - "type": "Compose", - "selectionSetIds": [ - 0 - ] - }, - { - "type": "ResolveByKeyBatch", - "subgraph": "Subgraph_2", - "document": "query fetch_products_2($__fusion_exports__1: [ID!]!) { productsById(ids: $__fusion_exports__1) { price __fusion_exports__1: id } }", - "selectionSetId": 1, - "path": [ - "productsById" - ], - "requires": [ - { - "variable": "__fusion_exports__1" - } - ] - }, - { - "type": "Compose", - "selectionSetIds": [ - 1 - ] - } - ] - }, - "state": { - "__fusion_exports__1": "Product_id" - } -} -``` -