Skip to content

Commit 141e7dd

Browse files
authored
Merge pull request #440 from fsprojects/coerce-errors-with-variable-name-and-path
2 parents 1045ef5 + 9325d0a commit 141e7dd

39 files changed

+1687
-1106
lines changed

samples/star-wars-api/MultipartRequest.fs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ module MultipartRequest =
8282
| NamedType tname -> tname = "Upload" || tname = "UploadRequest"
8383
| ListType t | NonNullType t -> isUpload t
8484
let ast = Parser.parse operation.Query
85-
let vardefs =
85+
let varDefs =
8686
ast.Definitions
8787
|> List.choose (function OperationDefinition def -> Some def.VariableDefinitions | _ -> None)
8888
|> List.collect id
89-
let vardef = vardefs |> List.find (fun x -> x.VariableName = varName)
90-
if isUpload vardef.Type
89+
let varDef = varDefs |> List.find (fun x -> x.VariableName = varName)
90+
if isUpload varDef.Type
9191
then
9292
match varValue.ValueKind with
9393
| JsonValueKind.Object ->

samples/star-wars-api/WebSocketJsonConverters.fs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ type GraphQLQueryConverter<'a>(executor : Executor<'a>, replacements: Map<string
8484
match vdef.DefaultValue, vdef.TypeDef with
8585
| Some _, _ -> ()
8686
| _, Nullable _ -> ()
87-
| None, _ -> failwithf "Variable %s has no default value and is missing!" vdef.Name
87+
| None, _ -> failwithf "A variable '$%s' has no default value and is missing!" vdef.Name
8888
acc)
8989
(ImmutableDictionary.CreateBuilder<string, JsonElement>())
9090
{ ExecutionPlan = executionPlan; Variables = variables.ToImmutable() }
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// The MIT License (MIT)
2+
3+
module FSharp.Data.GraphQL.ErrorMessagess
4+
5+
let variableNotFound variableName = $"A variable '$%s{variableName}' was not provided"
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
// The MIT License (MIT)
2+
namespace FSharp.Data.GraphQL
3+
4+
open System
5+
open System.Collections.Generic
6+
open FsToolkit.ErrorHandling
7+
open FSharp.Data.GraphQL.Types
8+
9+
type InputSource =
10+
| Variable of VarDef : VarDef
11+
| Argument of ArgDef : InputFieldDef
12+
| Unknown
13+
14+
[<Struct>]
15+
type internal ObjectFieldErrorDetails = { ObjectDef : InputDef; FieldDef : InputFieldDef voption }
16+
17+
type internal IInputSourceError =
18+
inherit IGQLError
19+
abstract member InputSource : InputSource with get, set
20+
21+
type internal CoercionError = {
22+
mutable InputSource : InputSource
23+
Message : string
24+
ErrorKind : ErrorKind
25+
Path : FieldPath
26+
FieldErrorDetails : ObjectFieldErrorDetails voption
27+
} with
28+
29+
interface IGQLError with
30+
31+
member this.Message = this.Message
32+
33+
interface IGQLErrorExtensions with
34+
35+
member this.Extensions =
36+
37+
[
38+
yield KeyValuePair (CustomErrorFields.Kind, this.ErrorKind |> box)
39+
match this.Path with
40+
| [] -> ()
41+
| path -> yield KeyValuePair (CustomErrorFields.Path, path |> List.rev |> box)
42+
43+
match this.InputSource with
44+
| Variable varDef ->
45+
yield KeyValuePair (CustomErrorFields.VariableName, varDef.Name |> box)
46+
yield KeyValuePair (CustomErrorFields.VariableType, (string varDef.TypeDef) |> box)
47+
| Argument argDef ->
48+
yield KeyValuePair (CustomErrorFields.ArgumentName, argDef.Name |> box)
49+
yield KeyValuePair (CustomErrorFields.ArgumentType, (string argDef.TypeDef) |> box)
50+
| Unknown -> ()
51+
52+
match this.FieldErrorDetails with
53+
| ValueSome details ->
54+
yield KeyValuePair (CustomErrorFields.ObjectType, (string details.ObjectDef) |> box)
55+
match details.FieldDef with
56+
| ValueSome fieldDef -> yield KeyValuePair (CustomErrorFields.FieldType, (string fieldDef.TypeDef) |> box)
57+
| ValueNone -> ()
58+
| ValueNone -> ()
59+
]
60+
|> Dictionary<_, _>
61+
:> IReadOnlyDictionary<string, obj>
62+
|> ValueSome
63+
64+
interface IInputSourceError with
65+
66+
member this.InputSource
67+
with get () = this.InputSource
68+
and set (value) = this.InputSource <- value
69+
70+
type internal CoercionErrorWrapper = {
71+
mutable InputSource : InputSource
72+
InnerError : IGQLError
73+
ErrorKind : ErrorKind
74+
Path : FieldPath
75+
FieldErrorDetails : ObjectFieldErrorDetails voption
76+
} with
77+
78+
interface IGQLError with
79+
80+
member this.Message = this.InnerError.Message
81+
82+
interface IGQLErrorExtensions with
83+
84+
member this.Extensions =
85+
86+
[
87+
yield KeyValuePair (CustomErrorFields.Kind, this.ErrorKind |> box)
88+
match this.Path with
89+
| [] -> ()
90+
| path -> yield KeyValuePair (CustomErrorFields.Path, path |> List.rev |> box)
91+
92+
match this.InputSource with
93+
| Variable varDef ->
94+
yield KeyValuePair (CustomErrorFields.VariableName, varDef.Name |> box)
95+
yield KeyValuePair (CustomErrorFields.VariableType, (string varDef.TypeDef) |> box)
96+
| Argument argDef ->
97+
yield KeyValuePair (CustomErrorFields.ArgumentName, argDef.Name |> box)
98+
yield KeyValuePair (CustomErrorFields.ArgumentType, (string argDef.TypeDef) |> box)
99+
| Unknown -> ()
100+
101+
match this.FieldErrorDetails with
102+
| ValueSome details ->
103+
yield KeyValuePair (CustomErrorFields.ObjectType, (string details.ObjectDef) |> box)
104+
match details.FieldDef with
105+
| ValueSome fieldDef -> yield KeyValuePair (CustomErrorFields.FieldType, (string fieldDef.TypeDef) |> box)
106+
| ValueNone -> ()
107+
| ValueNone -> ()
108+
109+
match this.InnerError with
110+
| :? IGQLErrorExtensions as ext ->
111+
match ext.Extensions with
112+
| ValueSome extensions -> yield! extensions
113+
| ValueNone -> ()
114+
| _ -> ()
115+
]
116+
|> Dictionary<_, _>
117+
:> IReadOnlyDictionary<string, obj>
118+
|> ValueSome
119+
120+
interface IInputSourceError with
121+
122+
member this.InputSource
123+
with get () = this.InputSource
124+
and set (value) = this.InputSource <- value
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// The MIT License (MIT)
2+
// Copyright (c) 2016 Bazinga Technologies Inc
3+
4+
namespace FSharp.Data.GraphQL
5+
6+
open System
7+
open System.Collections.Immutable
8+
9+
type InvalidInputTypeException (msg, unmatchedOptionalFields) =
10+
inherit Exception(msg)
11+
12+
member _.UnmatchedOptionalFields : string ImmutableHashSet = unmatchedOptionalFields
13+
14+
type MalformedGQLQueryException(msg) =
15+
inherit GraphQLException(msg)

src/FSharp.Data.GraphQL.Server/Execution.fs

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ open System.Text.Json
99
open FSharp.Control.Reactive
1010
open FsToolkit.ErrorHandling
1111

12-
open FSharp.Data.GraphQL
1312
open FSharp.Data.GraphQL.Ast
1413
open FSharp.Data.GraphQL.Errors
1514
open FSharp.Data.GraphQL.Extensions
1615
open FSharp.Data.GraphQL.Helpers
1716
open FSharp.Data.GraphQL.Types
1817
open FSharp.Data.GraphQL.Types.Patterns
18+
open FSharp.Data.GraphQL
1919

2020
type Output = IDictionary<string, obj>
2121

@@ -269,8 +269,9 @@ type StreamOutput =
269269
let private raiseErrors errs = AsyncVal.wrap <| Error errs
270270

271271
/// Given an error e, call ParseError in the given context's Schema to convert it into
272-
/// a list of one or more <see herf="IGQLErrors">IGQLErrors</see>, then convert those to a list of <see href="GQLProblemDetails">GQLProblemDetails</see>.
273-
let private resolverError path ctx e = ctx.Schema.ParseError path e |> List.map (GQLProblemDetails.OfFieldError (path |> List.rev))
272+
/// a list of one or more <see href="IGQLErrors">IGQLErrors</see>, then convert those
273+
/// to a list of <see href="GQLProblemDetails">GQLProblemDetails</see>.
274+
let private resolverError path ctx e = ctx.Schema.ParseError path e |> List.map (GQLProblemDetails.OfFieldExecutionError (path |> List.rev))
274275
// Helper functions for generating more specific <see href="GQLProblemDetails">GQLProblemDetails</see>.
275276
let private nullResolverError name path ctx = resolverError path ctx (GraphQLException <| sprintf "Non-Null field %s resolved as a null!" name)
276277
let private coercionError value tyName path ctx = resolverError path ctx (GraphQLException <| sprintf "Value '%O' could not be coerced to scalar %s" value tyName)
@@ -642,9 +643,9 @@ let private executeSubscription (resultSet: (string * ExecutionInfo) []) (ctx: E
642643
let private compileInputObject (indef: InputObjectDef) =
643644
indef.Fields
644645
|> Array.iter(fun inputField ->
645-
// TODO: Implement compilaiton cache to reuse for the same type
646-
let errMsg = $"Input object '%s{indef.Name}': in field '%s{inputField.Name}': "
647-
inputField.ExecuteInput <- compileByType errMsg inputField.TypeDef
646+
// TODO: Implement compilation cache to reuse for the same type
647+
let inputFieldTypeDef = inputField.TypeDef
648+
inputField.ExecuteInput <- compileByType [ box inputField.Name ] Unknown (inputFieldTypeDef, inputFieldTypeDef)
648649
match inputField.TypeDef with
649650
| InputObject inputObjDef -> inputObjDef.ExecuteInput <- inputField.ExecuteInput
650651
| _ -> ()
@@ -660,8 +661,10 @@ let private compileObject (objdef: ObjectDef) (executeFields: FieldDef -> unit)
660661
executeFields fieldDef
661662
fieldDef.Args
662663
|> Array.iter (fun arg ->
663-
let errMsg = $"Object '%s{objdef.Name}': field '%s{fieldDef.Name}': argument '%s{arg.Name}': "
664-
arg.ExecuteInput <- compileByType errMsg arg.TypeDef
664+
//let errMsg = $"Object '%s{objdef.Name}': field '%s{fieldDef.Name}': argument '%s{arg.Name}': "
665+
// TODO: Pass arg name
666+
let argTypeDef = arg.TypeDef
667+
arg.ExecuteInput <- compileByType [] (Argument arg) (argTypeDef, argTypeDef)
665668
match arg.TypeDef with
666669
| InputObject inputObjDef -> inputObjDef.ExecuteInput <- arg.ExecuteInput
667670
| _ -> ()
@@ -688,22 +691,28 @@ let internal coerceVariables (variables: VarDef list) (vars: ImmutableDictionary
688691
let variables, inlineValues, nulls =
689692
variables
690693
|> List.fold
691-
(fun (valiables, inlineValues, missing) vardef ->
692-
match vars.TryGetValue vardef.Name with
694+
(fun (valiables, inlineValues, missing) varDef ->
695+
match vars.TryGetValue varDef.Name with
693696
| false, _ ->
694-
match vardef.DefaultValue with
697+
match varDef.DefaultValue with
695698
| Some defaultValue ->
696-
let item = struct(vardef, defaultValue)
699+
let item = struct(varDef, defaultValue)
697700
(valiables, item::inlineValues, missing)
698701
| None ->
699702
let item =
700-
match vardef.TypeDef with
701-
| Nullable _ -> Ok <| KeyValuePair(vardef.Name, null)
702-
| Named typeDef -> Error [ { new IGQLError with member _.Message = $"Variable '$%s{vardef.Name}' of required type '%s{typeDef.Name}!' was not provided." } ]
703-
| _ -> System.Diagnostics.Debug.Fail $"{vardef.TypeDef.GetType().Name} is not Named"; failwith "Impossible case"
703+
match varDef.TypeDef with
704+
| Nullable _ -> Ok <| KeyValuePair(varDef.Name, null)
705+
| Named typeDef -> Error [ {
706+
Message = $"A variable '$%s{varDef.Name}' of type '%s{typeDef.Name}!' is not nullable but neither value was provided, nor a default value was specified."
707+
ErrorKind = InputCoercion
708+
InputSource = Variable varDef
709+
Path = []
710+
FieldErrorDetails = ValueNone
711+
} :> IGQLError ]
712+
| _ -> System.Diagnostics.Debug.Fail $"{varDef.TypeDef.GetType().Name} is not Named"; failwith "Impossible case"
704713
(valiables, inlineValues, item::missing)
705714
| true, jsonElement ->
706-
let item = struct(vardef, jsonElement)
715+
let item = struct(varDef, jsonElement)
707716
(item::valiables, inlineValues, missing)
708717
)
709718
([], [], [])
@@ -712,10 +721,22 @@ let internal coerceVariables (variables: VarDef list) (vars: ImmutableDictionary
712721
let! variablesBuilder =
713722
variables
714723
|> List.fold (
715-
fun (acc : Result<ImmutableDictionary<string, obj>.Builder, IGQLError list>) struct(vardef, jsonElement) -> validation {
716-
let! value = coerceVariableValue false vardef.TypeDef vardef jsonElement $"Variable '$%s{vardef.Name}': "
724+
fun (acc : Result<ImmutableDictionary<string, obj>.Builder, IGQLError list>) struct(varDef, jsonElement) -> validation {
725+
let! value =
726+
let varTypeDef = varDef.TypeDef
727+
coerceVariableValue false [] ValueNone (varTypeDef, varTypeDef) varDef jsonElement
728+
|> Result.mapError (
729+
List.map (fun err ->
730+
match err with
731+
| :? IInputSourceError as err ->
732+
match err.InputSource with
733+
| Variable _ -> ()
734+
| _ -> err.InputSource <- Variable varDef
735+
| _ -> ()
736+
err)
737+
)
717738
and! acc = acc
718-
acc.Add(vardef.Name, value)
739+
acc.Add(varDef.Name, value)
719740
return acc
720741
})
721742
(ImmutableDictionary.CreateBuilder<string, obj>() |> Ok)
@@ -727,11 +748,12 @@ let internal coerceVariables (variables: VarDef list) (vars: ImmutableDictionary
727748
let! variablesBuilder =
728749
inlineValues
729750
|> List.fold (
730-
fun (acc : Result<ImmutableDictionary<string, obj>.Builder, IGQLError list>) struct(vardef, defaultValue) -> validation {
731-
let executeInput = compileByType $"Variable '%s{vardef.Name}': " vardef.TypeDef
751+
fun (acc : Result<ImmutableDictionary<string, obj>.Builder, IGQLError list>) struct(varDef, defaultValue) -> validation {
752+
let varTypeDef = varDef.TypeDef
753+
let executeInput = compileByType [] (Variable varDef) (varTypeDef, varTypeDef)
732754
let! value = executeInput defaultValue suppliedVaribles
733755
and! acc = acc
734-
acc.Add(vardef.Name, value)
756+
acc.Add (varDef.Name, value)
735757
return acc
736758
})
737759
(variablesBuilder |> Ok)

src/FSharp.Data.GraphQL.Server/Executor.fs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
namespace FSharp.Data.GraphQL
22

33
open System.Collections.Immutable
4+
open System.Collections.Generic
5+
open System.Runtime.InteropServices
46
open System.Text.Json
57
open FsToolkit.ErrorHandling
68

@@ -10,7 +12,6 @@ open FSharp.Data.GraphQL.Ast
1012
open FSharp.Data.GraphQL.Validation
1113
open FSharp.Data.GraphQL.Parser
1214
open FSharp.Data.GraphQL.Planning
13-
open System.Runtime.InteropServices
1415

1516
/// A function signature that represents a middleware for schema compile phase.
1617
/// I takes two arguments: A schema compile context, containing all the data used for the
@@ -110,7 +111,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s
110111
let errors = System.Collections.Concurrent.ConcurrentBag<exn>()
111112
let root = data |> Option.map box |> Option.toObj
112113
match coerceVariables executionPlan.Variables variables with
113-
| Error errs -> return prepareOutput (GQLExecutionResult.Error(documentId, errs, executionPlan.Metadata))
114+
| Error errs -> return prepareOutput (GQLExecutionResult.Error (documentId, errs, executionPlan.Metadata))
114115
| Ok variables ->
115116
let executionCtx =
116117
{ Schema = schema
@@ -142,11 +143,17 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s
142143
| Mutation ->
143144
match schema.Mutation with
144145
| Some m -> Ok m
145-
| None -> Error <| [ GQLProblemDetails.Create "Operation to be executed is of type mutation, but no mutation root object was defined in current schema" ]
146+
| None -> Error <| [ GQLProblemDetails.CreateWithKind (
147+
"Operation to be executed is of type mutation, but no mutation root object was defined in current schema",
148+
ErrorKind.Validation
149+
)]
146150
| Subscription ->
147151
match schema.Subscription with
148152
| Some s -> Ok <| upcast s
149-
| None -> Error <| [ GQLProblemDetails.Create "Operation to be executed is of type subscription, but no subscription root object was defined in the current schema" ]
153+
| None -> Error <| [ GQLProblemDetails.CreateWithKind (
154+
"Operation to be executed is of type subscription, but no subscription root object was defined in the current schema",
155+
ErrorKind.Validation
156+
)]
150157
do!
151158
let schemaId = schema.Introspected.GetHashCode()
152159
let key = { DocumentId = documentId; SchemaId = schemaId }
@@ -160,7 +167,10 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s
160167
Operation = operation
161168
DocumentId = documentId }
162169
return runMiddlewares (fun x -> x.PlanOperation) planningCtx planOperation
163-
| None -> return! Error <| [ GQLProblemDetails.Create "No operation with specified name has been found for provided document" ]
170+
| None -> return! Error <| [ GQLProblemDetails.CreateWithKind (
171+
"No operation with specified name has been found for provided document",
172+
ErrorKind.Validation
173+
)]
164174
}
165175
|> Result.mapError (fun errs -> struct (documentId, errs))
166176

src/FSharp.Data.GraphQL.Server/FSharp.Data.GraphQL.Server.fsproj

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,10 @@
3333

3434
<ItemGroup>
3535
<Compile Include="ReflectionHelper.fs" />
36-
<Compile Include="Errors.fs" />
36+
<Compile Include="ErrorTypes.fs" />
37+
<Compile Include="ErrorMessages.fs" />
38+
<Compile Include="ErrorsProcessing.fs" />
39+
<Compile Include="Exceptions.fs" />
3740
<Compile Include="Values.fs" />
3841
<Compile Include="Planning.fs" />
3942
<Compile Include="ObservableExtensions.fs" />

src/FSharp.Data.GraphQL.Server/IO.fs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ type GQLResponse =
2626
Data = Skip
2727
Errors = Include errors }
2828

29-
3029
type GQLExecutionResult =
3130
{ DocumentId: int
3231
Content : GQLResponseContent

0 commit comments

Comments
 (0)