Skip to content

Commit f0feede

Browse files
committed
Implemented ability to add additional errors to resolved fields
Covered test case: Add additional error and return value from a non-nullable GraphQL field resolver
1 parent 717dafd commit f0feede

File tree

4 files changed

+73
-16
lines changed

4 files changed

+73
-16
lines changed

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

+15-4
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,23 @@ and private executeResolvers (ctx : ResolveFieldContext) (path : FieldPath) (par
364364
/// This handles all null resolver errors/error propagation.
365365
let resolveWith (ctx : ResolveFieldContext) (onSuccess : ResolveFieldContext -> FieldPath -> obj -> obj -> AsyncVal<ResolverResult<KeyValuePair<string, obj>>>) : AsyncVal<ResolverResult<KeyValuePair<string, obj>>> = asyncVal {
366366
let! resolved = value |> AsyncVal.rescue path ctx.Schema.ParseError
367+
let additionalErrs =
368+
match ctx.Context.Errors.TryGetValue ctx with
369+
| true, errors ->
370+
errors
371+
|> Seq.map (GQLProblemDetails.OfFieldExecutionError (path |> List.rev))
372+
|> Seq.toList
373+
| false, _ -> []
367374
match resolved with
368-
| Error errs when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, errs)
369-
| Ok None when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, [])
370-
| Error errs -> return Error errs
375+
| Error errs when ctx.ExecutionInfo.IsNullable -> return Ok (KeyValuePair(name, null), None, errs @ additionalErrs)
376+
| Ok None when ctx.ExecutionInfo.IsNullable ->
377+
return Ok (KeyValuePair(name, null), None, additionalErrs)
378+
| Error errs -> return Error (errs @ additionalErrs)
371379
| Ok None -> return Error (nullResolverError name path ctx)
372-
| Ok (Some v) -> return! onSuccess ctx path parent v
380+
| Ok (Some v) ->
381+
match! onSuccess ctx path parent v with
382+
| Ok (res, deferred, errs) -> return Ok (res, deferred, errs @ additionalErrs)
383+
| Error errs -> return Error (errs @ additionalErrs)
373384
}
374385

375386
match info.Kind, returnDef with

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

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
namespace FSharp.Data.GraphQL
22

3+
open System.Collections.Concurrent
34
open System.Collections.Immutable
45
open System.Collections.Generic
56
open System.Runtime.InteropServices
@@ -108,6 +109,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s
108109
| Stream (stream) -> GQLExecutionResult.Stream (documentId, stream, res.Metadata)
109110
async {
110111
try
112+
let errors = ConcurrentDictionary<ResolveFieldContext, ConcurrentBag<IGQLError>>()
111113
let root = data |> Option.map box |> Option.toObj
112114
match coerceVariables executionPlan.Variables variables with
113115
| Error errs -> return prepareOutput (GQLExecutionResult.Error (documentId, errs, executionPlan.Metadata))
@@ -117,6 +119,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s
117119
RootValue = root
118120
ExecutionPlan = executionPlan
119121
Variables = variables
122+
Errors = errors
120123
FieldExecuteMap = fieldExecuteMap
121124
Metadata = executionPlan.Metadata }
122125
let! res = runMiddlewares (fun x -> x.ExecuteOperationAsync) executionCtx executeOperation |> AsyncVal.toAsync

src/FSharp.Data.GraphQL.Shared/TypeSystem.fs

+20-3
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ namespace FSharp.Data.GraphQL.Types
55
open System
66
open System.Reflection
77
open System.Collections
8+
open System.Collections.Concurrent
89
open System.Collections.Generic
910
open System.Collections.Immutable
1011
open System.Text.Json
12+
1113
open FSharp.Data.GraphQL
1214
open FSharp.Data.GraphQL.Ast
1315
open FSharp.Data.GraphQL.Extensions
@@ -890,11 +892,22 @@ and ExecutionContext = {
890892
ExecutionPlan : ExecutionPlan
891893
/// Collection of variables provided to execute current operation.
892894
Variables : ImmutableDictionary<string, obj>
895+
/// Collection of errors that occurred while executing current operation.
896+
Errors : ConcurrentDictionary<ResolveFieldContext, ConcurrentBag<IGQLError>>
893897
/// A map of all fields of the query and their respective execution operations.
894898
FieldExecuteMap : FieldExecuteMap
895899
/// A simple dictionary to hold metadata that can be used by execution customizations.
896900
Metadata : Metadata
897-
}
901+
} with
902+
903+
/// Remembers an error, so it can be included in the final response.
904+
member this.AddError (fieldContext, error : IGQLError) : unit =
905+
this.Errors.AddOrUpdate(
906+
fieldContext,
907+
addValueFactory = (fun _ -> ConcurrentBag (Seq.singleton error)),
908+
updateValueFactory = (fun _ (bag)-> bag.Add error; bag)
909+
)
910+
|> ignore
898911

899912
/// An execution context for the particular field, applied as the first
900913
/// parameter for target resolve function.
@@ -919,9 +932,13 @@ and ResolveFieldContext = {
919932
Path : FieldPath
920933
} with
921934

935+
/// Remembers an error, so it can be included in the final response.
936+
member this.AddError (error : IGQLError) =
937+
this.Context.AddError (this, error)
938+
922939
/// Tries to find an argument by provided name.
923-
member x.TryArg (name : string) : 't option =
924-
match Map.tryFind name x.Args with
940+
member this.TryArg (name : string) : 't option =
941+
match Map.tryFind name this.Args with
925942
| Some o -> Some (o :?> 't) // TODO: Use Convert.ChangeType
926943
| None -> None
927944

tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs

+35-9
Original file line numberDiff line numberDiff line change
@@ -394,40 +394,66 @@ let ``Execution when querying returns unique document id with response`` () =
394394
| response -> fail $"Expected a 'Direct' GQLResponse but got\n{response}"
395395

396396
type InnerNullableTest = { Kaboom : string }
397-
type NullableTest = { Inner : InnerNullableTest }
397+
type NullableTest = {
398+
Inner : InnerNullableTest
399+
InnerPartialSuccess : InnerNullableTest
400+
}
398401

399402
[<Fact>]
400403
let ``Execution handles errors: properly propagates errors`` () =
401-
let InnerObj =
404+
let InnerObjType =
402405
Define.Object<InnerNullableTest>(
403406
"Inner", [
404407
Define.Field("kaboom", StringType, fun _ x -> x.Kaboom)
405408
])
409+
let InnerPartialSuccessObjType =
410+
let resolvePartialSuccess (ctx : ResolveFieldContext) (_ : InnerNullableTest) =
411+
ctx.AddError { new IGQLError with member _.Message = "Some non-critical error" }
412+
"Success"
413+
Define.Object<InnerNullableTest>(
414+
"InnerPartialSuccess", [
415+
Define.Field("kaboom", StringType, resolvePartialSuccess)
416+
])
406417
let schema =
407418
Schema(Define.Object<NullableTest>(
408-
"Type", [
409-
Define.Field("inner", Nullable InnerObj, fun _ x -> Some x.Inner)
410-
]))
419+
"Type", [
420+
Define.Field("inner", Nullable InnerObjType, fun _ x -> Some x.Inner)
421+
Define.Field("partialSuccess", Nullable InnerPartialSuccessObjType, fun _ x -> Some x.InnerPartialSuccess)
422+
]))
411423
let expectedData =
412424
NameValueLookup.ofList [
413425
"inner", null
426+
"partialSuccess", NameValueLookup.ofList [
427+
"kaboom", "Success"
428+
]
414429
]
415430
let expectedErrors = [
416431
GQLProblemDetails.CreateWithKind ("Non-Null field kaboom resolved as a null!", Execution, [ box "inner"; "kaboom" ])
432+
GQLProblemDetails.CreateWithKind ("Some non-critical error", Execution, [ box "partialSuccess"; "kaboom" ])
417433
]
418-
let result = sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", { Inner = { Kaboom = null } })
434+
let result =
435+
let variables = { Inner = { Kaboom = null }; InnerPartialSuccess = { Kaboom = "Success" } }
436+
sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } partialSuccess { kaboom } }", variables)
419437
ensureDirect result <| fun data errors ->
420438
result.DocumentId |> notEquals Unchecked.defaultof<int>
421439
data |> equals (upcast expectedData)
422440
errors |> equals expectedErrors
423441

442+
// TODO: Add other tests with possible error cases
443+
// 1. Add additional error and raise an exception in a nullable GraphQL field resolver
444+
// 2. Add additional error and return None from a nullable GraphQL field resolver
445+
// 3. Add additional error and raise an exception in a non-nullable GraphQL field resolver
446+
// 4. Add additional error and return null from a non-nullable GraphQL field resolver
447+
// Covered // 5.1. Add additional error and return value from a non-nullable GraphQL field resolver
448+
// 5.2. Add additional error and raise an exception in a non-nullable GraphQL field resolver
449+
424450
[<Fact>]
425451
let ``Execution handles errors: exceptions`` () =
426452
let schema =
427453
Schema(Define.Object<unit>(
428-
"Type", [
429-
Define.Field("a", StringType, fun _ _ -> failwith "Resolver Error!")
430-
]))
454+
"Type", [
455+
Define.Field("a", StringType, fun _ _ -> failwith "Resolver Error!")
456+
]))
431457
let expectedError = GQLProblemDetails.CreateWithKind ("Resolver Error!", Execution, [ box "a" ])
432458
let result = sync <| Executor(schema).AsyncExecute("query Test { a }", ())
433459
ensureRequestError result <| fun [ error ] -> error |> equals expectedError

0 commit comments

Comments
 (0)