Skip to content

Commit ac31ff0

Browse files
N-Olbertmichaelstaibtobias-tengler
authored
Reworked / harmonized error handling and propagation for custom TypeConverter-errors (#8677)
Co-authored-by: Michael Staib <[email protected]> Co-authored-by: Tobias Tengler <[email protected]>
1 parent b4d0243 commit ac31ff0

File tree

91 files changed

+1989
-202
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

91 files changed

+1989
-202
lines changed

‎src/HotChocolate/Core/src/Execution.Abstractions/ErrorBuilder.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,38 @@ public ErrorBuilder AddLocation(Location location)
8585
return this;
8686
}
8787

88+
/// <summary>
89+
/// Adds a GraphQL operation document location to the error, if the error does not already have a location.
90+
/// </summary>
91+
/// <param name="location">The location of the error.</param>
92+
/// <returns>The error builder.</returns>
93+
public ErrorBuilder TryAddLocation(Location location)
94+
{
95+
if (_locations == null || _locations.Count == 0)
96+
{
97+
_locations ??= [];
98+
_locations.Add(location);
99+
}
100+
101+
return this;
102+
}
103+
104+
/// <summary>
105+
/// Adds a GraphQL operation document location to the error.
106+
/// </summary>
107+
/// <param name="location">The location of the error.</param>
108+
/// <returns>The error builder.</returns>
109+
public ErrorBuilder TryAddLocation(Language.Location? location)
110+
{
111+
if (location != null && (_locations == null || _locations.Count == 0))
112+
{
113+
_locations ??= [];
114+
_locations.Add(new Location(location.Line, location.Column));
115+
}
116+
117+
return this;
118+
}
119+
88120
/// <summary>
89121
/// Clears the locations of the error.
90122
/// </summary>

‎src/HotChocolate/Core/src/Execution.Abstractions/ErrorBuilderExtensions.cs‎

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,30 @@ public static class ErrorBuilderExtensions
1212
/// Sets the field coordinate of the error.
1313
/// </summary>
1414
/// <param name="builder">The error builder.</param>
15-
/// <param name="fieldCoordinate">The field coordinate.</param>
15+
/// <param name="coordinate">The field coordinate.</param>
1616
/// <returns>The error builder.</returns>
17-
public static ErrorBuilder SetFieldCoordinate(
17+
public static ErrorBuilder SetCoordinate(
1818
this ErrorBuilder builder,
19-
SchemaCoordinate fieldCoordinate)
19+
SchemaCoordinate coordinate)
2020
{
2121
ArgumentNullException.ThrowIfNull(builder);
2222

23-
return builder.SetExtension(nameof(fieldCoordinate), fieldCoordinate.ToString());
23+
return builder.SetExtension(nameof(coordinate), coordinate.ToString());
24+
}
25+
26+
/// <summary>
27+
/// Sets the input path of the error.
28+
/// </summary>
29+
/// <param name="builder">The error builder.</param>
30+
/// <param name="inputPath">The input path.</param>
31+
/// <returns>The error builder.</returns>
32+
public static ErrorBuilder SetInputPath(
33+
this ErrorBuilder builder,
34+
Path inputPath)
35+
{
36+
ArgumentNullException.ThrowIfNull(builder);
37+
38+
return builder.SetExtension(nameof(inputPath), inputPath);
2439
}
2540

2641
/// <summary>
@@ -58,6 +73,23 @@ public static ErrorBuilder AddLocation(this ErrorBuilder builder, ISyntaxNode no
5873
return builder;
5974
}
6075

76+
/// <summary>
77+
/// Adds a location to the error if the error does not already have a location.
78+
/// </summary>
79+
/// <param name="builder">The error builder.</param>
80+
/// <param name="node">The syntax node.</param>
81+
/// <returns>The error builder.</returns>
82+
public static ErrorBuilder TryAddLocation(this ErrorBuilder builder, ISyntaxNode? node)
83+
{
84+
if (node?.Location is null)
85+
{
86+
return builder;
87+
}
88+
89+
builder.TryAddLocation(new Location(node.Location.Line, node.Location.Column));
90+
return builder;
91+
}
92+
6193
/// <summary>
6294
/// Adds multiple locations to the error.
6395
/// </summary>

‎src/HotChocolate/Core/src/Execution/ErrorHelper.cs‎

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,36 +10,23 @@ internal static class ErrorHelper
1010
{
1111
public static IError ArgumentNonNullError(
1212
ArgumentNode argument,
13-
string responseName,
1413
ArgumentNonNullValidator.ValidationResult validationResult)
1514
{
1615
return ErrorBuilder.New()
1716
.SetMessage(
1817
ErrorHelper_ArgumentNonNullError_Message,
1918
argument.Name.Value)
2019
.AddLocation(argument)
21-
.SetExtension("responseName", responseName)
2220
.SetExtension("errorPath", validationResult.Path)
2321
.Build();
2422
}
2523

2624
public static IError ArgumentValueIsInvalid(
27-
ArgumentNode argument,
28-
string responseName,
25+
ArgumentNode? argument,
2926
GraphQLException exception)
3027
{
3128
return ErrorBuilder.FromError(exception.Errors[0])
32-
.AddLocation(argument)
33-
.SetExtension("responseName", responseName)
34-
.Build();
35-
}
36-
37-
public static IError ArgumentDefaultValueIsInvalid(
38-
string responseName,
39-
GraphQLException exception)
40-
{
41-
return ErrorBuilder.FromError(exception.Errors[0])
42-
.SetExtension("responseName", responseName)
29+
.TryAddLocation(argument)
4330
.Build();
4431
}
4532

@@ -49,7 +36,7 @@ public static IError InvalidLeafValue(
4936
Path path)
5037
{
5138
return ErrorBuilder.FromError(exception.Errors[0])
52-
.AddLocation(field)
39+
.TryAddLocation(field)
5340
.SetPath(path)
5441
.SetCode(ErrorCodes.Execution.CannotSerializeLeafValue)
5542
.Build();

‎src/HotChocolate/Core/src/Execution/Processing/MiddlewareContext.Arguments.cs‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,18 @@ public T ArgumentValue<T>(string name)
2020
throw ResolverContext_ArgumentDoesNotExist(_selection.SyntaxNode, Path, name);
2121
}
2222

23-
return CoerceArgumentValue<T>(argument);
23+
try
24+
{
25+
return CoerceArgumentValue<T>(argument);
26+
}
27+
catch (SerializationException ex)
28+
{
29+
var syntaxNode = Selection.Arguments[argument.Name].ValueLiteral;
30+
throw new SerializationException(
31+
ErrorBuilder.FromError(ex.Errors[0]).SetPath(Path).TryAddLocation(syntaxNode).Build(),
32+
ex.Type,
33+
Path);
34+
}
2435
}
2536

2637
public Optional<T> ArgumentOptional<T>(string name)
@@ -94,7 +105,7 @@ private T CoerceArgumentValue<T>(ArgumentValue argument)
94105
}
95106

96107
if (value is T castedValue
97-
|| _operationContext.Converter.TryConvert(value, out castedValue))
108+
|| _operationContext.Converter.TryConvert(value, out castedValue, out var conversionException))
98109
{
99110
return castedValue;
100111
}
@@ -114,7 +125,8 @@ private T CoerceArgumentValue<T>(ArgumentValue argument)
114125
_selection.SyntaxNode,
115126
Path,
116127
argument.Name,
117-
typeof(T));
128+
typeof(T),
129+
conversionException);
118130
}
119131

120132
public IReadOnlyDictionary<string, ArgumentValue> ReplaceArguments(

‎src/HotChocolate/Core/src/Execution/Processing/MiddlewareContext.Pure.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ private T CoerceArgumentValue<T>(ArgumentValue argument)
234234
DefaultTypeConverter.Default;
235235

236236
if (value is T castedValue
237-
|| _typeConverter.TryConvert(value, out castedValue))
237+
|| _typeConverter.TryConvert(value, out castedValue, out var conversionException))
238238
{
239239
return castedValue;
240240
}
@@ -270,7 +270,11 @@ private T CoerceArgumentValue<T>(ArgumentValue argument)
270270

271271
// we are unable to convert the argument to the request type.
272272
throw ResolverContext_CannotConvertArgument(
273-
_selection.SyntaxNode, Path, argument.Name, typeof(T));
273+
_selection.SyntaxNode,
274+
Path,
275+
argument.Name,
276+
typeof(T),
277+
conversionException);
274278
}
275279
}
276280
}

‎src/HotChocolate/Core/src/Execution/Processing/OperationCompiler.ArgumentValues.cs‎

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Diagnostics;
12
using HotChocolate.Language;
23
using HotChocolate.Resolvers;
34
using HotChocolate.Types;
@@ -9,8 +10,7 @@ public sealed partial class OperationCompiler
910
{
1011
private ArgumentMap? CoerceArgumentValues(
1112
ObjectField field,
12-
FieldNode selection,
13-
string responseName)
13+
FieldNode selection)
1414
{
1515
if (field.Arguments.Count == 0)
1616
{
@@ -26,13 +26,7 @@ public sealed partial class OperationCompiler
2626
argumentValue.Name.Value,
2727
out var argument))
2828
{
29-
arguments[argument.Name] =
30-
CreateArgumentValue(
31-
responseName,
32-
argument,
33-
argumentValue,
34-
argumentValue.Value,
35-
false);
29+
arguments[argument.Name] = CreateArgumentValue(argument, argumentValue, argumentValue.Value, false);
3630
}
3731
}
3832

@@ -41,21 +35,15 @@ public sealed partial class OperationCompiler
4135
var argument = field.Arguments[i];
4236
if (!arguments.ContainsKey(argument.Name))
4337
{
44-
arguments[argument.Name] =
45-
CreateArgumentValue(
46-
responseName,
47-
argument,
48-
null,
49-
argument.DefaultValue ?? NullValueNode.Default,
50-
true);
38+
var value = argument.DefaultValue ?? NullValueNode.Default;
39+
arguments[argument.Name] = CreateArgumentValue(argument, null, value, true);
5140
}
5241
}
5342

5443
return new ArgumentMap(arguments);
5544
}
5645

5746
private ArgumentValue CreateArgumentValue(
58-
string responseName,
5947
Argument argument,
6048
ArgumentNode? argumentValue,
6149
IValueNode value,
@@ -73,7 +61,6 @@ private ArgumentValue CreateArgumentValue(
7361
argument,
7462
ErrorHelper.ArgumentNonNullError(
7563
argumentValue,
76-
responseName,
7764
validationResult));
7865
}
7966

@@ -91,16 +78,9 @@ private ArgumentValue CreateArgumentValue(
9178
}
9279
catch (SerializationException ex)
9380
{
94-
if (argumentValue is not null)
95-
{
96-
return new ArgumentValue(
97-
argument,
98-
ErrorHelper.ArgumentValueIsInvalid(argumentValue, responseName, ex));
99-
}
100-
10181
return new ArgumentValue(
10282
argument,
103-
ErrorHelper.ArgumentDefaultValueIsInvalid(responseName, ex));
83+
ErrorHelper.ArgumentValueIsInvalid(argumentValue, ex));
10484
}
10585
}
10686

@@ -140,6 +120,7 @@ internal static FieldDelegate CreateFieldPipeline(
140120
Schema schema,
141121
ObjectField field,
142122
FieldNode selection,
123+
Path? path,
143124
HashSet<string> processed,
144125
List<FieldMiddleware> pipelineComponents)
145126
{
@@ -152,7 +133,7 @@ internal static FieldDelegate CreateFieldPipeline(
152133

153134
// if we have selection directives we will inspect them and try to build a
154135
// pipeline from them if they have middleware components.
155-
BuildDirectivePipeline(schema, selection, processed, pipelineComponents);
136+
BuildDirectivePipeline(schema, selection, path, processed, pipelineComponents);
156137

157138
// if we found middleware components on the selection directives we will build a new
158139
// pipeline.
@@ -200,6 +181,7 @@ internal static FieldDelegate CreateFieldPipeline(
200181
private static void BuildDirectivePipeline(
201182
Schema schema,
202183
FieldNode selection,
184+
Path? path,
203185
HashSet<string> processed,
204186
List<FieldMiddleware> pipelineComponents)
205187
{
@@ -210,10 +192,23 @@ private static void BuildDirectivePipeline(
210192
&& directiveType.Middleware is not null
211193
&& (directiveType.IsRepeatable || processed.Add(directiveType.Name)))
212194
{
213-
var directive = new Directive(
214-
directiveType,
215-
directiveNode,
216-
directiveType.Parse(directiveNode));
195+
Debug.Assert(path != null, "path should not be null if a directive is present.");
196+
Directive directive;
197+
try
198+
{
199+
directive = new Directive(
200+
directiveType,
201+
directiveNode,
202+
directiveType.Parse(directiveNode));
203+
}
204+
catch (SerializationException ex)
205+
{
206+
throw new SerializationException(
207+
ErrorBuilder.FromError(ex.Errors[0]).SetPath(path).TryAddLocation(directiveNode).Build(),
208+
ex.Type,
209+
path);
210+
}
211+
217212
var directiveMiddleware = directiveType.Middleware;
218213
pipelineComponents.Add(next => directiveMiddleware(next, directive));
219214
}

‎src/HotChocolate/Core/src/Execution/Processing/OperationCompiler.cs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public OperationCompiler(InputParser parser)
5050
schema,
5151
field,
5252
selection,
53+
null,
5354
_directiveNames,
5455
_pipelineComponents);
5556
}
@@ -221,18 +222,26 @@ private void CompleteResolvers(Schema schema)
221222
{
222223
ref var searchSpace = ref GetReference(AsSpan(_selections));
223224

225+
Path? path = null;
224226
for (var i = 0; i < _selections.Count; i++)
225227
{
226228
var selection = Unsafe.Add(ref searchSpace, i);
227-
229+
path = path?.Append(selection.ResponseName);
228230
if (selection.ResolverPipeline is null && selection.PureResolver is null)
229231
{
230-
var field = Unsafe.As<ObjectField>(selection.Field);
232+
var field = selection.Field;
231233
var syntaxNode = selection.SyntaxNode;
234+
if (syntaxNode.Directives.Count > 0 && path == null)
235+
{
236+
// create the path only on demand
237+
path = PathHelper.CreatePathFromSelection(_selections, i + 1);
238+
}
239+
232240
var resolver = CreateFieldPipeline(
233241
schema,
234242
field,
235243
syntaxNode,
244+
path,
236245
_directiveNames,
237246
_pipelineComponents);
238247
var pureResolver = TryCreatePureField(schema, field, syntaxNode);
@@ -446,7 +455,7 @@ selection.SelectionSet is not null
446455
selection.SelectionSet.Selections))
447456
: selection,
448457
responseName: responseName,
449-
arguments: CoerceArgumentValues(field, selection, responseName),
458+
arguments: CoerceArgumentValues(field, selection),
450459
includeConditions: includeCondition == 0
451460
? null
452461
: [includeCondition],

0 commit comments

Comments
 (0)