diff --git a/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs b/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs index f4defba8..eaa7778c 100644 --- a/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs +++ b/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs @@ -4,8 +4,10 @@ namespace FSharp.Data.GraphQL open System open System.Collections.Generic open FsToolkit.ErrorHandling +open FSharp.Data.GraphQL.Errors open FSharp.Data.GraphQL.Types + type InputSource = | Variable of VarDef : VarDef | Argument of ArgDef : InputFieldDef @@ -38,7 +40,7 @@ type internal CoercionError = { yield KeyValuePair (CustomErrorFields.Kind, this.ErrorKind |> box) match this.Path with | [] -> () - | path -> yield KeyValuePair (CustomErrorFields.Path, path |> List.rev |> box) + | path -> yield KeyValuePair (CustomErrorFields.Path, normalizeErrorPath path) match this.InputSource with | Variable varDef -> @@ -87,7 +89,7 @@ type internal CoercionErrorWrapper = { yield KeyValuePair (CustomErrorFields.Kind, this.ErrorKind |> box) match this.Path with | [] -> () - | path -> yield KeyValuePair (CustomErrorFields.Path, path |> List.rev |> box) + | path -> yield KeyValuePair (CustomErrorFields.Path, normalizeErrorPath path) match this.InputSource with | Variable varDef -> diff --git a/src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs b/src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs index ed4ed327..6dbe54f7 100644 --- a/src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs +++ b/src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs @@ -114,3 +114,6 @@ let splitSeqErrorsList (items: Result<'t, IGQLError list> list) = else let values = items |> getSeqValuesList Ok values + +let internal normalizeErrorPath(fieldPath : FieldPath) = + fieldPath |> List.rev diff --git a/src/FSharp.Data.GraphQL.Server/Execution.fs b/src/FSharp.Data.GraphQL.Server/Execution.fs index 0c472d40..17e23c13 100644 --- a/src/FSharp.Data.GraphQL.Server/Execution.fs +++ b/src/FSharp.Data.GraphQL.Server/Execution.fs @@ -101,7 +101,7 @@ let private createFieldContext objdef argDefs ctx (info: ExecutionInfo) (path : Schema = ctx.Schema Args = args Variables = ctx.Variables - Path = path |> List.rev } + Path = normalizeErrorPath path } } let private resolveField (execute: ExecuteField) (ctx: ResolveFieldContext) (parentValue: obj) = @@ -136,7 +136,7 @@ let private raiseErrors errs = AsyncVal.wrap <| Error errs /// Given an error e, call ParseError in the given context's Schema to convert it into /// a list of one or more IGQLErrors, then convert those /// to a list of GQLProblemDetails. -let private resolverError path ctx e = ctx.Schema.ParseError path e |> List.map (GQLProblemDetails.OfFieldExecutionError (path |> List.rev)) +let private resolverError path ctx e = ctx.Schema.ParseError path e |> List.map (GQLProblemDetails.OfFieldExecutionError (normalizeErrorPath path)) // Helper functions for generating more specific GQLProblemDetails. let private nullResolverError name path ctx = resolverError path ctx (GQLMessageException <| sprintf "Non-Null field %s resolved as a null!" name) let private coercionError value tyName path ctx = resolverError path ctx (GQLMessageException <| sprintf "Value '%O' could not be coerced to scalar %s" value tyName) @@ -148,7 +148,7 @@ let private streamListError name tyName path ctx = resolverError path ctx (GQLMe let private resolved name v : AsyncVal>> = KeyValuePair(name, box v) |> ResolverResult.data |> AsyncVal.wrap let deferResults path (res : ResolverResult) : IObservable = - let formattedPath = path |> List.rev + let formattedPath = normalizeErrorPath path match res with | Ok (data, deferred, errs) -> let deferredData = @@ -364,12 +364,22 @@ and private executeResolvers (ctx : ResolveFieldContext) (path : FieldPath) (par /// This handles all null resolver errors/error propagation. let resolveWith (ctx : ResolveFieldContext) (onSuccess : ResolveFieldContext -> FieldPath -> obj -> obj -> AsyncVal>>) : AsyncVal>> = asyncVal { let! resolved = value |> AsyncVal.rescue path ctx.Schema.ParseError + let additionalErrs = + match ctx.Context.Errors.TryGetValue ctx with + | true, errors -> + errors + |> Seq.map (GQLProblemDetails.OfFieldExecutionError (normalizeErrorPath path)) + |> Seq.toList + | false, _ -> [] match resolved with - | Error errs when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, errs) - | Ok None when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, []) - | Error errs -> return Error errs - | Ok None -> return Error (nullResolverError name path ctx) - | Ok (Some v) -> return! onSuccess ctx path parent v + | Error errs when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, errs @ additionalErrs) + | Ok None when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, additionalErrs) + | Error errs -> return Error (errs @ additionalErrs) + | Ok None -> return Error ((nullResolverError name path ctx) @ additionalErrs) + | Ok (Some v) -> + match! onSuccess ctx path parent v with + | Ok (res, deferred, errs) -> return Ok (res, deferred, errs @ additionalErrs) + | Error errs -> return Error (errs @ additionalErrs) } match info.Kind, returnDef with @@ -452,7 +462,7 @@ let private executeQueryOrMutation (resultSet: (string * ExecutionInfo) []) (ctx Schema = ctx.Schema Args = args Variables = ctx.Variables - Path = path |> List.rev } + Path = normalizeErrorPath path } let execute = ctx.FieldExecuteMap.GetExecute(ctx.ExecutionPlan.RootDef.Name, info.Definition.Name) asyncVal { let! result = diff --git a/src/FSharp.Data.GraphQL.Server/Executor.fs b/src/FSharp.Data.GraphQL.Server/Executor.fs index ff36a50b..b332868b 100644 --- a/src/FSharp.Data.GraphQL.Server/Executor.fs +++ b/src/FSharp.Data.GraphQL.Server/Executor.fs @@ -1,5 +1,6 @@ namespace FSharp.Data.GraphQL +open System.Collections.Concurrent open System.Collections.Immutable open System.Collections.Generic open System.Runtime.InteropServices @@ -108,6 +109,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s | Stream (stream) -> GQLExecutionResult.Stream (documentId, stream, res.Metadata) async { try + let errors = ConcurrentDictionary>() let root = data |> Option.map box |> Option.toObj match coerceVariables executionPlan.Variables variables with | Error errs -> return prepareOutput (GQLExecutionResult.Error (documentId, errs, executionPlan.Metadata)) @@ -117,6 +119,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s RootValue = root ExecutionPlan = executionPlan Variables = variables + Errors = errors FieldExecuteMap = fieldExecuteMap Metadata = executionPlan.Metadata } let! res = runMiddlewares (fun x -> x.ExecuteOperationAsync) executionCtx executeOperation |> AsyncVal.toAsync diff --git a/src/FSharp.Data.GraphQL.Server/FSharp.Data.GraphQL.Server.fsproj b/src/FSharp.Data.GraphQL.Server/FSharp.Data.GraphQL.Server.fsproj index 3ad275cc..e2576040 100644 --- a/src/FSharp.Data.GraphQL.Server/FSharp.Data.GraphQL.Server.fsproj +++ b/src/FSharp.Data.GraphQL.Server/FSharp.Data.GraphQL.Server.fsproj @@ -36,9 +36,9 @@ + - diff --git a/src/FSharp.Data.GraphQL.Shared/TypeSystem.fs b/src/FSharp.Data.GraphQL.Shared/TypeSystem.fs index 4a5ac580..502cceca 100644 --- a/src/FSharp.Data.GraphQL.Shared/TypeSystem.fs +++ b/src/FSharp.Data.GraphQL.Shared/TypeSystem.fs @@ -5,9 +5,11 @@ namespace FSharp.Data.GraphQL.Types open System open System.Reflection open System.Collections +open System.Collections.Concurrent open System.Collections.Generic open System.Collections.Immutable open System.Text.Json + open FSharp.Data.GraphQL open FSharp.Data.GraphQL.Ast open FSharp.Data.GraphQL.Extensions @@ -890,11 +892,22 @@ and ExecutionContext = { ExecutionPlan : ExecutionPlan /// Collection of variables provided to execute current operation. Variables : ImmutableDictionary + /// Collection of errors that occurred while executing current operation. + Errors : ConcurrentDictionary> /// A map of all fields of the query and their respective execution operations. FieldExecuteMap : FieldExecuteMap /// A simple dictionary to hold metadata that can be used by execution customizations. Metadata : Metadata -} +} with + + /// Remembers an error, so it can be included in the final response. + member this.AddError (fieldContext, error : IGQLError) : unit = + this.Errors.AddOrUpdate( + fieldContext, + addValueFactory = (fun _ -> ConcurrentBag (Seq.singleton error)), + updateValueFactory = (fun _ bag -> bag.Add error; bag) + ) + |> ignore /// An execution context for the particular field, applied as the first /// parameter for target resolve function. @@ -919,9 +932,17 @@ and ResolveFieldContext = { Path : FieldPath } with + /// Remembers an error, so it can be included in the final response. + member this.AddError (error : IGQLError) = + this.Context.AddError (this, error) + + /// Remembers an error, so it can be included in the final response. + member this.AddError (errorMessage : string) = + this.Context.AddError (this, { new IGQLError with member _.Message = errorMessage } ) + /// Tries to find an argument by provided name. - member x.TryArg (name : string) : 't option = - match Map.tryFind name x.Args with + member this.TryArg (name : string) : 't option = + match Map.tryFind name this.Args with | Some o -> Some (o :?> 't) // TODO: Use Convert.ChangeType | None -> None diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs index 720555f3..244bef54 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs @@ -394,28 +394,47 @@ let ``Execution when querying returns unique document id with response`` () = | response -> fail $"Expected a 'Direct' GQLResponse but got\n{response}" type InnerNullableTest = { Kaboom : string } -type NullableTest = { Inner : InnerNullableTest } +type NullableTest = { + Inner : InnerNullableTest + InnerPartialSuccess : InnerNullableTest +} [] let ``Execution handles errors: properly propagates errors`` () = - let InnerObj = + let InnerObjType = Define.Object( "Inner", [ Define.Field("kaboom", StringType, fun _ x -> x.Kaboom) ]) + let InnerPartialSuccessObjType = + // executeResolvers/resolveWith, case 5 + let resolvePartialSuccess (ctx : ResolveFieldContext) (_ : InnerNullableTest) = + ctx.AddError { new IGQLError with member _.Message = "Some non-critical error" } + "Yes, Rico, Kaboom" + Define.Object( + "InnerPartialSuccess", [ + Define.Field("kaboom", StringType, resolvePartialSuccess) + ]) let schema = Schema(Define.Object( - "Type", [ - Define.Field("inner", Nullable InnerObj, fun _ x -> Some x.Inner) - ])) + "Type", [ + Define.Field("inner", Nullable InnerObjType, fun _ x -> Some x.Inner) + Define.Field("partialSuccess", Nullable InnerPartialSuccessObjType, fun _ x -> Some x.InnerPartialSuccess) + ])) let expectedData = NameValueLookup.ofList [ "inner", null + "partialSuccess", NameValueLookup.ofList [ + "kaboom", "Yes, Rico, Kaboom" + ] ] let expectedErrors = [ GQLProblemDetails.CreateWithKind ("Non-Null field kaboom resolved as a null!", Execution, [ box "inner"; "kaboom" ]) + GQLProblemDetails.CreateWithKind ("Some non-critical error", Execution, [ box "partialSuccess"; "kaboom" ]) ] - let result = sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", { Inner = { Kaboom = null } }) + let result = + let variables = { Inner = { Kaboom = null }; InnerPartialSuccess = { Kaboom = "Yes, Rico, Kaboom" } } + sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } partialSuccess { kaboom } }", variables) ensureDirect result <| fun data errors -> result.DocumentId |> notEquals Unchecked.defaultof data |> equals (upcast expectedData) @@ -425,9 +444,9 @@ let ``Execution handles errors: properly propagates errors`` () = let ``Execution handles errors: exceptions`` () = let schema = Schema(Define.Object( - "Type", [ - Define.Field("a", StringType, fun _ _ -> failwith "Resolver Error!") - ])) + "Type", [ + Define.Field("a", StringType, fun _ _ -> failwith "Resolver Error!") + ])) let expectedError = GQLProblemDetails.CreateWithKind ("Resolver Error!", Execution, [ box "a" ]) let result = sync <| Executor(schema).AsyncExecute("query Test { a }", ()) ensureRequestError result <| fun [ error ] -> error |> equals expectedError @@ -458,3 +477,129 @@ let ``Execution handles errors: nullable list fields`` () = result.DocumentId |> notEquals Unchecked.defaultof data |> equals (upcast expectedData) errors |> equals expectedErrors + + +[] +let ``Execution handles errors: additional error added when exception is rised in a nullable field resolver`` () = + let InnerNullableExceptionObjType = + // executeResolvers/resolveWith, case 1 + let resolveWithException (ctx : ResolveFieldContext) (_ : InnerNullableTest) : string option = + ctx.AddError { new IGQLError with member _.Message = "Non-critical error" } + raise (System.Exception "Unexpected error") + Define.Object( + "InnerNullableException", [ + Define.Field("kaboom", Nullable StringType, resolve = resolveWithException) + ]) + let schema = + Schema(Define.Object( + "Type", [ + Define.Field("inner", Nullable InnerNullableExceptionObjType, fun _ x -> Some x.Inner) + ])) + let expectedData = + NameValueLookup.ofList [ + "inner", NameValueLookup.ofList [ + "kaboom", null + ] + ] + let expectedErrors = + [ + GQLProblemDetails.CreateWithKind ("Unexpected error", Execution, [ box "inner"; "kaboom" ]) + GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) + ] + let result = + let variables = { Inner = { Kaboom = null }; InnerPartialSuccess = { Kaboom = "Yes, Rico, Kaboom" } } + sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", variables) + ensureDirect result <| fun data errors -> + result.DocumentId |> notEquals Unchecked.defaultof + data |> equals (upcast expectedData) + errors |> equals expectedErrors + +[] +let ``Execution handles errors: additional error added when None returned from a nullable field resolver`` () = + let InnerNullableNoneObjType = + // executeResolvers/resolveWith, case 2 + let resolveWithNone (ctx : ResolveFieldContext) (_ : InnerNullableTest) : string option = + ctx.AddError { new IGQLError with member _.Message = "Non-critical error" } + None + Define.Object( + "InnerNullableException", [ + Define.Field("kaboom", Nullable StringType, resolve = resolveWithNone) + ]) + let schema = + Schema(Define.Object( + "Type", [ + Define.Field("inner", Nullable InnerNullableNoneObjType, fun _ x -> Some x.Inner) + ])) + let expectedData = + NameValueLookup.ofList [ + "inner", NameValueLookup.ofList [ + "kaboom", null + ] + ] + let expectedErrors = + [ + GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) + ] + let result = + let variables = { Inner = { Kaboom = null }; InnerPartialSuccess = { Kaboom = "Yes, Rico, Kaboom" } } + sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", variables) + ensureDirect result <| fun data errors -> + result.DocumentId |> notEquals Unchecked.defaultof + data |> equals (upcast expectedData) + errors |> equals expectedErrors + +[] +let ``Execution handles errors: additional error added when exception is rised in a non-nullable field resolver`` () = + let InnerNonNullableExceptionObjType = + // executeResolvers/resolveWith, case 3 + let resolveWithException (ctx : ResolveFieldContext) (_ : InnerNullableTest) : string = + ctx.AddError { new IGQLError with member _.Message = "Non-critical error" } + raise (System.Exception "Fatal error") + Define.Object( + "InnerNonNullableException", [ + Define.Field("kaboom", StringType, resolve = resolveWithException) + ]) + let schema = + Schema(Define.Object( + "Type", [ + Define.Field("inner", InnerNonNullableExceptionObjType, fun _ x -> x.Inner) + ])) + let expectedErrors = + [ + GQLProblemDetails.CreateWithKind ("Fatal error", Execution, [ box "inner"; "kaboom" ]) + GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) + ] + let result = + let variables = { Inner = { Kaboom = "Yes, Rico, Kaboom" }; InnerPartialSuccess = { Kaboom = "Yes, Rico, Kaboom" } } + sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", variables) + ensureRequestError result <| fun errors -> + result.DocumentId |> notEquals Unchecked.defaultof + errors |> equals expectedErrors + +[] +let ``Execution handles errors: additional error added and when null returned from a non-nullable field resolver`` () = + let InnerNonNullableNullObjType = + // executeResolvers/resolveWith, case 4 + let resolveWithNull (ctx : ResolveFieldContext) (_ : InnerNullableTest) : string = + ctx.AddError { new IGQLError with member _.Message = "Non-critical error" } + null + Define.Object( + "InnerNonNullableNull", [ + Define.Field("kaboom", StringType, resolveWithNull) + ]) + let schema = + Schema(Define.Object( + "Type", [ + Define.Field("inner", InnerNonNullableNullObjType, fun _ x -> x.Inner) + ])) + let expectedErrors = + [ + GQLProblemDetails.CreateWithKind ("Non-Null field kaboom resolved as a null!", Execution, [ box "inner"; "kaboom" ]) + GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) + ] + let result = + let variables = { Inner = { Kaboom = "Yes, Rico, Kaboom" }; InnerPartialSuccess = { Kaboom = "Yes, Rico, Kaboom" } } + sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", variables) + ensureRequestError result <| fun errors -> + result.DocumentId |> notEquals Unchecked.defaultof + errors |> equals expectedErrors