Some cleanup

Author: tobias-tengler

src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Clients/SourceSchemaError.cs

@@ -387,32 +387,32 @@ private static void AddErrors(
         ImmutableArray<VariableValues> variables,
         ImmutableArray<string> responseNames)
     {
-        var bufferLength = Math.Max(variables.Length, 1);
-        var buffer = ArrayPool<SourceSchemaError>.Shared.Rent(bufferLength);
-
-        try
-        {
             var error = ErrorBuilder.FromException(exception).Build();
 
             if (variables.Length == 0)
             {
-                buffer[0] = new SourceSchemaError(error, Path.Root);
+                context.AddErrors(error, responseNames.AsSpan(), Path.Root);
             }
             else
             {
-                for (var i = 0; i < variables.Length; i++)
+                var bufferLength = Math.Max(variables.Length, 1);
+                var buffer = ArrayPool<Path>.Shared.Rent(bufferLength);
+
+                try
                 {
-                    buffer[i] = new SourceSchemaError(error, variables[i].Path);
+                    for (var i = 0; i < variables.Length; i++)
+                    {
+                        buffer[i] = variables[i].Path;
+                    }
+
+                    context.AddErrors(error, responseNames.AsSpan(), buffer.AsSpan(0, bufferLength));
+                }
+                finally
+                {
+                    buffer.AsSpan(0, bufferLength).Clear();
+                    ArrayPool<Path>.Shared.Return(buffer);
                 }
             }
-
-            context.AddErrors(buffer.AsSpan(0, bufferLength), responseNames.AsSpan());
-        }
-        finally
-        {
-            buffer.AsSpan(0, bufferLength).Clear();
-            ArrayPool<SourceSchemaError>.Shared.Return(buffer);
-        }
     }
 
     private sealed class SubscriptionEnumerable : IAsyncEnumerable<EventMessageResult>

src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Nodes/OperationExecutionNode.cs

@@ -105,8 +105,8 @@ internal void AddPartialResults(SelectionPath sourcePath, ReadOnlySpan<SourceSch
     internal void AddPartialResults(ObjectResult result, ReadOnlySpan<Selection> selections)
         => _resultStore.AddPartialResults(result, selections);
 
-    internal void AddErrors(ReadOnlySpan<SourceSchemaError> errors, ReadOnlySpan<string> responseNames)
-        => _resultStore.AddErrors(errors, responseNames);
+    internal void AddErrors(IError error, ReadOnlySpan<string> responseNames, params ReadOnlySpan<Path> paths)
+        => _resultStore.AddErrors(error, responseNames, paths);
 
     internal PooledArrayWriter CreateRentedBuffer()
         => _resultStore.CreateRentedBuffer();

src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/OperationPlanContext.cs

@@ -119,66 +119,64 @@ public bool AddPartialResults(
         }
     }
 
-    public bool AddErrors(ReadOnlySpan<SourceSchemaError> errors, ReadOnlySpan<string> responseNames)
+    public void AddPartialResults(ObjectResult result, ReadOnlySpan<Selection> selections)
     {
+        ObjectDisposedException.ThrowIf(_disposed, this);
+        ArgumentNullException.ThrowIfNull(result);
+
+        if (selections.Length == 0)
+        {
+            throw new ArgumentException(
+                "The selections span must contain at least one selection.",
+                nameof(selections));
+        }
+
         _lock.EnterWriteLock();
 
         try
         {
-            ref var error = ref MemoryMarshal.GetReference(errors);
-            ref var end = ref Unsafe.Add(ref error, errors.Length);
-
-            while (Unsafe.IsAddressLessThan(ref error, ref end))
+            foreach (var selection in selections)
             {
-                if (_root.IsInvalidated)
-                {
-                    return false;
-                }
-
-                var result = error.Path.IsRoot ? _root : GetStartObjectResult(error.Path);
-
-                if (result.IsInvalidated)
+                if (!selection.IsIncluded(_includeFlags))
                 {
                     continue;
                 }
 
-                _valueCompletion.BuildResult(result, responseNames, error);
-
-                error = ref Unsafe.Add(ref error, 1)!;
+                result.MoveFieldTo(selection.ResponseName, _root);
             }
         }
         finally
         {
             _lock.ExitWriteLock();
         }
-
-        return true;
     }
 
-    public void AddPartialResults(ObjectResult result, ReadOnlySpan<Selection> selections)
+    public void AddErrors(IError error, ReadOnlySpan<string> responseNames, params ReadOnlySpan<Path> paths)
     {
-        ObjectDisposedException.ThrowIf(_disposed, this);
-        ArgumentNullException.ThrowIfNull(result);
-
-        if (selections.Length == 0)
-        {
-            throw new ArgumentException(
-                "The selections span must contain at least one selection.",
-                nameof(selections));
-        }
-
         _lock.EnterWriteLock();
 
         try
         {
-            foreach (var selection in selections)
+            ref var path = ref MemoryMarshal.GetReference(paths);
+            ref var end = ref Unsafe.Add(ref path, paths.Length);
+
+            while (Unsafe.IsAddressLessThan(ref path, ref end))
             {
-                if (!selection.IsIncluded(_includeFlags))
+                if (_root.IsInvalidated)
+                {
+                    return;
+                }
+
+                var result = path.IsRoot ? _root : GetStartObjectResult(path);
+
+                if (result.IsInvalidated)
                 {
                     continue;
                 }
 
-                result.MoveFieldTo(selection.ResponseName, _root);
+                _valueCompletion.BuildResult(result, responseNames, error, path);
+
+                path = ref Unsafe.Add(ref path, 1)!;
             }
         }
         finally
@@ -203,21 +201,22 @@ private bool SaveSafe(
 
             while (Unsafe.IsAddressLessThan(ref result, ref end))
             {
-                if (result.Path.IsRoot)
+                if (_root.IsInvalidated)
                 {
-                    var selectionSet = _operation.RootSelectionSet;
-                    if (!_valueCompletion.BuildResult(selectionSet, data, errorTrie, _root))
-                    {
-                        return false;
-                    }
+                    return false;
                 }
-                else
+
+                var objectResult = result.Path.IsRoot ? _root : GetStartObjectResult(result.Path);
+                var selectionSet = result.Path.IsRoot ? _operation.RootSelectionSet : objectResult.SelectionSet;
+
+                if (objectResult.IsInvalidated)
                 {
-                    var startResult = GetStartObjectResult(result.Path);
-                    if (!_valueCompletion.BuildResult(startResult.SelectionSet, data, errorTrie, startResult))
-                    {
-                        return false;
-                    }
+                    continue;
+                }
+
+                if (!_valueCompletion.BuildResult(selectionSet, data, errorTrie, objectResult))
+                {
+                    return false;
                 }
 
                 result = ref Unsafe.Add(ref result, 1)!;

src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Results/FetchResultStore.cs

@@ -59,6 +59,11 @@ public bool BuildResult(
                     .Build();
 
                 _errors.Add(errorWithPath);
+
+                if (_errorHandling is ErrorHandling.Propagate)
+                {
+                    PropagateNullValues(objectResult);
+                }
             }
 
             return false;
@@ -94,7 +99,8 @@ public bool BuildResult(
     public void BuildResult(
         ObjectResult objectResult,
         ReadOnlySpan<string> responseNames,
-        SourceSchemaError sourceSchemaError)
+        IError error,
+        Path path)
     {
         foreach (var responseName in responseNames)
         {
@@ -105,12 +111,12 @@ public void BuildResult(
                 continue;
             }
 
-            var error = ErrorBuilder.FromError(sourceSchemaError.Error)
-                .SetPath(sourceSchemaError.Path.Append(responseName))
+            var errorWithPath = ErrorBuilder.FromError(error)
+                .SetPath(path.Append(responseName))
                 .AddLocation(fieldResult.Selection.SyntaxNodes[0].Node)
                 .Build();
 
-            _errors.Add(error);
+            _errors.Add(errorWithPath);
 
             if (_errorHandling is ErrorHandling.Propagate && fieldResult.Selection.Type.IsNonNullType())
             {

src/HotChocolate/Fusion-vnext/src/Fusion.Execution/Execution/Results/ValueCompletion.cs

@@ -299,6 +299,82 @@ public async Task Error_On_Lookup_Field()
         response.MatchSnapshot();
     }
 
+    [Fact]
+    public async Task Error_On_Lookup_Field_In_List()
+    {
+        // arrange
+        using var server1 = CreateSourceSchema(
+            "A",
+            b => b.AddQueryType<SourceSchema1.Query>());
+
+        using var server2 = CreateSourceSchema(
+            "B",
+            b => b.AddQueryType<SourceSchema7.Query>());
+
+        // act
+        using var gateway = await CreateCompositeSchemaAsync(
+        [
+            ("A", server1),
+            ("B", server2),
+        ]);
+
+        // assert
+        using var client = GraphQLHttpClient.Create(gateway.CreateClient());
+
+        using var result = await client.PostAsync(
+            """
+            {
+              topProducts {
+                price
+                name
+              }
+            }
+            """,
+            new Uri("http://localhost:5000/graphql"));
+
+        // act
+        using var response = await result.ReadAsResultAsync();
+        response.MatchSnapshot();
+    }
+
+    [Fact]
+    public async Task Error_On_Lookup_Field_In_List_Leaf_NonNull()
+    {
+        // arrange
+        using var server1 = CreateSourceSchema(
+            "A",
+            b => b.AddQueryType<SourceSchema1.Query>());
+
+        using var server2 = CreateSourceSchema(
+            "B",
+            b => b.AddQueryType<SourceSchema3.Query>());
+
+        // act
+        using var gateway = await CreateCompositeSchemaAsync(
+        [
+            ("A", server1),
+            ("B", server2),
+        ]);
+
+        // assert
+        using var client = GraphQLHttpClient.Create(gateway.CreateClient());
+
+        using var result = await client.PostAsync(
+            """
+            {
+              topProducts {
+                price
+                name
+              }
+            }
+            """,
+            new Uri("http://localhost:5000/graphql"));
+
+        // act
+        using var response = await result.ReadAsResultAsync();
+        response.MatchSnapshot();
+    }
+
     [Fact]
     public async Task No_Data_And_Error_With_Path_For_Lookup()
     {
@@ -686,4 +762,20 @@ public string GetName(IResolverContext context)
             }
         }
     }
+
+    public static class SourceSchema7
+    {
+        public class Query
+        {
+            [Lookup]
+            public Product? GetProductById(int id, IResolverContext context)
+                => throw new GraphQLException(ErrorBuilder.New().SetMessage("Could not resolve Product")
+                    .SetPath(context.Path).Build());
+        }
+
+        public record Product(int Id)
+        {
+            public string? GetName() => "Product " + Id;
+        }
+    }
 }

src/HotChocolate/Fusion-vnext/test/Fusion.AspNetCore.Tests/SubgraphErrorTests.cs

@@ -1,10 +1,4 @@
 {
-  "data": {
-    "topProduct": {
-      "price": 13.99,
-      "name": null
-    }
-  },
   "errors": [
     {
       "message": "Could not resolve Product",