Skip to content
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

Ability to add errors to resolved fields #512

Merged
Merged
Show file tree
Hide file tree
Changes from 3 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
6 changes: 4 additions & 2 deletions src/FSharp.Data.GraphQL.Server/ErrorTypes.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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, normalizedPathToObj(path))
VectorTetra marked this conversation as resolved.
Show resolved Hide resolved

match this.InputSource with
| Variable varDef ->
Expand Down Expand Up @@ -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, normalizedPathToObj(path))
VectorTetra marked this conversation as resolved.
Show resolved Hide resolved

match this.InputSource with
| Variable varDef ->
Expand Down
7 changes: 7 additions & 0 deletions src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,10 @@ let splitSeqErrorsList (items: Result<'t, IGQLError list> list) =
else
let values = items |> getSeqValuesList
Ok values

let internal normalizedPath(fieldPath : FieldPath) =
fieldPath |> List.rev

let internal normalizedPathToObj(fieldPath : FieldPath) =
fieldPath |> List.rev |> box

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let internal normalizedPath(fieldPath : FieldPath) =
fieldPath |> List.rev
let internal normalizedPathToObj(fieldPath : FieldPath) =
fieldPath |> List.rev |> box
let internal normalizeErrorPath (fieldPath : FieldPath) =
fieldPath |> List.rev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I remove normalizedPathToObj function?

28 changes: 19 additions & 9 deletions src/FSharp.Data.GraphQL.Server/Execution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = normalizedPath path }
}

let private resolveField (execute: ExecuteField) (ctx: ResolveFieldContext) (parentValue: obj) =
Expand Down Expand Up @@ -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 (normalizedPath 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)
Expand All @@ -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 = normalizedPath path
match res with
| Ok (data, deferred, errs) ->
let deferredData =
Expand Down 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 (normalizedPath 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
Expand Down Expand Up @@ -452,7 +462,7 @@ let private executeQueryOrMutation (resultSet: (string * ExecutionInfo) []) (ctx
Schema = ctx.Schema
Args = args
Variables = ctx.Variables
Path = path |> List.rev }
Path = normalizedPath path }
let execute = ctx.FieldExecuteMap.GetExecute(ctx.ExecutionPlan.RootDef.Name, info.Definition.Name)
asyncVal {
let! result =
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
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
27 changes: 24 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)
VectorTetra marked this conversation as resolved.
Show resolved Hide resolved
)
|> ignore

/// An execution context for the particular field, applied as the first
/// parameter for target resolve function.
Expand All @@ -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) =
xperiandri marked this conversation as resolved.
Show resolved Hide resolved
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

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" }
"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)
xperiandri marked this conversation as resolved.
Show resolved Hide resolved
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 = "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)
]))

VectorTetra marked this conversation as resolved.
Show resolved Hide resolved
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