Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ await Results.Problem(

var requestBuilder = OperationRequestBuilder.New()
.SetDocument(endpointDescriptor.Document)
.SetErrorHandlingMode(ErrorHandlingMode.Halt)
.SetErrorHandlingMode(ErrorHandlingMode.Propagate)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobias-tengler I have removed the halt mode as there are serious issues with it in combination with incremental delivery.

This has a downstream effect to this as we now get the old behavior as default.

I am not yet sure what we should use, we could also use the new error mode here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhmm. The intention was that there is no unnecessary work done if there's a single error the entire request will be returned as empty with HTTP 500 in that case anyway...

.SetVariableValues(variables);

await session.OnCreateAsync(context, requestBuilder, cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,7 @@ public GraphQLRequest ParseRequestFromParams(IQueryCollection parameters)
extensions = JsonDocument.Parse(se);
}

ErrorHandlingMode? errorHandlingMode = null;
if (!string.IsNullOrEmpty(onError))
{
errorHandlingMode = ParseErrorHandlingMode(onError);
}
var errorHandlingMode = ParseErrorHandlingMode(onError);

return new GraphQLRequest(
document,
Expand Down Expand Up @@ -327,8 +323,13 @@ public GraphQLRequest ParsePersistedOperationRequestFromParams(
return (documentHash, document);
}

private static ErrorHandlingMode? ParseErrorHandlingMode(string onError)
private static ErrorHandlingMode? ParseErrorHandlingMode(string? onError)
{
if (string.IsNullOrEmpty(onError))
{
return null;
}

if (onError.Equals("PROPAGATE", StringComparison.OrdinalIgnoreCase))
{
return ErrorHandlingMode.Propagate;
Expand All @@ -339,12 +340,8 @@ public GraphQLRequest ParsePersistedOperationRequestFromParams(
return ErrorHandlingMode.Null;
}

if (onError.Equals("HALT", StringComparison.OrdinalIgnoreCase))
{
return ErrorHandlingMode.Halt;
}

return null;
throw new InvalidGraphQLRequestException(
$"Unknown 'onError' value '{onError}'. Allowed values are 'PROPAGATE' or 'NULL'.");
}

public GraphQLRequest[] ParseRequest(string sourceText)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,6 @@ private static string GetErrorHandlingModeAsString(ErrorHandlingMode mode)
{
ErrorHandlingMode.Propagate => "PROPAGATE",
ErrorHandlingMode.Null => "NULL",
ErrorHandlingMode.Halt => "HALT",
_ => throw new ArgumentOutOfRangeException(nameof(mode))
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,6 @@ private static string GetErrorHandlingModeAsString(ErrorHandlingMode mode)
{
ErrorHandlingMode.Propagate => "PROPAGATE",
ErrorHandlingMode.Null => "NULL",
ErrorHandlingMode.Halt => "HALT",
_ => throw new ArgumentOutOfRangeException(nameof(mode))
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,27 @@ public async Task After_Execution_StatusCode_Is_200()
Assert.Equal(OK, response.StatusCode);
}

[Fact]
public async Task Unknown_OnError_Value_Returns_BadRequest()
{
// arrange
var client = GetClient(Latest);

// act
using var request = new HttpRequestMessage(HttpMethod.Post, s_url);
request.Content = new StringContent(
"""{"query":"{ __typename }","onError":"HALT"}""",
System.Text.Encoding.UTF8,
"application/json");

using var response = await client.SendAsync(request);

// assert
Assert.Equal(BadRequest, response.StatusCode);
var body = await response.Content.ReadAsStringAsync();
Assert.Contains("onError", body, StringComparison.OrdinalIgnoreCase);
}

private HttpClient GetClient(HttpTransportVersion serverTransportVersion)
{
var server = CreateStarWarsServer(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ public async Task Post_GraphQL_Query_With_OnError()

// act
var response = await client.PostAsync(
new OperationRequest(query, onError: ErrorHandlingMode.Halt),
new OperationRequest(query, onError: ErrorHandlingMode.Null),
cts.Token);

// assert
Expand All @@ -451,7 +451,7 @@ public async Task Post_GraphQL_Query_With_OnError()
"""
{
"data": {
"errorHandlingMode": "HALT"
"errorHandlingMode": "NULL"
}
}
""");
Expand Down Expand Up @@ -766,7 +766,7 @@ public async Task Get_GraphQL_Query_With_OnError()

// act
var response = await client.GetAsync(
new OperationRequest(query, onError: ErrorHandlingMode.Halt),
new OperationRequest(query, onError: ErrorHandlingMode.Null),
cts.Token);

// assert
Expand All @@ -775,7 +775,7 @@ public async Task Get_GraphQL_Query_With_OnError()
"""
{
"data": {
"errorHandlingMode": "HALT"
"errorHandlingMode": "NULL"
}
}
""");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ internal sealed partial class OperationContext
private int _branchId;
private int _variableIndex;
private object? _rootValue;
private bool _propagateNullValues;
private bool _isInitialized;

public OperationContext(
Expand Down Expand Up @@ -87,6 +88,10 @@ public void Initialize(
_batchDispatcher = batchDispatcher;
_variableIndex = variableIndex;

var errorHandlingMode = _requestContext.Request.ErrorHandlingMode
?? _schema.GetOptions().DefaultErrorHandlingMode;
_propagateNullValues = errorHandlingMode is Language.ErrorHandlingMode.Propagate;

IncludeFlags = operation.CreateIncludeFlags(variables);
DeferFlags = operation.CreateDeferFlags(variables);
Result.Data = new ResultDocument(operation, IncludeFlags);
Expand Down Expand Up @@ -128,6 +133,7 @@ public void InitializeDeferContext(
_currentBranchTracker = context._currentBranchTracker;
_currentWorkScheduler = context._currentWorkScheduler;
_currentDeferExecutionCoordinator = context._currentDeferExecutionCoordinator;
_propagateNullValues = context._propagateNullValues;
_branchId = executionBranchId;
_isInitialized = true;

Expand Down Expand Up @@ -178,6 +184,7 @@ public void Clean()
_resolveQueryRootValue = null!;
_batchDispatcher = null!;
_branchId = int.MinValue;
_propagateNullValues = false;
_isInitialized = false;
Result.Reset();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ public CancellationToken RequestAborted
}
}

/// <summary>
/// Gets whether null values should be propagated to nullable parents
/// (standard GraphQL behavior). When false, null stays at the field that caused it.
/// </summary>
public bool PropagateNullValues => _propagateNullValues;

/// <inheritdoc />
public IFeatureCollection Features
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,15 @@ private void CompleteValues(

if (resultValue is { IsNullable: false, IsNullOrInvalidated: true })
{
PropagateNullValues(resultValue);
if (_operationContext.PropagateNullValues)
{
PropagateNullValues(resultValue);
}
else
{
resultValue.SetNullValue();
}

_completionStatus = ExecutionTaskStatus.Faulted;
_operationContext.Result.AddNonNullViolation(context.Path);
_taskBuffer.Clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,15 @@ private void CompleteValue(bool success, CancellationToken cancellationToken)

if (resultValue is { IsNullable: false, IsNullOrInvalidated: true })
{
PropagateNullValues(resultValue);
if (_operationContext.PropagateNullValues)
{
PropagateNullValues(resultValue);
}
else
{
resultValue.SetNullValue();
}

_completionStatus = ExecutionTaskStatus.Faulted;
_operationContext.Result.AddNonNullViolation(_context.Path);
_taskBuffer.Clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,15 @@ private static void ResolveAndCompleteInline(

if (fieldValue is { IsNullable: false, IsNullOrInvalidated: true })
{
PropagateNullValues(fieldValue);
if (operationContext.PropagateNullValues)
{
PropagateNullValues(fieldValue);
}
else
{
fieldValue.SetNullValue();
}

operationContext.Result.AddNonNullViolation(fieldValue.Path);
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/HotChocolate/Core/src/Types/IReadOnlySchemaOptions.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Reflection;
using HotChocolate.Configuration;
using HotChocolate.Execution;
using HotChocolate.Language;
using HotChocolate.Types;

namespace HotChocolate;
Expand Down Expand Up @@ -230,4 +231,9 @@ public interface IReadOnlySchemaOptions
/// Applies the @serializeAs directive to scalar types that specify a serialization format.
/// </summary>
bool ApplySerializeAsToScalars { get; }

/// <summary>
/// Gets the default error handling mode for null propagation.
/// </summary>
ErrorHandlingMode DefaultErrorHandlingMode { get; }
}
7 changes: 6 additions & 1 deletion src/HotChocolate/Core/src/Types/SchemaOptions.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Reflection;
using HotChocolate.Configuration;
using HotChocolate.Execution;
using HotChocolate.Language;
using HotChocolate.Types;

namespace HotChocolate;
Expand Down Expand Up @@ -189,6 +190,9 @@ public int OperationDocumentCacheSize
/// <inheritdoc cref="IReadOnlySchemaOptions.ApplySerializeAsToScalars"/>
public bool ApplySerializeAsToScalars { get; set; }

/// <inheritdoc cref="IReadOnlySchemaOptions.DefaultErrorHandlingMode"/>
public ErrorHandlingMode DefaultErrorHandlingMode { get; set; } = ErrorHandlingMode.Propagate;

/// <summary>
/// Creates a mutable options object from a read-only options object.
/// </summary>
Expand Down Expand Up @@ -230,6 +234,7 @@ public static SchemaOptions FromOptions(IReadOnlySchemaOptions options)
ApplyShareableToPageInfo = options.ApplyShareableToPageInfo,
ApplyShareableToConnections = options.ApplyShareableToConnections,
ApplyShareableToNodeFields = options.ApplyShareableToNodeFields,
ApplySerializeAsToScalars = options.ApplySerializeAsToScalars
ApplySerializeAsToScalars = options.ApplySerializeAsToScalars,
DefaultErrorHandlingMode = options.DefaultErrorHandlingMode
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ public void BuildRequest_SetErrorHandlingMode()
var request =
OperationRequestBuilder.New()
.SetDocument("{ foo }")
.SetErrorHandlingMode(ErrorHandlingMode.Halt)
.SetErrorHandlingMode(ErrorHandlingMode.Null)
.Build();

// assert
Expand All @@ -330,7 +330,7 @@ public void BuildRequest_SetErrorHandlingMode_VariableBatchRequest()
var request =
OperationRequestBuilder.New()
.SetDocument("{ foo }")
.SetErrorHandlingMode(ErrorHandlingMode.Halt)
.SetErrorHandlingMode(ErrorHandlingMode.Null)
.SetVariableValues([new Dictionary<string, object?> { ["one"] = "foo" }])
.Build();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"document": "{ foo }",
"errorHandlingMode": "Halt"
"errorHandlingMode": "Null"
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"document": "{ foo }",
"errorHandlingMode": "Halt",
"errorHandlingMode": "Null",
"variableValues": [
{
"one": "foo"
Expand Down
Loading
Loading