Skip to content

Commit 184d60f

Browse files
michaelstaibmichaelstaib
authored
Fixed Fusion Subscription Error Handling. (#7896)
1 parent 92682d9 commit 184d60f

16 files changed

+218
-93
lines changed

‎src/HotChocolate/Fusion/src/Core/Execution/ExecutionUtils.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,27 @@ public static void ExtractErrors(
543543
}
544544
}
545545

546+
public static void ExtractErrors(
547+
DocumentNode document,
548+
OperationDefinitionNode operation,
549+
ResultBuilder resultBuilder,
550+
IErrorHandler errorHandler,
551+
JsonElement errors,
552+
Path parentPath,
553+
int pathDepth,
554+
bool addDebugInfo)
555+
{
556+
if (errors.ValueKind is not JsonValueKind.Array)
557+
{
558+
return;
559+
}
560+
561+
foreach (var error in errors.EnumerateArray())
562+
{
563+
ExtractError(document, operation, resultBuilder, errorHandler, error, parentPath, pathDepth, addDebugInfo);
564+
}
565+
}
566+
546567
private static void ExtractError(
547568
DocumentNode document,
548569
OperationDefinitionNode operation,

‎src/HotChocolate/Fusion/src/Core/Execution/Nodes/Subscribe.cs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System.Buffers;
22
using System.Runtime.CompilerServices;
3+
using System.Text.Json;
34
using HotChocolate.Execution;
45
using HotChocolate.Execution.DependencyInjection;
56
using HotChocolate.Execution.Processing;
@@ -58,7 +59,7 @@ internal async IAsyncEnumerable<IOperationResult> SubscribeAsync(
5859
var context = rootContext.Clone();
5960
var operationContext = context.OperationContext;
6061

61-
// we ensure that the query plan is only included once per stream
62+
// we ensure that the query plan is only included once per stream
6263
// in order to not inflate response sizes.
6364
if (initialResponse)
6465
{
@@ -80,6 +81,33 @@ internal async IAsyncEnumerable<IOperationResult> SubscribeAsync(
8081
}
8182
initialResponse = false;
8283

84+
if(response.Data.ValueKind is JsonValueKind.Null or JsonValueKind.Undefined
85+
&& response.Errors.ValueKind is JsonValueKind.Array)
86+
{
87+
ExtractErrors(
88+
operationContext.Operation.Document,
89+
operationContext.Operation.Definition,
90+
operationContext.Result,
91+
operationContext.ErrorHandler,
92+
response.Errors,
93+
HotChocolate.Path.Root,
94+
pathDepth: 0,
95+
addDebugInfo: context.ShowDebugInfo);
96+
97+
// Before we yield back the result we register with it the rented operation context.
98+
// When the result is disposed in the transport after usage
99+
// so will the operation context.
100+
context.Result.RegisterForCleanup(
101+
() =>
102+
{
103+
context.Dispose();
104+
return default;
105+
});
106+
107+
yield return context.Result.BuildResult();
108+
continue;
109+
}
110+
83111
// Before we can start building requests we need to rent state for the execution result.
84112
var rootSelectionSet = Unsafe.As<SelectionSet>(context.Operation.RootSelectionSet);
85113
var rootResult = context.Result.RentObject(rootSelectionSet.Selections.Count);

‎src/HotChocolate/Fusion/test/CommandLine.Tests/__snapshots__/ComposeCommandTests.Compose_Fusion_Graph_Append_Subgraph.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ type Mutation {
2828

2929
type Subscription {
3030
onNewReview: Review!
31+
onNewReviewError: Review!
3132
}
3233

3334
type AddReviewPayload {
@@ -123,6 +124,7 @@ type Mutation {
123124

124125
type Subscription {
125126
onNewReview: Review! @resolver(subgraph: "Reviews2", select: "{ onNewReview }", kind: "SUBSCRIBE")
127+
onNewReviewError: Review! @resolver(subgraph: "Reviews2", select: "{ onNewReviewError }", kind: "SUBSCRIBE")
126128
}
127129

128130
type AddReviewPayload {
@@ -218,7 +220,7 @@ scalar Date
218220
```json
219221
{
220222
"Name": "Reviews2",
221-
"Schema": "schema {\n query: Query\n mutation: Mutation\n subscription: Subscription\n}\n\n\"The node interface is implemented by entities that have a global unique identifier.\"\ninterface Node {\n id: ID!\n}\n\ntype Query {\n \"Fetches an object given its ID.\"\n node(\"ID of the object.\" id: ID!): Node\n \"Lookup nodes by a list of IDs.\"\n nodes(\"The list of node IDs.\" ids: [ID!]!): [Node]!\n reviews: [Review!]!\n reviewById(id: ID!): Review\n authorById(id: ID!): User\n productById(id: ID!): Product\n reviewOrAuthor: ReviewOrAuthor!\n viewer: Viewer!\n}\n\ntype Mutation {\n addReview(input: AddReviewInput!): AddReviewPayload!\n}\n\ntype Subscription {\n onNewReview: Review!\n}\n\ntype Product {\n reviews: [Review!]!\n id: ID!\n}\n\n\"The user who wrote the review.\"\ntype User implements Node {\n reviews: [Review!]!\n id: ID!\n name: String!\n}\n\ntype Review implements Node {\n errorField: String\n id: ID!\n author: User!\n product: Product!\n body: String!\n}\n\nunion ReviewOrAuthor = User | Review\n\ntype Viewer {\n latestReview: Review\n data: SomeData!\n}\n\ntype SomeData {\n reviewsValue: String!\n}\n\ninput AddReviewInput {\n body: String!\n authorId: Int!\n upc: Int!\n}\n\ntype AddReviewPayload {\n review: Review\n}",
223+
"Schema": "schema {\n query: Query\n mutation: Mutation\n subscription: Subscription\n}\n\n\"The node interface is implemented by entities that have a global unique identifier.\"\ninterface Node {\n id: ID!\n}\n\ntype Query {\n \"Fetches an object given its ID.\"\n node(\"ID of the object.\" id: ID!): Node\n \"Lookup nodes by a list of IDs.\"\n nodes(\"The list of node IDs.\" ids: [ID!]!): [Node]!\n reviews: [Review!]!\n reviewById(id: ID!): Review\n authorById(id: ID!): User\n productById(id: ID!): Product\n reviewOrAuthor: ReviewOrAuthor!\n viewer: Viewer!\n}\n\ntype Mutation {\n addReview(input: AddReviewInput!): AddReviewPayload!\n}\n\ntype Subscription {\n onNewReview: Review!\n onNewReviewError: Review!\n}\n\ntype Product {\n reviews: [Review!]!\n id: ID!\n}\n\n\"The user who wrote the review.\"\ntype User implements Node {\n reviews: [Review!]!\n id: ID!\n name: String!\n}\n\ntype Review implements Node {\n errorField: String\n id: ID!\n author: User!\n product: Product!\n body: String!\n}\n\nunion ReviewOrAuthor = User | Review\n\ntype Viewer {\n latestReview: Review\n data: SomeData!\n}\n\ntype SomeData {\n reviewsValue: String!\n}\n\ninput AddReviewInput {\n body: String!\n authorId: Int!\n upc: Int!\n}\n\ntype AddReviewPayload {\n review: Review\n}",
222224
"Extensions": [
223225
"extend type Query {\n authorById(id: ID!\n @is(field: \"id\")): User\n productById(id: ID!\n @is(field: \"id\")): Product\n}\n\nschema\n @rename(coordinate: \"Query.authorById\", newName: \"userById\") {\n\n}"
224226
],

‎src/HotChocolate/Fusion/test/Composition.Tests/__snapshots__/DemoIntegrationTests.Accounts_And_Reviews2_Products_With_Nodes.graphql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ type Mutation {
7979
type Subscription {
8080
onNewReview: Review!
8181
@resolver(subgraph: "Reviews2", select: "{ onNewReview }", kind: "SUBSCRIBE")
82+
onNewReviewError: Review!
83+
@resolver(subgraph: "Reviews2", select: "{ onNewReviewError }", kind: "SUBSCRIBE")
8284
}
8385

8486
type AddReviewPayload {

‎src/HotChocolate/Fusion/test/Core.Tests/DemoIntegrationTests.cs

Lines changed: 53 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -362,12 +362,11 @@ public async Task Authors_And_Reviews_Subscription_OnNewReview()
362362

363363
// act
364364
var fusionGraph = await new FusionGraphComposer(logFactory: _logFactory).ComposeAsync(
365-
new[]
366-
{
365+
[
367366
demoProject.Reviews2.ToConfiguration(Reviews2ExtensionSdl),
368-
demoProject.Accounts.ToConfiguration(AccountsExtensionSdl),
369-
},
370-
default,
367+
demoProject.Accounts.ToConfiguration(AccountsExtensionSdl)
368+
],
369+
null,
371370
cts.Token);
372371

373372
var executor = await new ServiceCollection()
@@ -403,6 +402,55 @@ subscription OnNewReview {
403402
await snapshot.MatchMarkdownAsync(cts.Token);
404403
}
405404

405+
[Fact]
406+
public async Task Authors_And_Reviews_Subscription_OnNewReviewError()
407+
{
408+
// arrange
409+
using var cts = TestEnvironment.CreateCancellationTokenSource();
410+
using var demoProject = await DemoProject.CreateAsync(cts.Token);
411+
412+
// act
413+
var fusionGraph = await new FusionGraphComposer(logFactory: _logFactory).ComposeAsync(
414+
[
415+
demoProject.Reviews2.ToConfiguration(Reviews2ExtensionSdl),
416+
demoProject.Accounts.ToConfiguration(AccountsExtensionSdl)
417+
],
418+
null,
419+
cts.Token);
420+
421+
var executor = await new ServiceCollection()
422+
.AddSingleton(demoProject.HttpClientFactory)
423+
.AddSingleton(demoProject.WebSocketConnectionFactory)
424+
.AddFusionGatewayServer()
425+
.ConfigureFromDocument(SchemaFormatter.FormatAsDocument(fusionGraph))
426+
.BuildRequestExecutorAsync(cancellationToken: cts.Token);
427+
428+
var request = Parse(
429+
"""
430+
subscription OnNewReview {
431+
onNewReviewError {
432+
body
433+
author {
434+
name
435+
}
436+
}
437+
}
438+
""");
439+
440+
// act
441+
await using var result = await executor.ExecuteAsync(
442+
OperationRequestBuilder
443+
.New()
444+
.SetDocument(request)
445+
.Build(),
446+
cts.Token);
447+
448+
// assert
449+
var snapshot = new Snapshot();
450+
await CollectStreamSnapshotData(snapshot, request, result, cts.Token);
451+
await snapshot.MatchMarkdownAsync(cts.Token);
452+
}
453+
406454
[Fact]
407455
public async Task Authors_And_Reviews_Subscription_OnError()
408456
{
@@ -1913,60 +1961,6 @@ query TopProducts {
19131961
Assert.Null(result.ExpectOperationResult().Errors);
19141962
}
19151963

1916-
// TODO : FIX THIS TEST
1917-
[Fact(Skip = "This test does not work anymore as it uses CCN which we removed ... ")]
1918-
public async Task ResolveByKey_Handles_Null_Item_Correctly()
1919-
{
1920-
// arrange
1921-
using var demoProject = await DemoProject.CreateAsync();
1922-
1923-
// act
1924-
var fusionGraph = await new FusionGraphComposer(logFactory: _logFactory).ComposeAsync(
1925-
new[]
1926-
{
1927-
demoProject.Products.ToConfiguration(),
1928-
demoProject.Resale.ToConfiguration(),
1929-
}, new FusionFeatureCollection(FusionFeatures.NodeField));
1930-
1931-
var executor = await new ServiceCollection()
1932-
.AddSingleton(demoProject.HttpClientFactory)
1933-
.AddSingleton(demoProject.WebSocketConnectionFactory)
1934-
.AddFusionGatewayServer()
1935-
.ConfigureFromDocument(SchemaFormatter.FormatAsDocument(fusionGraph))
1936-
.BuildRequestExecutorAsync();
1937-
1938-
var request = Parse(
1939-
"""
1940-
{
1941-
viewer {
1942-
# The second product does not exist in the products subgraph
1943-
recommendedResalableProducts {
1944-
edges {
1945-
node {
1946-
product? {
1947-
id
1948-
name
1949-
}
1950-
}
1951-
}
1952-
}
1953-
}
1954-
}
1955-
""");
1956-
1957-
// act
1958-
await using var result = await executor.ExecuteAsync(
1959-
OperationRequestBuilder
1960-
.New()
1961-
.SetDocument(request)
1962-
.Build());
1963-
1964-
// assert
1965-
var snapshot = new Snapshot();
1966-
CollectSnapshotData(snapshot, request, result);
1967-
await snapshot.MatchMarkdownAsync();
1968-
}
1969-
19701964
public sealed class HotReloadConfiguration : IObservable<GatewayConfiguration>
19711965
{
19721966
private GatewayConfiguration _configuration;

‎src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/ConfigurationRewriterTests.Rewrite_HttpClient_Configuration.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ type Mutation {
2828

2929
type Subscription {
3030
onNewReview: Review! @resolver(subgraph: "Reviews2", select: "{ onNewReview }", kind: "SUBSCRIBE")
31+
onNewReviewError: Review! @resolver(subgraph: "Reviews2", select: "{ onNewReviewError }", kind: "SUBSCRIBE")
3132
}
3233

3334
type AddReviewPayload {
@@ -123,6 +124,7 @@ type Mutation {
123124

124125
type Subscription {
125126
onNewReview: Review! @resolver(subgraph: "Reviews2", select: "{ onNewReview }", kind: "SUBSCRIBE")
127+
onNewReviewError: Review! @resolver(subgraph: "Reviews2", select: "{ onNewReviewError }", kind: "SUBSCRIBE")
126128
}
127129

128130
type AddReviewPayload {

‎src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.Authors_And_Reviews_And_Products_AutoCompose.graphql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ type Mutation {
6868
type Subscription {
6969
onNewReview: Review!
7070
@resolver(subgraph: "Reviews2", select: "{ onNewReview }", kind: "SUBSCRIBE")
71+
onNewReviewError: Review!
72+
@resolver(subgraph: "Reviews2", select: "{ onNewReviewError }", kind: "SUBSCRIBE")
7173
}
7274

7375
type AddReviewPayload {

‎src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.Authors_And_Reviews_And_Products_Introspection.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,13 @@
458458
"name": null,
459459
"kind": "NON_NULL"
460460
}
461+
},
462+
{
463+
"name": "onNewReviewError",
464+
"type": {
465+
"name": null,
466+
"kind": "NON_NULL"
467+
}
461468
}
462469
]
463470
},

‎src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.Authors_And_Reviews_AutoCompose.graphql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ type Mutation {
4949
type Subscription {
5050
onNewReview: Review!
5151
@resolver(subgraph: "Reviews2", select: "{ onNewReview }", kind: "SUBSCRIBE")
52+
onNewReviewError: Review!
53+
@resolver(subgraph: "Reviews2", select: "{ onNewReviewError }", kind: "SUBSCRIBE")
5254
}
5355

5456
type AddReviewPayload {

‎src/HotChocolate/Fusion/test/Core.Tests/__snapshots__/DemoIntegrationTests.Authors_And_Reviews_Subscription_OnError.md

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,7 @@
2121
{
2222
"errors": [
2323
{
24-
"message": "Cannot return null for non-nullable field.",
25-
"locations": [
26-
{
27-
"line": 2,
28-
"column": 5
29-
}
30-
],
31-
"path": [
32-
"onError"
33-
],
34-
"extensions": {
35-
"code": "HC0018"
36-
}
24+
"message": "Unexpected Execution Error"
3725
}
3826
],
3927
"data": null

0 commit comments

Comments
 (0)