Improve ResolveByKey

ChilliCream · Sep 27, 2024 · a728bb2 · a728bb2
1 parent 5dcad4d
commit a728bb2
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 198 deletions.
diff --git a/src/HotChocolate/Fusion/src/Core/Execution/ErrorTrie.cs b/src/HotChocolate/Fusion/src/Core/Execution/ErrorTrie.cs
@@ -41,15 +41,15 @@ public static ErrorTrie FromErrors(List<IError> errors)
 
                 if (i == lastPathIndex)
                 {
-                    currentTrie.PushError(error);
+                    currentTrie.AddError(error);
                 }
             }
         }
 
         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
@@ -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<IError> 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
@@ -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<RootSelection> 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<RootSelection> 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<string, IValueNode> BuildVariables(
         BatchExecutionState[] batchExecutionState,
         Dictionary<string, ITypeNode> argumentTypes)

diff --git 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/...olveByKey_SubField_NonNull_ListItem_NonNull_Second_Service_Errors_EntryField.md b/...olveByKey_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/...lveByKey_SubField_NonNull_ListItem_Nullable_Second_Service_Errors_EntryField.md b/...lveByKey_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/...veByKey_SubField_Nullable_ListItem_Nullable_Second_Service_Errors_EntryField.md b/...veByKey_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"
       ]
     }
   ],