Prevent silent data: null responses when Fusion diagnostics throw

AGENTS.md

@@ -95,3 +95,28 @@ After adding or editing any `.graphql` document under `src/HotChocolate/Fusion/s
 ```
 
 Never hand-write or hand-edit a `.sha256` sidecar. The `update` command is the only source of sidecar content, and `verify` must pass before handoff.
+
+## Components
+
+### All components
+
+#### Exceptions
+
+Create exceptions through the `ThrowHelper` class of the project you are editing instead of inlining `throw new ...`. Each project keeps its own, which centralizes exception messages.
+Example: `src/HotChocolate/Fusion/src/Fusion.Execution/Execution/ThrowHelper.cs`
+
+#### GraphQL errors
+
+Create GraphQL errors through the `ErrorHelper` class of the project you are editing instead of inlining `ErrorBuilder` calls. Each project keeps its own, which centralizes error messages.
+Example: `src/HotChocolate/AspNetCore/src/AspNetCore.Pipeline/Utilities/ErrorHelper.cs`
+
+### src/Fusion
+
+#### Execution nodes
+
+When you add a value to `ExecutionNodeType`, map it in two places:
+
+- `ExecutePlanNodeSpan.KindValues` in `src/HotChocolate/Fusion/src/Fusion.Diagnostics/Spans/ExecutePlanNodeSpan.cs`
+- `GraphQL.Operation.Step.KindValues` in `src/HotChocolate/Diagnostics/src/Diagnostics.Core/SemanticConventions.cs`, if the kind needs a new constant. Tag values are snake_case.
+
+`KindValues` supplies the `graphql.operation.step.kind` tag on the step span. An unmapped type does not fail execution — `ExecutePlanNodeSpan.Start` falls back to an untagged span — so the node silently loses its kind in traces. The guard test `StepSpan_Should_MapEveryExecutionNodeTypeToAKindValue` in `src/HotChocolate/Fusion/test/Fusion.Diagnostics.Tests/FusionActivityExecutionDiagnosticListenerTests.cs` fails until the mapping exists.

src/HotChocolate/Diagnostics/src/Diagnostics.Core/SemanticConventions.cs

@@ -60,6 +60,7 @@ public static class KindValues
                 {
                     public const string Operation = "operation";
                     public const string OperationBatch = "operation_batch";
+                    public const string EventStream = "event_stream";
                     public const string Introspection = "introspection";
                     public const string Node = "node";
                 }

src/HotChocolate/Fusion/src/Fusion.Diagnostics/Spans/ExecutePlanNodeSpan.cs

@@ -15,11 +15,12 @@ internal sealed class ExecutePlanNodeSpan(
     string? schemaName,
     FusionActivityEnricher enricher) : SpanBase(activity)
 {
-    private static FrozenDictionary<ExecutionNodeType, string> KindValues { get; } =
+    internal static FrozenDictionary<ExecutionNodeType, string> KindValues { get; } =
         new Dictionary<ExecutionNodeType, string>
         {
             [ExecutionNodeType.Operation] = GraphQL.Operation.Step.KindValues.Operation,
             [ExecutionNodeType.OperationBatch] = GraphQL.Operation.Step.KindValues.OperationBatch,
+            [ExecutionNodeType.EventStream] = GraphQL.Operation.Step.KindValues.EventStream,
             [ExecutionNodeType.Introspection] = GraphQL.Operation.Step.KindValues.Introspection,
             [ExecutionNodeType.Node] = GraphQL.Operation.Step.KindValues.Node
         }.ToFrozenDictionary();
@@ -50,7 +51,14 @@ internal sealed class ExecutePlanNodeSpan(
         activity.EnrichDocumentInfo(context.RequestContext.OperationDocumentInfo);
 
         activity.SetTag(GraphQL.Operation.Step.Id, node.Id.ToString(CultureInfo.InvariantCulture));
-        activity.SetTag(GraphQL.Operation.Step.Kind, KindValues[node.Type]);
+
+        // An execution node type that has no mapped kind value produces an untagged
+        // span instead of failing the node's execution.
+        if (KindValues.TryGetValue(node.Type, out var kind))
+        {
+            activity.SetTag(GraphQL.Operation.Step.Kind, kind);
+        }
+
         activity.SetTag(GraphQL.Operation.Step.Plan.Id, context.OperationPlan.Id);
 
         SetSourceSchemaTags(activity, node, schemaName);

src/HotChocolate/Fusion/src/Fusion.Execution/Execution/ExecutionState.cs

@@ -20,6 +20,7 @@ internal sealed class ExecutionState
     private readonly ConcurrentQueue<ExecutionNodeResult> _completedResults = new();
     private readonly ConcurrentQueue<PendingMerge> _pendingMerges = new();
     private Dictionary<int, Exception?>? _mergeFailures;
+    private List<Exception>? _failedNodeExceptions;
     private ulong[] _failedOrSkippedBitset = [];
 
     private bool _collectTelemetry;
@@ -53,6 +54,13 @@ public void SetCancellationSource(CancellationTokenSource cts)
     /// </summary>
     public bool ProcessingCompletedEarly => _processingCompletedEarly;
 
+    /// <summary>
+    /// The exceptions of nodes that completed as failed after their exception escaped the node's
+    /// own error handling. Lets the completion surface a GraphQL error for failures that would
+    /// otherwise leave the response without any.
+    /// </summary>
+    public IReadOnlyList<Exception>? FailedNodeExceptions => _failedNodeExceptions;
+
     public void Clean()
     {
         Reset();
@@ -158,6 +166,7 @@ public void Reset()
 
         ClearPendingMerges();
         _mergeFailures?.Clear();
+        _failedNodeExceptions?.Clear();
         _activeNodes = 0;
         _processingCompletedEarly = false;
 
@@ -230,10 +239,13 @@ public ExecutionNodeResult ApplyPendingMergeFailure(ExecutionNodeResult result)
         if (_mergeFailures is not null
             && _mergeFailures.Remove(result.Id, out var exception))
         {
+            // ApplyMerge already added the errors for the failed merge to the
+            // result store, so the completion must not surface them again.
             return result with
             {
                 Status = ExecutionStatus.Failed,
-                Exception = exception
+                Exception = exception,
+                ErrorReported = true
             };
         }
 
@@ -346,6 +358,16 @@ public void CompleteNode(
 
         if (result.Status is ExecutionStatus.Skipped or ExecutionStatus.Failed)
         {
+            // A failed node whose result carries an unreported exception did not get the
+            // chance to add an error explaining the missing data to the result store. We
+            // track those exceptions so the completion can surface them.
+            if (result.Exception is { } exception
+                && !result.ErrorReported
+                && !(exception is OperationCanceledException && _cts.IsCancellationRequested))
+            {
+                (_failedNodeExceptions ??= []).Add(exception);
+            }
+
             SkipNode(plan, node);
         }
     }

src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/ApolloOperationExecutionNode.cs

@@ -1,5 +1,4 @@
 using System.Collections.Immutable;
-using System.Diagnostics;
 using System.Runtime.InteropServices;
 using HotChocolate.Execution;
 using HotChocolate.Fusion.Execution.ApolloFederation;
@@ -327,7 +326,5 @@ protected override async ValueTask<ExecutionStatus> OnExecuteAsync(
     }
 
     protected override IDisposable CreateScope(OperationPlanContext context)
-    {
-        return context.DiagnosticEvents.ExecuteApolloOperationExecutionNode(context, this, _schemaName);
-    }
+        => context.DiagnosticEvents.ExecuteApolloOperationExecutionNode(context, this, _schemaName);
 }

src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/ExecutionNodeResult.cs

@@ -12,4 +12,7 @@ internal readonly record struct ExecutionNodeResult(
     ImmutableArray<ExecutionNode> DependentsToExecute,
     ImmutableArray<IOperationPlanNode> SkippedDefinitions,
     ImmutableArray<VariableValues> VariableValueSets,
-    (Uri? Uri, string? ContentType) TransportDetails = default);
+    (Uri? Uri, string? ContentType) TransportDetails = default,
+    // true when a GraphQL error explaining Exception has already been added
+    // to the result store, so the completion must not surface it again.
+    bool ErrorReported = false);

src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/IntrospectionExecutionNode.cs

@@ -27,12 +27,40 @@ public IntrospectionExecutionNode(
         }
 
         Id = id;
-        _selections = selections;
+
+        // The planner feeds this node from two paths: an introspection-only operation
+        // passes the whole root selection set, a mixed one passes only the introspection
+        // selections. Both are narrowed here, once, to what this node can actually
+        // resolve. Everything left varies per request only through the include flags,
+        // which the result document builder applies on its own.
+        _selections = FilterResolvableSelections(selections);
+
+        // The result selection set stays over the selections as they were handed in.
+        // It backs error pocketing, where a wider set is safe and a narrower one is not.
         var selectionSetNode = new SelectionSetNode(selections.Select(t => t.SyntaxNodes[0].Node).ToArray());
         _resultSelectionSet = ResultSelectionSet.Create(selectionSetNode);
         _conditions = conditions;
     }
 
+    private static Selection[] FilterResolvableSelections(Selection[] selections)
+    {
+        var resolvable = new Selection[selections.Length];
+        var count = 0;
+
+        foreach (var selection in selections)
+        {
+            if ((selection.Resolver is null && selection.AsyncResolver is null)
+                || !selection.Field.IsIntrospectionField)
+            {
+                continue;
+            }
+
+            resolvable[count++] = selection;
+        }
+
+        return count == selections.Length ? selections : resolvable[..count];
+    }
+
     /// <inheritdoc />
     public override int Id { get; }
 
@@ -55,29 +83,27 @@ protected override async ValueTask<ExecutionStatus> OnExecuteAsync(
         CancellationToken cancellationToken = default)
     {
         var backlog = new Stack<(object? Parent, Selection Selection, SourceResultElementBuilder Result)>();
+
+        // The document is shaped from exactly the selections this node resolves. The
+        // builder drops the ones this request excludes and stamps each remaining slot
+        // with its selection, so enumerating the slots back is what keeps the document
+        // and this node's work in step.
         var resultBuilder = new SourceResultDocumentBuilder(
             context.Memory,
             context.OperationPlan.Operation,
-            context.IncludeFlags);
-        var root = resultBuilder.Root;
-        var index = 0;
+            context.IncludeFlags,
+            _selections);
 
-        foreach (var selection in _selections)
+        foreach (var (selection, property) in resultBuilder.Root.EnumerateProperties())
         {
-            if ((selection.Resolver is null && selection.AsyncResolver is null)
-                || !selection.Field.IsIntrospectionField
-                || !selection.IsIncluded(context.IncludeFlags))
-            {
-                continue;
-            }
-
-            var property = root.CreateProperty(selection, index++);
             backlog.Push((null, selection, property));
         }
 
         try
         {
             await ExecuteSelectionsAsync(context, backlog, cancellationToken).ConfigureAwait(false);
+
+            context.AddPartialResults(resultBuilder.Build(), _resultSelectionSet);
         }
         catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
         {
@@ -100,8 +126,6 @@ protected override async ValueTask<ExecutionStatus> OnExecuteAsync(
             return ExecutionStatus.Failed;
         }
 
-        context.AddPartialResults(resultBuilder.Build(), _resultSelectionSet);
-
         return ExecutionStatus.Success;
     }
 
@@ -113,7 +137,6 @@ private static async ValueTask ExecuteSelectionsAsync(
         Stack<(object? Parent, Selection Selection, SourceResultElementBuilder Result)> backlog,
         CancellationToken cancellationToken)
     {
-        var operation = context.OperationPlan.Operation;
         var fieldContext = new ReusableFieldContext(
             context.Schema,
             context.Variables,
@@ -147,44 +170,22 @@ private static async ValueTask ExecuteSelectionsAsync(
             {
                 var namedType = selection.Type.NamedType();
 
+                // The resolver shaped these objects through CreateObjectValue, which
+                // resolved the selection set and applied the include flags itself. The
+                // slots it laid out are the authority on what still has to be executed,
+                // so they are read back rather than derived a second time here.
                 if (result.ValueKind is JsonValueKind.Object
                     && (namedType.IsObjectType() || namedType.IsAbstractType()))
                 {
-                    var objectType = ResolveObjectType(
-                        namedType,
-                        fieldContext.RuntimeResults[0],
-                        context.Schema);
-                    var selectionSet = operation.GetSelectionSet(selection, objectType);
-
-                    var j = 0;
-                    for (var i = 0; i < selectionSet.Selections.Length; i++)
+                    foreach (var (childSelection, property) in result.EnumerateProperties())
                     {
-                        var childSelection = selectionSet.Selections[i];
-
-                        if (!childSelection.IsIncluded(context.IncludeFlags))
-                        {
-                            continue;
-                        }
-
-                        var property = result.CreateProperty(childSelection, j++);
                         backlog.Push((fieldContext.RuntimeResults[0], childSelection, property));
                     }
                 }
                 else if (result.ValueKind is JsonValueKind.Array
                     && selection.Type.IsListType()
                     && (namedType.IsObjectType() || namedType.IsAbstractType()))
                 {
-                    var isAbstract = namedType.IsAbstractType();
-
-                    // For non-abstract list types, resolve the selection set once.
-                    SelectionSet? staticSelectionSet = null;
-                    if (!isAbstract)
-                    {
-                        var objectType = namedType as IObjectTypeDefinition
-                            ?? selection.Type.NamedType<IObjectTypeDefinition>();
-                        staticSelectionSet = operation.GetSelectionSet(selection, objectType);
-                    }
-
                     var i = 0;
                     foreach (var element in result.EnumerateArray())
                     {
@@ -195,40 +196,13 @@ private static async ValueTask ExecuteSelectionsAsync(
                             continue;
                         }
 
-                        var selectionSet = staticSelectionSet
-                            ?? operation.GetSelectionSet(
-                                selection,
-                                ResolveObjectType(namedType, runtimeResult, context.Schema));
-
-                        var k = 0;
-                        for (var j = 0; j < selectionSet.Selections.Length; j++)
+                        foreach (var (childSelection, property) in element.EnumerateProperties())
                         {
-                            var childSelection = selectionSet.Selections[j];
-
-                            if (!childSelection.IsIncluded(context.IncludeFlags))
-                            {
-                                continue;
-                            }
-
-                            var property = element.CreateProperty(childSelection, k++);
                             backlog.Push((runtimeResult, childSelection, property));
                         }
                     }
                 }
             }
         }
     }
-
-    private static IObjectTypeDefinition ResolveObjectType(
-        IType namedType,
-        object? runtimeResult,
-        ISchemaDefinition schema)
-    {
-        if (namedType is IObjectTypeDefinition objectType)
-        {
-            return objectType;
-        }
-
-        return SchemaDefinitionTypeResolver.ResolveObjectType(schema, runtimeResult);
-    }
 }

src/HotChocolate/Fusion/src/Fusion.Execution/Execution/Nodes/OperationCompiler.cs

@@ -654,7 +654,7 @@ public void Register(object element, int id)
             {
                 var newArray = s_objectArrayPool.Rent(_elementsById.Length * 2);
                 _elementsById.AsSpan().CopyTo(newArray);
-                s_objectArrayPool.Return(_elementsById);
+                s_objectArrayPool.Return(_elementsById, clearArray: true);
                 _elementsById = newArray;
             }
 

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

@@ -768,6 +768,27 @@ internal OperationResult Complete(bool reusable = false, bool retainMemoryForDef
 
         var resultDocument = _resultStore.Result;
 
+        // A node that failed after its exception escaped the node's own error handling
+        // has not reported a GraphQL error explaining the missing data. Surface those
+        // failures here so the response never silently drops a part of the result, and
+        // never pairs missing data with an empty error list.
+        if (_executionState.FailedNodeExceptions is { Count: > 0 } failedNodeExceptions)
+        {
+            foreach (var exception in failedNodeExceptions)
+            {
+                _resultStore.AddError(
+                    _errorHandler.Handle(ErrorBuilder.FromException(exception).Build()));
+            }
+        }
+        else if (resultDocument.Data.IsInvalidated && _resultStore.Errors is not { Count: > 0 })
+        {
+            _resultStore.AddError(
+                _errorHandler.Handle(
+                    ErrorBuilder.New()
+                        .SetMessage("Unexpected Execution Error")
+                        .Build()));
+        }
+
         // Deferred responses keep result resources available for incremental
         // plans. If no delivery groups remain active, ownership is transferred
         // to the completed result.