From 0bd5eda775af171176c79e947db23a9147177210 Mon Sep 17 00:00:00 2001 From: VectorTetra Date: Tue, 4 Feb 2025 20:59:57 +0200 Subject: [PATCH] PR fixes --- src/FSharp.Data.GraphQL.Server/ErrorTypes.fs | 6 ++++-- src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs | 7 +++++++ src/FSharp.Data.GraphQL.Server/Execution.fs | 10 +++++----- .../FSharp.Data.GraphQL.Server.fsproj | 2 +- src/FSharp.Data.GraphQL.Shared/TypeSystem.fs | 4 ++++ tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs | 14 +++++++------- 6 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs b/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs index f4defba80..316ddb375 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, normalizedPathToObj(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, normalizedPathToObj(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 ed4ed3270..edc28b862 100644 --- a/src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs +++ b/src/FSharp.Data.GraphQL.Server/ErrorsProcessing.fs @@ -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 + diff --git a/src/FSharp.Data.GraphQL.Server/Execution.fs b/src/FSharp.Data.GraphQL.Server/Execution.fs index bb1524533..ced70583f 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 = normalizedPath 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 (normalizedPath 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 = normalizedPath path match res with | Ok (data, deferred, errs) -> let deferredData = @@ -368,7 +368,7 @@ and private executeResolvers (ctx : ResolveFieldContext) (path : FieldPath) (par match ctx.Context.Errors.TryGetValue ctx with | true, errors -> errors - |> Seq.map (GQLProblemDetails.OfFieldExecutionError (path |> List.rev)) + |> Seq.map (GQLProblemDetails.OfFieldExecutionError (normalizedPath path)) |> Seq.toList | false, _ -> [] match resolved with @@ -462,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 = 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 3ad275cc7..e25760407 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 e632db87e..4ac6bb685 100644 --- a/src/FSharp.Data.GraphQL.Shared/TypeSystem.fs +++ b/src/FSharp.Data.GraphQL.Shared/TypeSystem.fs @@ -936,6 +936,10 @@ and ResolveFieldContext = { 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 this.TryArg (name : string) : 't option = match Map.tryFind name this.Args with diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs index 86b867736..221539cc1 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs @@ -410,7 +410,7 @@ let ``Execution handles errors: properly propagates errors`` () = // executeResolvers/resolveWith, case 5 let resolvePartialSuccess (ctx : ResolveFieldContext) (_ : InnerNullableTest) = ctx.AddError { new IGQLError with member _.Message = "Some non-critical error" } - "Success" + "Yes, Rico, Kaboom" Define.Object( "InnerPartialSuccess", [ Define.Field("kaboom", StringType, resolvePartialSuccess) @@ -425,7 +425,7 @@ let ``Execution handles errors: properly propagates errors`` () = NameValueLookup.ofList [ "inner", null "partialSuccess", NameValueLookup.ofList [ - "kaboom", "Success" + "kaboom", "Yes, Rico, Kaboom" ] ] let expectedErrors = [ @@ -433,7 +433,7 @@ let ``Execution handles errors: properly propagates errors`` () = GQLProblemDetails.CreateWithKind ("Some non-critical error", Execution, [ box "partialSuccess"; "kaboom" ]) ] let result = - let variables = { Inner = { Kaboom = null }; InnerPartialSuccess = { Kaboom = "Success" } } + 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 @@ -507,7 +507,7 @@ let ``Execution handles errors: additional error added when exception is rised i GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) ] let result = - let variables = { Inner = { Kaboom = null }; InnerPartialSuccess = { Kaboom = "Success" } } + 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 @@ -541,7 +541,7 @@ let ``Execution handles errors: additional error added when None returned from a GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) ] let result = - let variables = { Inner = { Kaboom = null }; InnerPartialSuccess = { Kaboom = "Success" } } + 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 @@ -571,7 +571,7 @@ let ``Execution handles errors: additional error added when exception is rised i GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) ] let result = - let variables = { Inner = { Kaboom = "Explosion" }; InnerPartialSuccess = { Kaboom = "Success" } } + 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 @@ -599,7 +599,7 @@ let ``Execution handles errors: additional error added and when null returned fr GQLProblemDetails.CreateWithKind ("Non-critical error", Execution, [ box "inner"; "kaboom" ]) ] let result = - let variables = { Inner = { Kaboom = "Explosion" }; InnerPartialSuccess = { Kaboom = "Success" } } + 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