Skip to content

Commit

Permalink
Associate subgraph transport errors with fields (#7347)
Browse files Browse the repository at this point in the history
  • Loading branch information
tobias-tengler authored Aug 22, 2024
1 parent efe5acc commit 91fc947
Show file tree
Hide file tree
Showing 60 changed files with 676 additions and 1,834 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,9 @@ public ValueTask<OperationResult> ReadAsResultAsync(CancellationToken cancellati
#endif
}

// if the media type is anything else we will return a transport error.
return new ValueTask<OperationResult>(_transportError);
_message.EnsureSuccessStatusCode();

throw new InvalidOperationException("Received a successful response with an unexpected content type.");
}

#if NET6_0_OR_GREATER
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Net;
using System.Text.Json;
using CookieCrumble;
using HotChocolate.AspNetCore.Tests.Utilities;
Expand All @@ -14,6 +15,82 @@ public class GraphQLHttpClientTests : ServerTestBase
/// <inheritdoc />
public GraphQLHttpClientTests(TestServerFactory serverFactory) : base(serverFactory) { }

[Fact]
public async Task Post_Http_200_Wrong_Content_Type()
{
// arrange
var httpClient = new HttpClient(new CustomHttpClientHandler(HttpStatusCode.OK));

var query =
"""
query {
hero(episode: JEDI) {
name
}
}
""";

var client = new DefaultGraphQLHttpClient(httpClient);

// act
var response = await client.PostAsync(query, "/graphql");

async Task Error() => await response.ReadAsResultAsync();

// assert
var exception = await Assert.ThrowsAsync<InvalidOperationException>(Error);
Assert.Equal("Received a successful response with an unexpected content type.", exception.Message);
}

[Fact]
public async Task Post_Http_404_Wrong_Content_Type()
{
var httpClient = new HttpClient(new CustomHttpClientHandler(HttpStatusCode.NotFound));

var query =
"""
query {
hero(episode: JEDI) {
name
}
}
""";

var client = new DefaultGraphQLHttpClient(httpClient);

// act
var response = await client.PostAsync(query, "/graphql");

async Task Error() => await response.ReadAsResultAsync();

// assert
await Assert.ThrowsAsync<HttpRequestException>(Error);
}

[Fact]
public async Task Post_Transport_Error()
{
var httpClient = new HttpClient(new CustomHttpClientHandler());

var query =
"""
query {
hero(episode: JEDI) {
name
}
}
""";

var client = new DefaultGraphQLHttpClient(httpClient);

// act
async Task Error() => await client.PostAsync(query, "/graphql");

// assert
var exception = await Assert.ThrowsAsync<Exception>(Error);
Assert.Equal("Something went wrong", exception.Message);
}

[Fact]
public async Task Post_GraphQL_Query_With_RequestUri()
{
Expand Down Expand Up @@ -778,6 +855,21 @@ public async Task Post_GraphQL_FileUpload_With_ObjectValueNode()
""");
}

private class CustomHttpClientHandler(HttpStatusCode? httpStatusCode = null) : HttpClientHandler
{
protected override Task<HttpResponseMessage> SendAsync(
HttpRequestMessage request,
CancellationToken cancellationToken)
{
if (httpStatusCode.HasValue)
{
return Task.FromResult(new HttpResponseMessage(httpStatusCode.Value));
}

throw new Exception("Something went wrong");
}
}

public class ErrorSubscription
{
public async IAsyncEnumerable<string> CreateStream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ namespace HotChocolate.Fusion.Clients;

internal sealed class DefaultHttpGraphQLClient : IGraphQLClient
{
private static readonly GraphQLResponse _transportError = new(CreateTransportError());
private readonly HttpClientConfiguration _config;
private readonly DefaultGraphQLHttpClient _client;

Expand Down Expand Up @@ -51,20 +50,12 @@ private async Task<GraphQLResponse> ExecuteInternalAsync(SubgraphGraphQLRequest
var result = await response.ReadAsResultAsync(ct).ConfigureAwait(false);
return new GraphQLResponse(result);
}
catch
catch (Exception exception)
{
return _transportError;
return new GraphQLResponse(exception);
}
}

private static JsonElement CreateTransportError()
{
return JsonDocument.Parse(
"""
[{"message": "Internal Execution Error"}]
""").RootElement;
}

public ValueTask DisposeAsync()
{
_client.Dispose();
Expand Down
30 changes: 7 additions & 23 deletions src/HotChocolate/Fusion/src/Core/Clients/GraphQLResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ public sealed class GraphQLResponse : IDisposable
{
private readonly IDisposable? _resource;

internal GraphQLResponse(JsonElement errors)
internal GraphQLResponse(Exception transportException)
{
Errors = errors;
TransportException = transportException;
}

internal GraphQLResponse(OperationResult result)
Expand All @@ -20,32 +20,16 @@ internal GraphQLResponse(OperationResult result)
Extensions = result.Extensions;
}

public GraphQLResponse(JsonDocument document)
{
_resource = document;

if (document.RootElement.TryGetProperty(ResponseProperties.Data, out var value))
{
Data = value;
}

if (document.RootElement.TryGetProperty(ResponseProperties.Errors, out value))
{
Errors = value;
}

if (document.RootElement.TryGetProperty(ResponseProperties.Extensions, out value))
{
Extensions = value;
}
}

public JsonElement Data { get; }

public JsonElement Errors { get; }

public JsonElement Extensions { get; }

public Exception? TransportException { get; private set; }

public void Dispose()
=> _resource?.Dispose();
{
_resource?.Dispose();
}
}
43 changes: 36 additions & 7 deletions src/HotChocolate/Fusion/src/Core/Execution/ExecutionUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using HotChocolate.Execution.Processing;
using HotChocolate.Fusion.Execution.Nodes;
using HotChocolate.Fusion.Metadata;
using HotChocolate.Fusion.Planning;
using HotChocolate.Fusion.Utilities;
using HotChocolate.Language;
using HotChocolate.Language.Visitors;
Expand Down Expand Up @@ -394,8 +395,8 @@ private static void ExtractSelectionResults(

while (Unsafe.IsAddressLessThan(ref selection, ref endSelection))
{
if (data.ValueKind is not JsonValueKind.Null &&
data.TryGetProperty(selection.ResponseName, out var value))
if (data.ValueKind is not JsonValueKind.Null
&& data.TryGetProperty(selection.ResponseName, out var value))
{
selectionData = selectionData.AddResult(new JsonResult(schemaName, value));
}
Expand All @@ -417,8 +418,8 @@ private static void ExtractSelectionResults(

while (Unsafe.IsAddressLessThan(ref selection, ref endSelection))
{
if (element.ValueKind is not JsonValueKind.Null &&
element.TryGetProperty(selection.ResponseName, out var value))
if (element.ValueKind is not JsonValueKind.Null
&& element.TryGetProperty(selection.ResponseName, out var value))
{
selectionData = selectionData.AddResult(new JsonResult(schemaName, value));
}
Expand Down Expand Up @@ -494,6 +495,33 @@ public static void TryInitializeExecutionState(QueryPlan queryPlan, ExecutionSta
executionState.IsInitialized = true;
}

public static void CreateTransportErrors(
Exception transportException,
ResultBuilder resultBuilder,
IErrorHandler errorHandler,
ObjectResult selectionSetResult,
List<RootSelection> rootSelections,
string subgraphName,
bool addDebugInfo)
{
foreach (var rootSelection in rootSelections)
{
var errorBuilder = errorHandler.CreateUnexpectedError(transportException);

errorBuilder.AddLocation(rootSelection.Selection.SyntaxNode);
errorBuilder.SetPath(PathHelper.CreatePathFromContext(rootSelection.Selection, selectionSetResult, 0));

if (addDebugInfo)
{
errorBuilder.SetExtension("subgraphName", subgraphName);
}

var error = errorHandler.Handle(errorBuilder.Build());

resultBuilder.AddError(error);
}
}

public static void ExtractErrors(
DocumentNode document,
OperationDefinitionNode operation,
Expand All @@ -509,10 +537,11 @@ public static void ExtractErrors(
return;
}

var path = PathHelper.CreatePathFromContext(selectionSetResult);
var parentPath = PathHelper.CreatePathFromContext(selectionSetResult);

foreach (var error in errors.EnumerateArray())
{
ExtractError(document, operation, resultBuilder, errorHandler, error, path, pathDepth, addDebugInfo);
ExtractError(document, operation, resultBuilder, errorHandler, error, parentPath, pathDepth, addDebugInfo);
}
}

Expand Down Expand Up @@ -724,7 +753,7 @@ private sealed class ErrorPathVisitor : SyntaxWalker<ErrorPathContext>

Visit(operation, context);

var field = context.Field;
var field = context.Field;

context.Reset();
Interlocked.Exchange(ref _errorPathContext, context);
Expand Down
12 changes: 12 additions & 0 deletions src/HotChocolate/Fusion/src/Core/Execution/Nodes/Resolve.cs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,18 @@ private void ProcessResponses(
var exportKeys = state.Requires;
var variableValues = state.VariableValues;

if (response.TransportException is not null)
{
CreateTransportErrors(
response.TransportException,
context.Result,
context.ErrorHandler,
selectionSetResult,
RootSelections,
subgraphName,
context.ShowDebugInfo);
}

ExtractErrors(
context.Operation.Document,
context.Operation.Definition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ protected override async Task OnExecuteAsync(
// query plan nodes be interested in it.
lock (executionState)
{
ProcessResult(context, response, batchExecutionState);
ProcessResult(context, response, batchExecutionState, SubgraphName);
}
}
catch (Exception ex)
Expand Down Expand Up @@ -142,14 +142,30 @@ private static void InitializeRequests(FusionExecutionContext context, List<Exec
private void ProcessResult(
FusionExecutionContext context,
GraphQLResponse response,
BatchExecutionState[] batchExecutionState)
BatchExecutionState[] batchExecutionState,
string subgraphName)
{
var result = UnwrapResult(response, Requires);
ref var batchState = ref MemoryMarshal.GetArrayDataReference(batchExecutionState);
ref var end = ref Unsafe.Add(ref batchState, batchExecutionState.Length);
var pathLength = Path.Length;
var first = true;

if (response.TransportException is not null)
{
foreach (var state in batchExecutionState)
{
CreateTransportErrors(
response.TransportException,
context.Result,
context.ErrorHandler,
state.SelectionSetResult,
RootSelections,
subgraphName,
context.ShowDebugInfo);
}
}

while (Unsafe.IsAddressLessThan(ref batchState, ref end))
{
if (first)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Runtime.CompilerServices;
using HotChocolate.Execution.Processing;
using HotChocolate.Fusion.Clients;
using HotChocolate.Fusion.Planning;
using HotChocolate.Fusion.Utilities;
using HotChocolate.Language;

Expand Down Expand Up @@ -31,6 +32,9 @@ internal readonly ref struct Config
/// <param name="selectionSet">
/// The selection set for which this request provides a patch.
/// </param>
/// <param name="rootSelections">
/// The root selections of this subgraph request.
/// </param>
/// <param name="provides">
/// The variables that this resolver node will provide.
/// </param>
Expand All @@ -51,6 +55,7 @@ public Config(
DocumentNode document,
ISelection? parent,
ISelectionSet selectionSet,
List<RootSelection> rootSelections,
IEnumerable<string> provides,
IEnumerable<string> requires,
IEnumerable<string> forwardedVariables,
Expand All @@ -63,6 +68,7 @@ public Config(
SubgraphName = subgraphName;
Document = document.ToString(false);
Parent = parent;
RootSelections = rootSelections;
SelectionSet = Unsafe.As<SelectionSet>(selectionSet);
Provides = CollectionUtils.CopyToArray(provides, ref buffer, ref usedCapacity);
Requires = CollectionUtils.CopyToArray(requires, ref buffer, ref usedCapacity);
Expand Down Expand Up @@ -94,6 +100,11 @@ public Config(
/// </summary>
public ISelection? Parent { get; }

/// <summary>
/// Gets the root selections of this subgraph request.
/// </summary>
public List<RootSelection> RootSelections { get; }

/// <summary>
/// Gets the selection set for which this request provides a patch.
/// </summary>
Expand Down
Loading

0 comments on commit 91fc947

Please sign in to comment.