Skip to content

Ability to add errors to resolved fields #512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions src/FSharp.Data.GraphQL.Server/Execution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 (path |> List.rev))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You either do List.rev inside function OfFieldExecutionError or at least create function normalizedPath() = path |> List.rev somewhere

|> 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
Expand Down
3 changes: 3 additions & 0 deletions src/FSharp.Data.GraphQL.Server/Executor.fs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace FSharp.Data.GraphQL

open System.Collections.Concurrent
open System.Collections.Immutable
open System.Collections.Generic
open System.Runtime.InteropServices
Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand Down
23 changes: 20 additions & 3 deletions src/FSharp.Data.GraphQL.Shared/TypeSystem.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -919,9 +932,13 @@ 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)

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

Expand Down
164 changes: 155 additions & 9 deletions tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
"Success"
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", "Success"
]
]
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 = "Success" } }
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)
Expand All @@ -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
Expand Down Expand Up @@ -458,3 +477,130 @@ 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 = "Success" } }
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 = "Success" } }
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 = "Explosion" }; InnerPartialSuccess = { Kaboom = "Success" } }
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 = "Explosion" }; InnerPartialSuccess = { Kaboom = "Success" } }
sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", variables)
ensureRequestError result <| fun errors ->
result.DocumentId |> notEquals Unchecked.defaultof<int>
errors |> equals expectedErrors