Skip to content

Commit c072d3a

Browse files
tobias-tenglertobias-tengler
committed
WIP: Create transport errors
1 parent 07aa945 commit c072d3a

File tree

9 files changed

+308
-65
lines changed

9 files changed

+308
-65
lines changed

‎src/HotChocolate/AspNetCore/src/Transport.Http/GraphQLHttpResponse.cs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ namespace HotChocolate.Transport.Http;
99
/// </summary>
1010
public sealed class GraphQLHttpResponse : IDisposable
1111
{
12-
private static readonly OperationResult s_transportError = CreateTransportError();
13-
1412
private readonly HttpResponseMessage _message;
1513

1614
/// <summary>
@@ -182,21 +180,16 @@ public IAsyncEnumerable<OperationResult> ReadAsResultStreamAsync()
182180
ct => ReadAsResultInternalAsync(contentType.CharSet, ct));
183181
}
184182

185-
return SingleResult(new ValueTask<OperationResult>(s_transportError));
183+
_message.EnsureSuccessStatusCode();
184+
185+
throw new InvalidOperationException("Received a successful response with an unexpected content type.");
186186
}
187187

188188
private static async IAsyncEnumerable<OperationResult> SingleResult(ValueTask<OperationResult> result)
189189
{
190190
yield return await result.ConfigureAwait(false);
191191
}
192192

193-
private static OperationResult CreateTransportError()
194-
=> new OperationResult(
195-
errors: JsonDocument.Parse(
196-
"""
197-
[{"message": "Internal Execution Error"}]
198-
""").RootElement);
199-
200193
/// <summary>
201194
/// Disposes the underlying <see cref="HttpResponseMessage"/>.
202195
/// </summary>

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace HotChocolate.Fusion.Execution.Clients;
2+
3+
public sealed class SourceSchemaError(IError error, Path path)
4+
{
5+
public IError Error { get; } = error;
6+
7+
public Path Path { get; } = path;
8+
}

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

Lines changed: 109 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -56,57 +56,12 @@ public OperationExecutionNode(
5656

5757
_variables = variables.ToArray();
5858

59-
var selectionSet = GetSelectionSetNodeFromPath(operation, source);
60-
61-
_providingSelectionSet = selectionSet ?? throw new InvalidOperationException("Could not determine source selection set");
59+
// TODO: This might include requirements that we wouldn't want to create a front-end facing error for
60+
_responseNames = GetResponseNamesFromPath(operation, source);
6261
}
6362

64-
private readonly SelectionSetNode _providingSelectionSet;
65-
66-
// TODO: Move
67-
private static SelectionSetNode? GetSelectionSetNodeFromPath(OperationDefinitionNode operationDefinition, SelectionPath path)
68-
{
69-
var current = operationDefinition.SelectionSet;
70-
71-
if (path.IsRoot)
72-
{
73-
return current;
74-
}
75-
76-
for (var i = path.Segments.Length - 1; i >= 0; i--)
77-
{
78-
var segment = path.Segments[i];
79-
80-
if (segment.Kind == SelectionPathSegmentKind.InlineFragment)
81-
{
82-
// TODO: Do we need to handle the case without a type condition?
83-
var selection = current.Selections
84-
.OfType<InlineFragmentNode>()
85-
.FirstOrDefault(s => s.TypeCondition?.Name.Value == segment.Name);
86-
87-
if (selection is null)
88-
{
89-
return null;
90-
}
91-
92-
current = selection.SelectionSet;
93-
}
94-
else if (segment.Kind is SelectionPathSegmentKind.Field)
95-
{
96-
var selection = current.Selections
97-
.OfType<FieldNode>()
98-
.FirstOrDefault(s => s.Alias?.Value == segment.Name || s.Name.Value == segment.Name);
99-
100-
if (selection?.SelectionSet is null)
101-
{
102-
return null;
103-
}
104-
current = selection.SelectionSet;
105-
}
106-
}
107-
108-
return current;
109-
}
63+
// TODO: Should this be a span
64+
private readonly ImmutableArray<string> _responseNames;
11065

11166
public override int Id { get; }
11267

@@ -198,7 +153,7 @@ private async Task<ExecutionNodeResult> ExecuteInternalAsync(
198153
}
199154
catch (Exception exception)
200155
{
201-
context.AddException(exception);
156+
AddErrors(context, exception, variables, _responseNames);
202157

203158
return new ExecutionNodeResult(
204159
Id,
@@ -231,7 +186,7 @@ private async Task<ExecutionNodeResult> ExecuteInternalAsync(
231186
result?.Dispose();
232187
}
233188

234-
context.AddException(exception);
189+
AddErrors(context, exception, variables, _responseNames);
235190

236191
return new ExecutionNodeResult(
237192
Id,
@@ -357,6 +312,109 @@ protected internal override void Seal()
357312
_isSealed = true;
358313
}
359314

315+
private static ImmutableArray<string> GetResponseNamesFromPath(
316+
OperationDefinitionNode operationDefinition,
317+
SelectionPath path)
318+
{
319+
var selectionSet = GetSelectionSetNodeFromPath(operationDefinition, path);
320+
321+
if (selectionSet is null)
322+
{
323+
return [];
324+
}
325+
326+
var responseNames = new List<string>();
327+
328+
foreach (var selection in selectionSet.Selections)
329+
{
330+
// TODO: We need to handle InlineFragmentNodes here
331+
if (selection is FieldNode fieldNode)
332+
{
333+
responseNames.Add(fieldNode.Alias?.Value ?? fieldNode.Name.Value);
334+
}
335+
}
336+
337+
return [..responseNames];
338+
}
339+
340+
private static SelectionSetNode? GetSelectionSetNodeFromPath(OperationDefinitionNode operationDefinition, SelectionPath path)
341+
{
342+
var current = operationDefinition.SelectionSet;
343+
344+
if (path.IsRoot)
345+
{
346+
return current;
347+
}
348+
349+
for (var i = path.Segments.Length - 1; i >= 0; i--)
350+
{
351+
var segment = path.Segments[i];
352+
353+
if (segment.Kind == SelectionPathSegmentKind.InlineFragment)
354+
{
355+
// TODO: Do we need to handle the case without a type condition?
356+
var selection = current.Selections
357+
.OfType<InlineFragmentNode>()
358+
.FirstOrDefault(s => s.TypeCondition?.Name.Value == segment.Name);
359+
360+
if (selection is null)
361+
{
362+
return null;
363+
}
364+
365+
current = selection.SelectionSet;
366+
}
367+
else if (segment.Kind is SelectionPathSegmentKind.Field)
368+
{
369+
var selection = current.Selections
370+
.OfType<FieldNode>()
371+
.FirstOrDefault(s => s.Alias?.Value == segment.Name || s.Name.Value == segment.Name);
372+
373+
if (selection?.SelectionSet is null)
374+
{
375+
return null;
376+
}
377+
current = selection.SelectionSet;
378+
}
379+
}
380+
381+
return current;
382+
}
383+
384+
private static void AddErrors(
385+
OperationPlanContext context,
386+
Exception exception,
387+
ImmutableArray<VariableValues> variables,
388+
ImmutableArray<string> responseNames)
389+
{
390+
var bufferLength = Math.Max(variables.Length, 1);
391+
var buffer = ArrayPool<SourceSchemaError>.Shared.Rent(bufferLength);
392+
393+
try
394+
{
395+
var error = ErrorBuilder.FromException(exception).Build();
396+
397+
if (variables.Length == 0)
398+
{
399+
buffer[0] = new SourceSchemaError(error, Path.Root);
400+
}
401+
else
402+
{
403+
for (var i = 0; i < variables.Length; i++)
404+
{
405+
buffer[i] = new SourceSchemaError(error, variables[i].Path);
406+
}
407+
}
408+
409+
context.AddErrors(buffer.AsSpan(0, bufferLength), responseNames);
410+
}
411+
finally
412+
{
413+
buffer.AsSpan(0, bufferLength).Clear();
414+
ArrayPool<SourceSchemaError>.Shared.Return(buffer);
415+
}
416+
}
417+
360418
private sealed class SubscriptionEnumerable : IAsyncEnumerable<EventMessageResult>
361419
{
362420
private readonly OperationPlanContext _context;

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,8 @@ internal void AddPartialResults(SelectionPath sourcePath, ReadOnlySpan<SourceSch
105105
internal void AddPartialResults(ObjectResult result, ReadOnlySpan<Selection> selections)
106106
=> _resultStore.AddPartialResults(result, selections);
107107

108-
internal void AddException(Exception? exception = null)
109-
{
110-
// TODO: Implement and change name
111-
}
108+
internal void AddErrors(ReadOnlySpan<SourceSchemaError> errors, ImmutableArray<string> responseNames)
109+
=> _resultStore.AddErrors(errors, responseNames);
112110

113111
internal PooledArrayWriter CreateRentedBuffer()
114112
=> _resultStore.CreateRentedBuffer();

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

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,54 @@ public bool AddPartialResults(
119119
}
120120
}
121121

122+
public void AddErrors(ReadOnlySpan<SourceSchemaError> errors, ImmutableArray<string> responseNames)
123+
{
124+
_lock.EnterWriteLock();
125+
126+
try
127+
{
128+
ref var error = ref MemoryMarshal.GetReference(errors);
129+
ref var end = ref Unsafe.Add(ref error, errors.Length);
130+
131+
while (Unsafe.IsAddressLessThan(ref error, ref end))
132+
{
133+
// if (error.Path.IsRoot)
134+
// {
135+
// var selectionSet = _operation.RootSelectionSet;
136+
// if (!_valueCompletion.BuildResult(selectionSet, data, errorTrie, _root))
137+
// {
138+
// return false;
139+
// }
140+
// }
141+
// else
142+
// {
143+
// var startResult = GetStartObjectResult(error.Path);
144+
// if (!_valueCompletion.BuildResult(startResult.SelectionSet, data, errorTrie, startResult))
145+
// {
146+
// return false;
147+
// }
148+
// }
149+
150+
foreach (var responseName in responseNames)
151+
{
152+
var errorWithPath = ErrorBuilder.FromError(error.Error)
153+
.SetPath(error.Path.Append(responseName))
154+
// TODO: Locations
155+
.Build();
156+
157+
// TODO: Null propagation
158+
_errors.Add(errorWithPath);
159+
}
160+
161+
error = ref Unsafe.Add(ref error, 1)!;
162+
}
163+
}
164+
finally
165+
{
166+
_lock.ExitWriteLock();
167+
}
168+
}
169+
122170
public void AddPartialResults(ObjectResult result, ReadOnlySpan<Selection> selections)
123171
{
124172
ObjectDisposedException.ThrowIf(_disposed, this);

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,45 @@ public async Task Subgraph_Request_Fails_For_Lookup()
434434
response.MatchSnapshot();
435435
}
436436

437+
[Fact]
438+
public async Task Subgraph_Request_Fails_For_Lookup_On_List()
439+
{
440+
// arrange
441+
using var server1 = CreateSourceSchema(
442+
"A",
443+
b => b.AddQueryType<SourceSchema1.Query>());
444+
445+
using var server2 = CreateSourceSchema(
446+
"B",
447+
b => b.AddQueryType<SourceSchema3.Query>(),
448+
isOffline: true);
449+
450+
// act
451+
using var gateway = await CreateCompositeSchemaAsync(
452+
[
453+
("A", server1),
454+
("B", server2),
455+
]);
456+
457+
// assert
458+
using var client = GraphQLHttpClient.Create(gateway.CreateClient());
459+
460+
using var result = await client.PostAsync(
461+
"""
462+
{
463+
topProducts {
464+
price
465+
name
466+
}
467+
}
468+
""",
469+
new Uri("http://localhost:5000/graphql"));
470+
471+
// act
472+
using var response = await result.ReadAsResultAsync();
473+
response.MatchSnapshot();
474+
}
475+
437476
#endregion
438477

439478
public static class SourceSchema1
@@ -442,6 +481,9 @@ public class Query
442481
{
443482
public Product GetTopProduct() => new(1, 13.99);
444483

484+
public List<Product> GetTopProducts()
485+
=> [new(1, 13.99), new(2, 13.99), new(3, 13.99)];
486+
445487
[Lookup]
446488
[Internal]
447489
public Product? GetProductById(int id) => new(id, 13.99);

‎src/HotChocolate/Fusion-vnext/test/Fusion.AspNetCore.Tests/__snapshots__/SubgraphErrorTests.Subgraph_Request_Fails_For_Lookup.snap

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@
55
"name": null
66
}
77
},
8+
"errors": [
9+
{
10+
"message": "Unexpected Execution Error",
11+
"path": [
12+
"topProduct",
13+
"name"
14+
]
15+
}
16+
],
817
"extensions": {
918
"fusion": {
1019
"operationPlan": {

0 commit comments

Comments
 (0)