Ability to add errors to resolved fields (#512)

* Implemented the ability to add additional errors to resolved fields
* Added tests to cover all cases of additional errors in `executeResolvers`/`resolveWith`

Author: VectorTetra

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 ->

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

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 <see href="IGQLErrors">IGQLErrors</see>, then convert those
 /// to a list of <see href="GQLProblemDetails">GQLProblemDetails</see>.
-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 <see href="GQLProblemDetails">GQLProblemDetails</see>.
 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<ResolverResult<KeyValuePair<string, obj>>> = KeyValuePair(name, box v) |> ResolverResult.data |> AsyncVal.wrap
 
 let deferResults path (res : ResolverResult<obj>) : IObservable<GQLDeferredResponseContent> =
-    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<ResolverResult<KeyValuePair<string, obj>>>) : AsyncVal<ResolverResult<KeyValuePair<string, obj>>> = 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 =

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<ResolveFieldContext, ConcurrentBag<IGQLError>>()
                 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

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

@@ -36,9 +36,9 @@
   <ItemGroup>
     <Compile Include="Ast.fs" />
     <Compile Include="ReflectionHelper.fs" />
+    <Compile Include="ErrorsProcessing.fs" />
     <Compile Include="ErrorTypes.fs" />
     <Compile Include="ErrorMessages.fs" />
-    <Compile Include="ErrorsProcessing.fs" />
     <Compile Include="Exceptions.fs" />
     <Compile Include="TypeSystem.fs" />
     <Compile Include="Values.fs" />

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<string, obj>
+    /// Collection of errors that occurred while executing current operation.
+    Errors : ConcurrentDictionary<ResolveFieldContext, ConcurrentBag<IGQLError>>
     /// 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
 

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
+}
 
 [<Fact>]
 let ``Execution handles errors: properly propagates errors`` () =
-    let InnerObj =
+    let InnerObjType =
         Define.Object<InnerNullableTest>(
             "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<InnerNullableTest>(
+            "InnerPartialSuccess", [
+                Define.Field("kaboom", StringType, resolvePartialSuccess)
+            ])
     let schema =
         Schema(Define.Object<NullableTest>(
-                 "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<int>
         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<unit>(
-                 "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<int>
         data |> equals (upcast expectedData)
         errors |> equals expectedErrors
+
+
+[<Fact>]
+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<InnerNullableTest>(
+            "InnerNullableException", [
+                Define.Field("kaboom", Nullable StringType, resolve = resolveWithException)
+            ])
+    let schema =
+        Schema(Define.Object<NullableTest>(
+            "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<int>
+        data |> equals (upcast expectedData)
+        errors |> equals expectedErrors
+
+[<Fact>]
+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<InnerNullableTest>(
+            "InnerNullableException", [
+                Define.Field("kaboom", Nullable StringType, resolve = resolveWithNone)
+            ])
+    let schema =
+        Schema(Define.Object<NullableTest>(
+            "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<int>
+        data |> equals (upcast expectedData)
+        errors |> equals expectedErrors
+
+[<Fact>]
+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<InnerNullableTest>(
+            "InnerNonNullableException", [
+                Define.Field("kaboom", StringType, resolve = resolveWithException)
+            ])
+    let schema =
+        Schema(Define.Object<NullableTest>(
+            "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<int>
+        errors |> equals expectedErrors
+
+[<Fact>]
+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<InnerNullableTest>(
+            "InnerNonNullableNull", [
+                Define.Field("kaboom", StringType, resolveWithNull)
+            ])
+    let schema =
+        Schema(Define.Object<NullableTest>(
+            "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<int>
+        errors |> equals expectedErrors