From e829181c9a0b7948529796f0142007dac53b2851 Mon Sep 17 00:00:00 2001 From: valber Date: Fri, 5 Apr 2024 18:03:03 +0200 Subject: [PATCH 01/17] .AspNetCore: BREAKING CHANGE: logging server exceptions with exception info too There were two problems before: 1. The error message itself was not being logged, but only a mention that there was an error; 2. It was not possible to access the possibly original exception which led to the request processing error. However, it's extremely useful to log the error with the whole exception information including the stack trace of where the exception was thrown. However, these changes also introduce a breaking change as IGQLError and GQLProblemDetails now each have an additional mandatory member holding the possible exception which generated the error. These types (especiall GQLProblemDetails) are public and could be used by code using FSharp.Data.GraphQL. --- .../Giraffe/HttpHandlers.fs | 22 +++++++--- .../MiddlewareDefinitions.fs | 2 +- .../SchemaDefinitions.fs | 4 +- src/FSharp.Data.GraphQL.Server/ErrorTypes.fs | 2 + src/FSharp.Data.GraphQL.Server/Exceptions.fs | 1 + src/FSharp.Data.GraphQL.Server/Executor.fs | 5 ++- src/FSharp.Data.GraphQL.Server/IO.fs | 8 ++-- src/FSharp.Data.GraphQL.Server/Schema.fs | 4 +- src/FSharp.Data.GraphQL.Server/Values.fs | 8 +++- src/FSharp.Data.GraphQL.Shared/Errors.fs | 34 +++++++++----- .../SchemaDefinitions.fs | 44 ++++++++++++++----- .../ValidationTypes.fs | 1 + .../MiddlewareTests.fs | 2 +- .../FSharp.Data.GraphQL.Tests/SchemaTests.fs | 4 +- .../Variables and Inputs/InputComplexTests.fs | 4 +- .../InputObjectValidatorTests.fs | 4 +- ...OptionalsNormalizationTests.ValidString.fs | 4 +- .../OptionalsNormalizationTests.fs | 4 +- 18 files changed, 115 insertions(+), 42 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index 6434951af..c288b4a5a 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -139,11 +139,23 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> - logger.LogWarning( - $"Produced request error GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", - documentId, - metadata - ) + match errs |> List.choose (fun x -> x.Exception) |> List.tryHead with + | Some ex -> + logger.LogError( + ex, + "Error while processing request that generated response with documentId '{documentId}'", + documentId + ) + | None -> + logger.LogWarning( + ( "Produced request error GraphQL response with:\n" + + "- documentId: '{documentId}'\n" + + "- error(s):\n {requestError}\n" + + "- metadata:\n {metadata}\n" ), + documentId, + errs, + metadata + ) GQLResponse.RequestError(documentId, errs) diff --git a/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs b/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs index b744df81c..a2444ffed 100644 --- a/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs +++ b/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs @@ -47,7 +47,7 @@ type internal QueryWeightMiddleware(threshold : float, reportToMetadata : bool) | ResolveLive info -> checkThreshold current (info :: xs) checkThreshold 0.0 fields let error (ctx : ExecutionContext) = - GQLExecutionResult.ErrorAsync(ctx.ExecutionPlan.DocumentId, "Query complexity exceeds maximum threshold. Please reduce query complexity and try again.", ctx.Metadata) + GQLExecutionResult.ErrorAsync(ctx.ExecutionPlan.DocumentId, "Query complexity exceeds maximum threshold. Please reduce query complexity and try again.", None, ctx.Metadata) let (pass, totalWeight) = measureThreshold threshold ctx.ExecutionPlan.Fields let ctx = match reportToMetadata with diff --git a/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs b/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs index cd4a0f18e..e830d2603 100644 --- a/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs +++ b/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs @@ -104,7 +104,9 @@ module SchemaDefinitions = | ObjectValue x -> mapInput x | NullValue -> NoFilter |> Ok // TODO: Get union case - | _ -> Error [{ new IGQLError with member _.Message = $"'ObjectListFilter' must be defined as object but got '{x.GetType ()}'" }] + | _ -> Error [{ new IGQLError with + member _.Message = $"'ObjectListFilter' must be defined as object but got '{x.GetType ()}'" + member _.Exception = None }] let private coerceObjectListFilterValue (x : obj) : ObjectListFilter option = match x with diff --git a/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs b/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs index f4defba80..a2d14f397 100644 --- a/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs +++ b/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs @@ -29,6 +29,7 @@ type internal CoercionError = { interface IGQLError with member this.Message = this.Message + member this.Exception = None interface IGQLErrorExtensions with @@ -78,6 +79,7 @@ type internal CoercionErrorWrapper = { interface IGQLError with member this.Message = this.InnerError.Message + member this.Exception = this.InnerError.Exception interface IGQLErrorExtensions with diff --git a/src/FSharp.Data.GraphQL.Server/Exceptions.fs b/src/FSharp.Data.GraphQL.Server/Exceptions.fs index 28ed75f94..f4198c99a 100644 --- a/src/FSharp.Data.GraphQL.Server/Exceptions.fs +++ b/src/FSharp.Data.GraphQL.Server/Exceptions.fs @@ -21,6 +21,7 @@ type GQLMessageExceptionBase (errorKind, msg, [] extensions) = inherit GraphQLException (msg) interface IGQLError with member _.Message = msg + member this.Exception = Some this interface IGQLErrorExtensions with member _.Extensions = match extensions with diff --git a/src/FSharp.Data.GraphQL.Server/Executor.fs b/src/FSharp.Data.GraphQL.Server/Executor.fs index 6ad5b8288..32c0f969c 100644 --- a/src/FSharp.Data.GraphQL.Server/Executor.fs +++ b/src/FSharp.Data.GraphQL.Server/Executor.fs @@ -123,7 +123,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s return prepareOutput res with | :? GQLMessageException as ex -> return prepareOutput(GQLExecutionResult.Error (documentId, ex, executionPlan.Metadata)) - | ex -> return prepareOutput (GQLExecutionResult.Error(documentId, ex.ToString(), executionPlan.Metadata)) // TODO: Handle better + | ex -> return prepareOutput (GQLExecutionResult.Error(documentId, ex.ToString(), Some ex, executionPlan.Metadata)) // TODO: Handle better } let execute (executionPlan: ExecutionPlan, data: 'Root option, variables: ImmutableDictionary option) = @@ -143,6 +143,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s | Some m -> Ok m | None -> Error <| [ GQLProblemDetails.CreateWithKind ( "Operation to be executed is of type mutation, but no mutation root object was defined in current schema", + None, ErrorKind.Validation )] | Subscription -> @@ -150,6 +151,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s | Some s -> Ok <| upcast s | None -> Error <| [ GQLProblemDetails.CreateWithKind ( "Operation to be executed is of type subscription, but no subscription root object was defined in the current schema", + None, ErrorKind.Validation )] do! @@ -167,6 +169,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s return runMiddlewares (fun x -> x.PlanOperation) planningCtx planOperation | None -> return! Error <| [ GQLProblemDetails.CreateWithKind ( "No operation with specified name has been found for provided document", + None, ErrorKind.Validation )] } diff --git a/src/FSharp.Data.GraphQL.Server/IO.fs b/src/FSharp.Data.GraphQL.Server/IO.fs index b9fe6e73b..8d097f44b 100644 --- a/src/FSharp.Data.GraphQL.Server/IO.fs +++ b/src/FSharp.Data.GraphQL.Server/IO.fs @@ -56,12 +56,12 @@ type GQLExecutionResult = GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.OfError error ], meta) static member Error(documentId, errors, meta) = GQLExecutionResult.RequestError(documentId, errors |> List.map GQLProblemDetails.OfError, meta) - static member Error(documentId, msg, meta) = - GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create msg ], meta) + static member Error(documentId, msg, ex : Exception option, meta) = + GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create (msg, ex) ], meta) static member Invalid(documentId, errors, meta) = GQLExecutionResult.RequestError(documentId, errors, meta) - static member ErrorAsync(documentId, msg : string, meta) = - AsyncVal.wrap (GQLExecutionResult.Error (documentId, msg, meta)) + static member ErrorAsync(documentId, msg : string, ex : Exception option, meta) = + AsyncVal.wrap (GQLExecutionResult.Error (documentId, msg, ex, meta)) static member ErrorAsync(documentId, error : IGQLError, meta) = AsyncVal.wrap (GQLExecutionResult.Error (documentId, error, meta)) diff --git a/src/FSharp.Data.GraphQL.Server/Schema.fs b/src/FSharp.Data.GraphQL.Server/Schema.fs index ba44fdceb..7fa9b4887 100644 --- a/src/FSharp.Data.GraphQL.Server/Schema.fs +++ b/src/FSharp.Data.GraphQL.Server/Schema.fs @@ -133,7 +133,9 @@ type SchemaConfig = fun path ex -> match ex with | :? GQLMessageException as ex -> [ex] - | ex -> [{ new IGQLError with member _.Message = ex.Message }] + | ex -> [{ new IGQLError with + member _.Message = ex.Message + member _.Exception = Some ex }] SubscriptionProvider = SchemaConfig.DefaultSubscriptionProvider() LiveFieldSubscriptionProvider = SchemaConfig.DefaultLiveFieldSubscriptionProvider() JsonOptions = JsonFSharpOptions.Default().ToJsonSerializerOptions() } diff --git a/src/FSharp.Data.GraphQL.Server/Values.fs b/src/FSharp.Data.GraphQL.Server/Values.fs index cddd8c0c7..ab46f6857 100644 --- a/src/FSharp.Data.GraphQL.Server/Values.fs +++ b/src/FSharp.Data.GraphQL.Server/Values.fs @@ -209,7 +209,9 @@ let rec internal compileByType (inputObjectPath: FieldPath) (inputSource : Input return found else Debugger.Break() - return! Error [{ new IGQLError with member _.Message = $"A variable '${variableName}' is not an object" }] + return! Error [{ new IGQLError with + member _.Message = $"A variable '${variableName}' is not an object" + member _.Exception = None }] | false, _ -> return null } | _ -> Ok null @@ -267,7 +269,9 @@ let rec internal compileByType (inputObjectPath: FieldPath) (inputSource : Input | VariableName variableName -> match variables.TryGetValue variableName with | true, var -> Ok var - | false, _ -> Error [ { new IGQLError with member _.Message = $"A variable '${variableName}' not found" } ] + | false, _ -> Error [ { new IGQLError with + member _.Message = $"A variable '${variableName}' not found" + member _.Exception = None } ] | _ -> result { let! coerced = coerceEnumInput value diff --git a/src/FSharp.Data.GraphQL.Shared/Errors.fs b/src/FSharp.Data.GraphQL.Shared/Errors.fs index a408f1d85..39d15d367 100644 --- a/src/FSharp.Data.GraphQL.Shared/Errors.fs +++ b/src/FSharp.Data.GraphQL.Shared/Errors.fs @@ -11,6 +11,7 @@ type internal FieldPath = obj list type IGQLError = abstract member Message : string with get + abstract member Exception : (Exception option) with get type internal ICoerceGQLError = inherit IGQLError @@ -66,6 +67,12 @@ type GQLProblemDetails = { [] Message : string + /// + /// The possible exception associated with this error. It won't be serialized. + /// + [] + Exception : Exception option + /// /// An array of fields path segments that that identify the specific field path in a GraphQL query where the resolution problem occurs. /// @@ -127,50 +134,57 @@ type GQLProblemDetails = { | :? IReadOnlyDictionary as extensions -> extensions | _ -> ReadOnlyDictionary mutableExtensions - static member internal CreateWithKind (message, kind : ErrorKind, ?path) = { + static member internal CreateWithKind (message : string, ex : Exception option, kind : ErrorKind, ?path) = { Message = message + Exception = ex Path = path |> Skippable.ofOption Locations = Skip Extensions = Dictionary 1 |> GQLProblemDetails.SetErrorKind kind |> Include } - static member Create (message, ?extensions : IReadOnlyDictionary) = { + static member Create (message : string, ex: Exception option, ?extensions : IReadOnlyDictionary) = { Message = message + Exception = ex Path = Skip Locations = Skip Extensions = extensions |> Skippable.ofOption } - static member Create (message, extensions) = { + static member Create (message : string, ex: Exception option, extensions : Skippable>) = { Message = message + Exception = ex Path = Skip Locations = Skip Extensions = extensions } - static member Create (message, path, ?extensions : IReadOnlyDictionary) = { + static member Create (message : string, ex: Exception option, path : FieldPath, ?extensions : IReadOnlyDictionary) = { Message = message + Exception = ex Path = Include path Locations = Skip Extensions = extensions |> Skippable.ofOption } - static member Create (message, path, extensions) = { + static member Create (message : string, ex: Exception option, path : FieldPath, extensions : Skippable>) = { Message = message + Exception = ex Path = Include path Locations = Skip Extensions = extensions } - static member Create (message, locations, ?extensions : IReadOnlyDictionary) = { + static member Create (message : string, ex: Exception option, locations : GQLProblemLocation list, ?extensions : IReadOnlyDictionary) = { Message = message + Exception = ex Path = Skip Locations = Include locations Extensions = extensions |> Skippable.ofOption } - static member Create (message, locations, extensions) = { + static member Create (message : string, ex: Exception option, locations : GQLProblemLocation list, extensions : Skippable>) = { Message = message + Exception = ex Path = Skip Locations = Include locations Extensions = extensions @@ -187,7 +201,7 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, extensions) + GQLProblemDetails.Create (message, error.Exception, extensions) static member OfFieldError (path : FieldPath) (error : IGQLError) = let extensions = @@ -200,7 +214,7 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, path, extensions) + GQLProblemDetails.Create (message, error.Exception, path, extensions) static member internal OfFieldExecutionError (path : FieldPath) (error : IGQLError) = let extensions = @@ -216,7 +230,7 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, path, extensions) + GQLProblemDetails.Create (message, error.Exception, path, extensions) override this.GetHashCode () = let extensionsHashCode = diff --git a/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs b/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs index 5f5275b3d..995d04d1f 100644 --- a/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs +++ b/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs @@ -31,7 +31,9 @@ module SchemaDefinitions = | NullValue -> $"Inline value 'null' cannot be converted into {destinationType}" | EnumValue value -> getMessage "enum" value | value -> raise <| NotSupportedException $"{value} cannot be passed as scalar input" - Error [{ new IGQLError with member _.Message = message }] + Error [{ new IGQLError with + member _.Message = message + member _.Exception = None }] member inputValue.GetCoerceRangeError(destinationType, minValue, maxValue) = let getMessage inputType value = $"Inline value '{value}' of type %s{inputType} cannot be converted into %s{destinationType} of range from {minValue} to {maxValue}" @@ -44,23 +46,33 @@ module SchemaDefinitions = | NullValue -> $"Inline value 'null' cannot be converted into {destinationType}" | EnumValue value -> getMessage "enum" value | value -> raise <| NotSupportedException $"{value} cannot be passed as scalar input" - Error [{ new IGQLError with member _.Message = message }] + Error [{ new IGQLError with + member _.Message = message + member _.Exception = None }] type JsonElement with member e.GetDeserializeError(destinationType, minValue, maxValue ) = let jsonValue = match e.ValueKind with JsonValueKind.String -> e.GetString() | _ -> e.GetRawText() - Error [{ new IGQLError with member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType} of range from {minValue} to {maxValue}" }] + Error [{ new IGQLError with + member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType} of range from {minValue} to {maxValue}" + member _.Exception = None }] member e.GetDeserializeError(destinationType) = let jsonValue = match e.ValueKind with JsonValueKind.String -> e.GetString() | _ -> e.GetRawText() - Error [{ new IGQLError with member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType}" }] + Error [{ new IGQLError with + member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType}" + member _.Exception = None }] let getParseRangeError (destinationType, minValue, maxValue) value = - Error [{ new IGQLError with member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType} of range from {minValue} to {maxValue}" }] + Error [{ new IGQLError with + member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType} of range from {minValue} to {maxValue}" + member _.Exception = None }] let getParseError destinationType value = - Error [{ new IGQLError with member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType}" }] + Error [{ new IGQLError with + member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType}" + member _.Exception = None }] open System.Globalization @@ -369,7 +381,9 @@ module SchemaDefinitions = | VariableName variableName -> match variables.TryGetValue variableName with | true, value -> Ok value - | false, _ -> Error [{ new IGQLError with member _.Message = $"A variable '$%s{variableName}' not found" }] + | false, _ -> Error [{ new IGQLError with + member _.Message = $"A variable '$%s{variableName}' not found" + member _.Exception = None }] | v -> other v /// GraphQL type of int @@ -537,7 +551,9 @@ module SchemaDefinitions = coerceOutput : obj -> 'T option, ?description : string) : ScalarDefinition<'T> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with member _.Message = msg } |> List.singleton) + CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with + member _.Message = msg + member _.Exception = None } |> List.singleton) CoerceOutput = coerceOutput } /// @@ -551,7 +567,9 @@ module SchemaDefinitions = coerceOutput : obj -> 'T option, ?description : string) : ScalarDefinition<'T> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with member _.Message = msg })) + CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with + member _.Message = msg + member _.Exception = None })) CoerceOutput = coerceOutput } /// @@ -593,7 +611,9 @@ module SchemaDefinitions = coerceOutput : obj -> 'Primitive option, ?description : string) : ScalarDefinition<'Primitive, 'Wrapper> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with member _.Message = msg } |> List.singleton) + CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with + member _.Message = msg + member _.Exception = None } |> List.singleton) CoerceOutput = coerceOutput } /// @@ -607,7 +627,9 @@ module SchemaDefinitions = coerceOutput : obj -> 'Primitive option, ?description : string) : ScalarDefinition<'Primitive, 'Wrapper> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with member _.Message = msg })) + CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with + member _.Message = msg + member _.Exception = None })) CoerceOutput = coerceOutput } /// diff --git a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs index d97cb8bba..491de1fa5 100644 --- a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs +++ b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs @@ -56,6 +56,7 @@ type AstError = [ { Message = message + Exception = None Path = path |> Skippable.ofOption |> Skippable.map List.rev Locations = Skip Extensions = diff --git a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs index aaf31f99e..d52b3c4e2 100644 --- a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs @@ -89,7 +89,7 @@ let execute (query : Document) = executor.AsyncExecute(query) |> sync let expectedErrors : GQLProblemDetails list = - [ GQLProblemDetails.Create ("Query complexity exceeds maximum threshold. Please reduce query complexity and try again.") ] + [ GQLProblemDetails.Create ("Query complexity exceeds maximum threshold. Please reduce query complexity and try again.", None) ] [] let ``Simple query: Should pass when below threshold``() = diff --git a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs index 39b0f1260..32d9d80b3 100644 --- a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs @@ -46,7 +46,9 @@ let ``Schema config should be able to override default error handling`` () = (fun path ex -> let i = idx idx <- idx + 1 - [ { new IGQLError with member __.Message = i.ToString () } ]) + [ { new IGQLError with + member __.Message = i.ToString () + member __.Exception = None } ]) } let TestType = Define.Object ( diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs index ecebe84e4..5a0309f33 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs @@ -30,7 +30,9 @@ let TestComplexScalar = | InputParameterValue.InlineConstant (StringValue s) -> s if value = "SerializedValue" then Ok "DeserializedValue" - else Error { new IGQLError with member _.Message = "" }), + else Error { new IGQLError with + member _.Message = "" + member _.Exception = None }), coerceOutput = (fun value -> if value = upcast "DeserializedValue" then Some "SerializedValue" diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs index d9bb21a16..19b6d5c63 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs @@ -21,7 +21,9 @@ open ErrorHelpers type InputRecord = { Country : string; ZipCode : string; City : string } let createSingleError message = - [{ new IGQLError with member _.Message = message }] + [{ new IGQLError with + member _.Message = message + member _.Exception = None }] let InputRecordType = Define.InputObject ( diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs index 1bede223a..4ebec160b 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs @@ -122,7 +122,9 @@ module ValidationErrors = let toIGQLErrors (errors: ValidationErrors) : IGQLError list = errors |> ValidationErrors.toList - |> List.map (fun e -> { new IGQLError with member _.Message = e }) + |> List.map (fun e -> { new IGQLError with + member _.Message = e + member _.Exception = None }) module Operators = diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs index 1fb88fc2e..6b3a354a0 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs @@ -148,7 +148,9 @@ open FSharp.Data.GraphQL.Validation.ValidationResult open ErrorHelpers let createSingleError message = - [{ new IGQLError with member _.Message = message }] + [{ new IGQLError with + member _.Message = message + member _.Exception = None }] type InputRecordNested = { HomeAddress : AddressRecord; WorkAddress : AddressRecord option; MailingAddress : AddressRecord voption } From b03bf400d8bf19eedb8526db6ba86a4f3dbd9369 Mon Sep 17 00:00:00 2001 From: valber Date: Fri, 5 Apr 2024 18:09:55 +0200 Subject: [PATCH 02/17] refactor: trying to improve readability by removing confusing active pattern --- .../GraphQLWebsocketMiddleware.fs | 2 +- src/FSharp.Data.GraphQL.Server/Execution.fs | 8 -------- src/FSharp.Data.GraphQL.Server/Executor.fs | 2 +- tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs | 2 +- .../ExecutorMiddlewareTests.fs | 2 +- tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs | 4 ++-- tests/FSharp.Data.GraphQL.Tests/MutationTests.fs | 4 ++-- .../Relay/ConnectionTests.fs | 4 ++-- tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs | 2 +- tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs | 12 ++++++------ .../InputObjectValidatorTests.fs | 2 +- 11 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs index 7e225cf6a..81831ad41 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs @@ -203,7 +203,7 @@ type GraphQLWebSocketMiddleware<'Root> } let applyPlanExecutionResult (id : SubscriptionId) (socket) (executionResult : GQLExecutionResult) : Task = task { - match executionResult with + match executionResult.Content with | Stream observableOutput -> (subscriptions, socket, observableOutput, serializerOptions) |> addClientSubscription id sendSubscriptionResponseOutput diff --git a/src/FSharp.Data.GraphQL.Server/Execution.fs b/src/FSharp.Data.GraphQL.Server/Execution.fs index ea4053c81..10351cae4 100644 --- a/src/FSharp.Data.GraphQL.Server/Execution.fs +++ b/src/FSharp.Data.GraphQL.Server/Execution.fs @@ -19,14 +19,6 @@ open FSharp.Data.GraphQL type Output = IDictionary -let (|RequestError|Direct|Deferred|Stream|) (response : GQLExecutionResult) = - match response.Content with - | RequestError errs -> RequestError errs - | Direct (data, errors) -> Direct (data, errors) - | Deferred (data, errors, deferred) -> Deferred (data, errors, deferred) - | Stream data -> Stream data - - /// Name value lookup used as output to be serialized into JSON. /// It has a form of a dictionary with fixed set of keys. Values under keys /// can be set, but no new entry can be added or removed, once lookup diff --git a/src/FSharp.Data.GraphQL.Server/Executor.fs b/src/FSharp.Data.GraphQL.Server/Executor.fs index 32c0f969c..3a12ba6f1 100644 --- a/src/FSharp.Data.GraphQL.Server/Executor.fs +++ b/src/FSharp.Data.GraphQL.Server/Executor.fs @@ -101,7 +101,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s let eval (executionPlan: ExecutionPlan, data: 'Root option, variables: ImmutableDictionary): Async = let documentId = executionPlan.DocumentId let prepareOutput res = - match res with + match res.Content with | RequestError errs -> GQLExecutionResult.Error (documentId, errs, res.Metadata) | Direct (data, errors) -> GQLExecutionResult.Direct (documentId, data, errors, res.Metadata) | Deferred (data, errors, deferred) -> GQLExecutionResult.Deferred (documentId, data, errors, deferred, res.Metadata) diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs index 99ffafa01..1638b020f 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs @@ -376,7 +376,7 @@ let ``Execution when querying returns unique document id with response`` () = let result2 = sync <| Executor(schema).AsyncExecute("query Example { a, b, a }", { A = "aa"; B = 2 }) result1.DocumentId |> notEquals Unchecked.defaultof result1.DocumentId |> equals result2.DocumentId - match result1,result2 with + match result1.Content,result2.Content with | Direct(data1, errors1), Direct(data2, errors2) -> equals data1 data2 equals errors1 errors2 diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs index 377e098e2..10168f057 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs @@ -117,7 +117,7 @@ let ``Executor middleware: change fields and measure planning time`` () = [ "a", upcast "Cookie" "b", upcast "Banana" "d", upcast false ] ] - match result with + match result.Content with | Direct (data, errors) -> empty errors data |> equals (upcast expected) diff --git a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs index d52b3c4e2..613effde0 100644 --- a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs @@ -130,7 +130,7 @@ let ``Simple query: Should pass when below threshold``() = ] ] let result = execute query - match result with + match result.Content with | Direct (data, errors) -> empty errors data |> equals (upcast expected) @@ -453,7 +453,7 @@ let ``Inline fragment query : Should not pass when above threshold``() = } }""" let result = execute query - match result with + match result.Content with | RequestError errors -> errors |> equals expectedErrors | response -> fail $"Expected 'RequestError' GQLResponse but got\n{response}" diff --git a/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs b/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs index def1055f7..66e32c389 100644 --- a/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs @@ -72,7 +72,7 @@ let ``Execute handles mutation execution ordering: evaluates mutations serially` "fourth", upcast NameValueLookup.ofList [ "theNumber", 4 :> obj] "fifth", upcast NameValueLookup.ofList [ "theNumber", 5 :> obj] ] - match mutationResult with + match mutationResult.Content with | Direct(data, errors) -> empty errors data |> equals (upcast expected) @@ -113,7 +113,7 @@ let ``Execute handles mutation execution ordering: evaluates mutations correctly "sixth", null ] - match mutationResult with + match mutationResult.Content with | Direct(data, errors) -> data |> equals (upcast expected) List.length errors |> equals 2 diff --git a/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs b/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs index 7ee01429b..e620fde19 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs @@ -156,7 +156,7 @@ let ``Connection definition includes connection and edge fields for simple cases ] ] ] - match result with + match result.Content with | Direct (data, errors) -> empty errors data |> equals (upcast expected) @@ -206,7 +206,7 @@ let ``Connection definition includes connection and edge fields for complex case ] ] ]]]]]] - match result with + match result.Content with | Direct (data, errors) -> empty errors data |> equals (upcast expected) diff --git a/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs b/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs index 74172dac6..84e96a4a4 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs @@ -97,6 +97,6 @@ let ``Relay cursor works for types with nested fileds`` () = let result = sync <| schemaProcessor.AsyncExecute (parse query) - match result with + match result.Content with | Direct (_, errors) -> empty errors | response -> fail $"Expected a 'Direct' GQLResponse but got\n{response}" diff --git a/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs b/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs index 17f9786c0..99c57ee54 100644 --- a/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs @@ -116,7 +116,7 @@ let ``Can subscribe to sync field and get results``() = } }""" let result = executor.AsyncExecute(query) |> sync - match result with + match result.Content with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -143,7 +143,7 @@ let ``Can subscribe to tagged sync field and get results with expected tag``() = } }""" let result = executor.AsyncExecute(query) |> sync - match result with + match result.Content with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -163,7 +163,7 @@ let ``Can subscribe to tagged sync field and do not get results with unexpected } }""" let result = executor.AsyncExecute(query) |> sync - match result with + match result.Content with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -185,7 +185,7 @@ let ``Can subscribe to async field and get results``() = } }""" let result = executor.AsyncExecute(query) |> sync - match result with + match result.Content with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -212,7 +212,7 @@ let ``Can subscribe to tagged async field and get results with expected tag``() } }""" let result = executor.AsyncExecute(query) |> sync - match result with + match result.Content with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -232,7 +232,7 @@ let ``Can subscribe to tagged async field and do not get results with unexpected } }""" let result = executor.AsyncExecute(query) |> sync - match result with + match result.Content with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs index 19b6d5c63..4384c5ec0 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs @@ -142,7 +142,7 @@ let ``Execute handles validation of invalid inline input records with all fields ) }""" let result = sync <| schema.AsyncExecute(parse query) - match result with + match result.Content with | RequestError [ zipCodeError ; addressError ] -> zipCodeError |> ensureInputObjectValidationError (Argument "record") "ZipCode must be 5 characters for US" [] "InputRecord!" addressError |> ensureInputObjectValidationError (Argument "recordNested") "HomeAddress and MailingAddress must be different" [] "InputRecordNested" From d24abb4b9331d120b1848daec637dad8b14f693e Mon Sep 17 00:00:00 2001 From: valber Date: Fri, 5 Apr 2024 18:40:59 +0200 Subject: [PATCH 03/17] adapting code to new exception parameter --- tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs | 4 ++-- tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs | 8 ++++---- tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs | 8 ++++---- tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs | 4 ++-- .../Variables and Inputs/InputComplexTests.fs | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs b/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs index 7cde89879..cdd05257f 100644 --- a/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs @@ -11,8 +11,8 @@ open FSharp.Data.GraphQL.Types module internal GQLProblemDetails = - let CreateValidation message = GQLProblemDetails.CreateWithKind (message, Validation) - let CreateValidationFor path message = GQLProblemDetails.CreateWithKind (message, Validation, path) + let CreateValidation message = GQLProblemDetails.CreateWithKind (message, None, Validation) + let CreateValidationFor path message = GQLProblemDetails.CreateWithKind (message, None, Validation, path) type Command = | SIT = 1 diff --git a/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs b/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs index a5660fc3a..6fc56fc66 100644 --- a/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs @@ -276,7 +276,7 @@ let ``Resolver error`` () = let expectedDeferred = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverError"; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Resolver error!", None, Execution, [ box "testData"; "resolverError"; "value" ]) ], [ "testData"; "resolverError" ] ) let query = parse """{ @@ -305,13 +305,13 @@ let ``Resolver list error`` () = let expectedDeferred1 = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverListError"; 0; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Resolver error!", None, Execution, [ box "testData"; "resolverListError"; 0; "value" ]) ], [ box "testData"; "resolverListError"; 0 ] ) let expectedDeferred2 = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverListError"; 1; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Resolver error!", None, Execution, [ box "testData"; "resolverListError"; 1; "value" ]) ], [ box "testData"; "resolverListError"; 1 ] ) let query = parse """{ @@ -344,7 +344,7 @@ let ``Nullable error`` () = let expectedDeferred = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Non-Null field value resolved as a null!", Execution, [ box "testData"; "nullableError"; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Non-Null field value resolved as a null!", None, Execution, [ box "testData"; "nullableError"; "value" ]) ], [ "testData"; "nullableError" ] ) let query = parse """{ diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs index 1638b020f..0c90bd83a 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs @@ -402,7 +402,7 @@ let ``Execution handles errors: properly propagates errors`` () = "inner", null ] let expectedErrors = [ - GQLProblemDetails.CreateWithKind ("Non-Null field kaboom resolved as a null!", Execution, [ box "inner"; "kaboom" ]) + GQLProblemDetails.CreateWithKind ("Non-Null field kaboom resolved as a null!", None, Execution, [ box "inner"; "kaboom" ]) ] let result = sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", { Inner = { Kaboom = null } }) ensureDirect result <| fun data errors -> @@ -417,7 +417,7 @@ let ``Execution handles errors: exceptions`` () = "Type", [ Define.Field("a", StringType, fun _ _ -> failwith "Resolver Error!") ])) - let expectedError = GQLProblemDetails.CreateWithKind ("Resolver Error!", Execution, [ box "a" ]) + let expectedError = GQLProblemDetails.CreateWithKind ("Resolver Error!", None, Execution, [ box "a" ]) let result = sync <| Executor(schema).AsyncExecute("query Test { a }", ()) ensureRequestError result <| fun [ error ] -> error |> equals expectedError @@ -439,8 +439,8 @@ let ``Execution handles errors: nullable list fields`` () = ] let expectedErrors = [ - GQLProblemDetails.CreateWithKind ("Resolver Error!", Execution, [ box "list"; 0; "error" ]) - GQLProblemDetails.CreateWithKind ("Resolver Error!", Execution, [ box "list"; 1; "error" ]) + GQLProblemDetails.CreateWithKind ("Resolver Error!", None, Execution, [ box "list"; 0; "error" ]) + GQLProblemDetails.CreateWithKind ("Resolver Error!", None, Execution, [ box "list"; 1; "error" ]) ] let result = sync <| Executor(schema).AsyncExecute("query Test { list { error } }", ()) ensureDirect result <| fun data errors -> diff --git a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs index 32d9d80b3..ec6b4185c 100644 --- a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs @@ -75,8 +75,8 @@ let ``Schema config should be able to override default error handling`` () = let expected = NameValueLookup.ofList [ "test", box <| NameValueLookup.ofList [ "failing1", null; "passing", box "ok"; "failing2", null ] ] let expectedErrors = [ - GQLProblemDetails.CreateWithKind ("0", Execution, [ box "test"; "failing1" ]) - GQLProblemDetails.CreateWithKind ("1", Execution, [ box "test"; "failing2" ]) + GQLProblemDetails.CreateWithKind ("0", None, Execution, [ box "test"; "failing1" ]) + GQLProblemDetails.CreateWithKind ("1", None, Execution, [ box "test"; "failing2" ]) ] ensureDirect result <| fun data errors -> diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs index 5a0309f33..8101bf7a7 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs @@ -225,5 +225,5 @@ let ``Execute handles list inputs and nullability and does not allow unknown typ let result = sync <| Executor(schema).AsyncExecute (ast, variables = params') let expectedError = let message = "A variable '$input' in operation 'q' has a type that is not an input type defined by the schema (UnknownType!)." - GQLProblemDetails.CreateWithKind (message, Validation) + GQLProblemDetails.CreateWithKind (message, None, Validation) ensureRequestError result <| fun [ error ] -> error |> equals expectedError From 46d5b760d988fc2e45dac274495883f866c18856 Mon Sep 17 00:00:00 2001 From: valber Date: Fri, 5 Apr 2024 18:52:06 +0200 Subject: [PATCH 04/17] PART II of "adapting code to new exception parameter" --- .../CustomSchemaTypes.fs | 4 +++- tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs b/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs index e117c8c33..f02c5a5bc 100644 --- a/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs +++ b/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs @@ -19,7 +19,9 @@ module SchemaDefinitions = let private coerceUploadInput (_ : InputParameterValue) : Result = Result.Error [ - { new IGQLError with member _.Message = "Cannot coerce upload input. The type `Upload` can only be passed as a variable through a multipart request." } + { new IGQLError with + member _.Message = "Cannot coerce upload input. The type `Upload` can only be passed as a variable through a multipart request." + member _.Exception = None } ] let private coerceUploadValue (value : obj) = diff --git a/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json b/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json index c3fe6fa10..9be5e60f2 100644 --- a/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json +++ b/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json @@ -1,5 +1,5 @@ { - "documentId": -727244275, + "documentId": -683423135, "data": { "__schema": { "queryType": { From 6b97da92ddb0588102d037f4f732f78dfc6c7888 Mon Sep 17 00:00:00 2001 From: valber Date: Mon, 8 Apr 2024 22:07:05 +0200 Subject: [PATCH 05/17] Revert "PART II of "adapting code to new exception parameter"" This reverts commit 46d5b760d988fc2e45dac274495883f866c18856. --- .../CustomSchemaTypes.fs | 4 +--- tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs b/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs index f02c5a5bc..e117c8c33 100644 --- a/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs +++ b/tests/FSharp.Data.GraphQL.IntegrationTests.Server/CustomSchemaTypes.fs @@ -19,9 +19,7 @@ module SchemaDefinitions = let private coerceUploadInput (_ : InputParameterValue) : Result = Result.Error [ - { new IGQLError with - member _.Message = "Cannot coerce upload input. The type `Upload` can only be passed as a variable through a multipart request." - member _.Exception = None } + { new IGQLError with member _.Message = "Cannot coerce upload input. The type `Upload` can only be passed as a variable through a multipart request." } ] let private coerceUploadValue (value : obj) = diff --git a/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json b/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json index 9be5e60f2..c3fe6fa10 100644 --- a/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json +++ b/tests/FSharp.Data.GraphQL.IntegrationTests/introspection.json @@ -1,5 +1,5 @@ { - "documentId": -683423135, + "documentId": -727244275, "data": { "__schema": { "queryType": { From d448aaaec03c712a1541e8e3b943db02d598947d Mon Sep 17 00:00:00 2001 From: valber Date: Mon, 8 Apr 2024 22:07:08 +0200 Subject: [PATCH 06/17] Revert "adapting code to new exception parameter" This reverts commit d24abb4b9331d120b1848daec637dad8b14f693e. --- tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs | 4 ++-- tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs | 8 ++++---- tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs | 8 ++++---- tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs | 4 ++-- .../Variables and Inputs/InputComplexTests.fs | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs b/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs index cdd05257f..7cde89879 100644 --- a/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/AstValidationTests.fs @@ -11,8 +11,8 @@ open FSharp.Data.GraphQL.Types module internal GQLProblemDetails = - let CreateValidation message = GQLProblemDetails.CreateWithKind (message, None, Validation) - let CreateValidationFor path message = GQLProblemDetails.CreateWithKind (message, None, Validation, path) + let CreateValidation message = GQLProblemDetails.CreateWithKind (message, Validation) + let CreateValidationFor path message = GQLProblemDetails.CreateWithKind (message, Validation, path) type Command = | SIT = 1 diff --git a/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs b/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs index 6fc56fc66..a5660fc3a 100644 --- a/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/DeferredTests.fs @@ -276,7 +276,7 @@ let ``Resolver error`` () = let expectedDeferred = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Resolver error!", None, Execution, [ box "testData"; "resolverError"; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverError"; "value" ]) ], [ "testData"; "resolverError" ] ) let query = parse """{ @@ -305,13 +305,13 @@ let ``Resolver list error`` () = let expectedDeferred1 = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Resolver error!", None, Execution, [ box "testData"; "resolverListError"; 0; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverListError"; 0; "value" ]) ], [ box "testData"; "resolverListError"; 0 ] ) let expectedDeferred2 = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Resolver error!", None, Execution, [ box "testData"; "resolverListError"; 1; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Resolver error!", Execution, [ box "testData"; "resolverListError"; 1; "value" ]) ], [ box "testData"; "resolverListError"; 1 ] ) let query = parse """{ @@ -344,7 +344,7 @@ let ``Nullable error`` () = let expectedDeferred = DeferredErrors ( null, - [ GQLProblemDetails.CreateWithKind ("Non-Null field value resolved as a null!", None, Execution, [ box "testData"; "nullableError"; "value" ]) ], + [ GQLProblemDetails.CreateWithKind ("Non-Null field value resolved as a null!", Execution, [ box "testData"; "nullableError"; "value" ]) ], [ "testData"; "nullableError" ] ) let query = parse """{ diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs index 0c90bd83a..1638b020f 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs @@ -402,7 +402,7 @@ let ``Execution handles errors: properly propagates errors`` () = "inner", null ] let expectedErrors = [ - GQLProblemDetails.CreateWithKind ("Non-Null field kaboom resolved as a null!", None, Execution, [ box "inner"; "kaboom" ]) + GQLProblemDetails.CreateWithKind ("Non-Null field kaboom resolved as a null!", Execution, [ box "inner"; "kaboom" ]) ] let result = sync <| Executor(schema).AsyncExecute("query Example { inner { kaboom } }", { Inner = { Kaboom = null } }) ensureDirect result <| fun data errors -> @@ -417,7 +417,7 @@ let ``Execution handles errors: exceptions`` () = "Type", [ Define.Field("a", StringType, fun _ _ -> failwith "Resolver Error!") ])) - let expectedError = GQLProblemDetails.CreateWithKind ("Resolver Error!", None, Execution, [ box "a" ]) + 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 @@ -439,8 +439,8 @@ let ``Execution handles errors: nullable list fields`` () = ] let expectedErrors = [ - GQLProblemDetails.CreateWithKind ("Resolver Error!", None, Execution, [ box "list"; 0; "error" ]) - GQLProblemDetails.CreateWithKind ("Resolver Error!", None, Execution, [ box "list"; 1; "error" ]) + GQLProblemDetails.CreateWithKind ("Resolver Error!", Execution, [ box "list"; 0; "error" ]) + GQLProblemDetails.CreateWithKind ("Resolver Error!", Execution, [ box "list"; 1; "error" ]) ] let result = sync <| Executor(schema).AsyncExecute("query Test { list { error } }", ()) ensureDirect result <| fun data errors -> diff --git a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs index ec6b4185c..32d9d80b3 100644 --- a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs @@ -75,8 +75,8 @@ let ``Schema config should be able to override default error handling`` () = let expected = NameValueLookup.ofList [ "test", box <| NameValueLookup.ofList [ "failing1", null; "passing", box "ok"; "failing2", null ] ] let expectedErrors = [ - GQLProblemDetails.CreateWithKind ("0", None, Execution, [ box "test"; "failing1" ]) - GQLProblemDetails.CreateWithKind ("1", None, Execution, [ box "test"; "failing2" ]) + GQLProblemDetails.CreateWithKind ("0", Execution, [ box "test"; "failing1" ]) + GQLProblemDetails.CreateWithKind ("1", Execution, [ box "test"; "failing2" ]) ] ensureDirect result <| fun data errors -> diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs index 8101bf7a7..5a0309f33 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs @@ -225,5 +225,5 @@ let ``Execute handles list inputs and nullability and does not allow unknown typ let result = sync <| Executor(schema).AsyncExecute (ast, variables = params') let expectedError = let message = "A variable '$input' in operation 'q' has a type that is not an input type defined by the schema (UnknownType!)." - GQLProblemDetails.CreateWithKind (message, None, Validation) + GQLProblemDetails.CreateWithKind (message, Validation) ensureRequestError result <| fun [ error ] -> error |> equals expectedError From 295dd601d84481b3ed5638226f4bb752b0546994 Mon Sep 17 00:00:00 2001 From: valber Date: Mon, 8 Apr 2024 22:07:09 +0200 Subject: [PATCH 07/17] Revert "refactor: trying to improve readability by removing confusing active pattern" This reverts commit b03bf400d8bf19eedb8526db6ba86a4f3dbd9369. --- .../GraphQLWebsocketMiddleware.fs | 2 +- src/FSharp.Data.GraphQL.Server/Execution.fs | 8 ++++++++ src/FSharp.Data.GraphQL.Server/Executor.fs | 2 +- tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs | 2 +- .../ExecutorMiddlewareTests.fs | 2 +- tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs | 4 ++-- tests/FSharp.Data.GraphQL.Tests/MutationTests.fs | 4 ++-- .../Relay/ConnectionTests.fs | 4 ++-- tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs | 2 +- tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs | 12 ++++++------ .../InputObjectValidatorTests.fs | 2 +- 11 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs index 81831ad41..7e225cf6a 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/GraphQLWebsocketMiddleware.fs @@ -203,7 +203,7 @@ type GraphQLWebSocketMiddleware<'Root> } let applyPlanExecutionResult (id : SubscriptionId) (socket) (executionResult : GQLExecutionResult) : Task = task { - match executionResult.Content with + match executionResult with | Stream observableOutput -> (subscriptions, socket, observableOutput, serializerOptions) |> addClientSubscription id sendSubscriptionResponseOutput diff --git a/src/FSharp.Data.GraphQL.Server/Execution.fs b/src/FSharp.Data.GraphQL.Server/Execution.fs index 10351cae4..ea4053c81 100644 --- a/src/FSharp.Data.GraphQL.Server/Execution.fs +++ b/src/FSharp.Data.GraphQL.Server/Execution.fs @@ -19,6 +19,14 @@ open FSharp.Data.GraphQL type Output = IDictionary +let (|RequestError|Direct|Deferred|Stream|) (response : GQLExecutionResult) = + match response.Content with + | RequestError errs -> RequestError errs + | Direct (data, errors) -> Direct (data, errors) + | Deferred (data, errors, deferred) -> Deferred (data, errors, deferred) + | Stream data -> Stream data + + /// Name value lookup used as output to be serialized into JSON. /// It has a form of a dictionary with fixed set of keys. Values under keys /// can be set, but no new entry can be added or removed, once lookup diff --git a/src/FSharp.Data.GraphQL.Server/Executor.fs b/src/FSharp.Data.GraphQL.Server/Executor.fs index 3a12ba6f1..32c0f969c 100644 --- a/src/FSharp.Data.GraphQL.Server/Executor.fs +++ b/src/FSharp.Data.GraphQL.Server/Executor.fs @@ -101,7 +101,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s let eval (executionPlan: ExecutionPlan, data: 'Root option, variables: ImmutableDictionary): Async = let documentId = executionPlan.DocumentId let prepareOutput res = - match res.Content with + match res with | RequestError errs -> GQLExecutionResult.Error (documentId, errs, res.Metadata) | Direct (data, errors) -> GQLExecutionResult.Direct (documentId, data, errors, res.Metadata) | Deferred (data, errors, deferred) -> GQLExecutionResult.Deferred (documentId, data, errors, deferred, res.Metadata) diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs index 1638b020f..99ffafa01 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutionTests.fs @@ -376,7 +376,7 @@ let ``Execution when querying returns unique document id with response`` () = let result2 = sync <| Executor(schema).AsyncExecute("query Example { a, b, a }", { A = "aa"; B = 2 }) result1.DocumentId |> notEquals Unchecked.defaultof result1.DocumentId |> equals result2.DocumentId - match result1.Content,result2.Content with + match result1,result2 with | Direct(data1, errors1), Direct(data2, errors2) -> equals data1 data2 equals errors1 errors2 diff --git a/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs b/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs index 10168f057..377e098e2 100644 --- a/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/ExecutorMiddlewareTests.fs @@ -117,7 +117,7 @@ let ``Executor middleware: change fields and measure planning time`` () = [ "a", upcast "Cookie" "b", upcast "Banana" "d", upcast false ] ] - match result.Content with + match result with | Direct (data, errors) -> empty errors data |> equals (upcast expected) diff --git a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs index 613effde0..d52b3c4e2 100644 --- a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs @@ -130,7 +130,7 @@ let ``Simple query: Should pass when below threshold``() = ] ] let result = execute query - match result.Content with + match result with | Direct (data, errors) -> empty errors data |> equals (upcast expected) @@ -453,7 +453,7 @@ let ``Inline fragment query : Should not pass when above threshold``() = } }""" let result = execute query - match result.Content with + match result with | RequestError errors -> errors |> equals expectedErrors | response -> fail $"Expected 'RequestError' GQLResponse but got\n{response}" diff --git a/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs b/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs index 66e32c389..def1055f7 100644 --- a/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/MutationTests.fs @@ -72,7 +72,7 @@ let ``Execute handles mutation execution ordering: evaluates mutations serially` "fourth", upcast NameValueLookup.ofList [ "theNumber", 4 :> obj] "fifth", upcast NameValueLookup.ofList [ "theNumber", 5 :> obj] ] - match mutationResult.Content with + match mutationResult with | Direct(data, errors) -> empty errors data |> equals (upcast expected) @@ -113,7 +113,7 @@ let ``Execute handles mutation execution ordering: evaluates mutations correctly "sixth", null ] - match mutationResult.Content with + match mutationResult with | Direct(data, errors) -> data |> equals (upcast expected) List.length errors |> equals 2 diff --git a/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs b/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs index e620fde19..7ee01429b 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Relay/ConnectionTests.fs @@ -156,7 +156,7 @@ let ``Connection definition includes connection and edge fields for simple cases ] ] ] - match result.Content with + match result with | Direct (data, errors) -> empty errors data |> equals (upcast expected) @@ -206,7 +206,7 @@ let ``Connection definition includes connection and edge fields for complex case ] ] ]]]]]] - match result.Content with + match result with | Direct (data, errors) -> empty errors data |> equals (upcast expected) diff --git a/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs b/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs index 84e96a4a4..74172dac6 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Relay/CursorTests.fs @@ -97,6 +97,6 @@ let ``Relay cursor works for types with nested fileds`` () = let result = sync <| schemaProcessor.AsyncExecute (parse query) - match result.Content with + match result with | Direct (_, errors) -> empty errors | response -> fail $"Expected a 'Direct' GQLResponse but got\n{response}" diff --git a/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs b/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs index 99c57ee54..17f9786c0 100644 --- a/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/SubscriptionTests.fs @@ -116,7 +116,7 @@ let ``Can subscribe to sync field and get results``() = } }""" let result = executor.AsyncExecute(query) |> sync - match result.Content with + match result with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -143,7 +143,7 @@ let ``Can subscribe to tagged sync field and get results with expected tag``() = } }""" let result = executor.AsyncExecute(query) |> sync - match result.Content with + match result with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -163,7 +163,7 @@ let ``Can subscribe to tagged sync field and do not get results with unexpected } }""" let result = executor.AsyncExecute(query) |> sync - match result.Content with + match result with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -185,7 +185,7 @@ let ``Can subscribe to async field and get results``() = } }""" let result = executor.AsyncExecute(query) |> sync - match result.Content with + match result with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -212,7 +212,7 @@ let ``Can subscribe to tagged async field and get results with expected tag``() } }""" let result = executor.AsyncExecute(query) |> sync - match result.Content with + match result with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" @@ -232,7 +232,7 @@ let ``Can subscribe to tagged async field and do not get results with unexpected } }""" let result = executor.AsyncExecute(query) |> sync - match result.Content with + match result with | Stream data -> use sub = Observer.create data updateValue 1 "Updated value 1" diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs index 4384c5ec0..19b6d5c63 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs @@ -142,7 +142,7 @@ let ``Execute handles validation of invalid inline input records with all fields ) }""" let result = sync <| schema.AsyncExecute(parse query) - match result.Content with + match result with | RequestError [ zipCodeError ; addressError ] -> zipCodeError |> ensureInputObjectValidationError (Argument "record") "ZipCode must be 5 characters for US" [] "InputRecord!" addressError |> ensureInputObjectValidationError (Argument "recordNested") "HomeAddress and MailingAddress must be different" [] "InputRecordNested" From c1f33192937ef77364798dd0c2b7b7cfbce2bfc7 Mon Sep 17 00:00:00 2001 From: valber Date: Mon, 8 Apr 2024 22:07:10 +0200 Subject: [PATCH 08/17] Revert ".AspNetCore: BREAKING CHANGE: logging server exceptions with exception info too" This reverts commit e829181c9a0b7948529796f0142007dac53b2851. --- .../Giraffe/HttpHandlers.fs | 22 +++------- .../MiddlewareDefinitions.fs | 2 +- .../SchemaDefinitions.fs | 4 +- src/FSharp.Data.GraphQL.Server/ErrorTypes.fs | 2 - src/FSharp.Data.GraphQL.Server/Exceptions.fs | 1 - src/FSharp.Data.GraphQL.Server/Executor.fs | 5 +-- src/FSharp.Data.GraphQL.Server/IO.fs | 8 ++-- src/FSharp.Data.GraphQL.Server/Schema.fs | 4 +- src/FSharp.Data.GraphQL.Server/Values.fs | 8 +--- src/FSharp.Data.GraphQL.Shared/Errors.fs | 34 +++++--------- .../SchemaDefinitions.fs | 44 +++++-------------- .../ValidationTypes.fs | 1 - .../MiddlewareTests.fs | 2 +- .../FSharp.Data.GraphQL.Tests/SchemaTests.fs | 4 +- .../Variables and Inputs/InputComplexTests.fs | 4 +- .../InputObjectValidatorTests.fs | 4 +- ...OptionalsNormalizationTests.ValidString.fs | 4 +- .../OptionalsNormalizationTests.fs | 4 +- 18 files changed, 42 insertions(+), 115 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index c288b4a5a..6434951af 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -139,23 +139,11 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> - match errs |> List.choose (fun x -> x.Exception) |> List.tryHead with - | Some ex -> - logger.LogError( - ex, - "Error while processing request that generated response with documentId '{documentId}'", - documentId - ) - | None -> - logger.LogWarning( - ( "Produced request error GraphQL response with:\n" - + "- documentId: '{documentId}'\n" - + "- error(s):\n {requestError}\n" - + "- metadata:\n {metadata}\n" ), - documentId, - errs, - metadata - ) + logger.LogWarning( + $"Produced request error GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", + documentId, + metadata + ) GQLResponse.RequestError(documentId, errs) diff --git a/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs b/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs index a2444ffed..b744df81c 100644 --- a/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs +++ b/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs @@ -47,7 +47,7 @@ type internal QueryWeightMiddleware(threshold : float, reportToMetadata : bool) | ResolveLive info -> checkThreshold current (info :: xs) checkThreshold 0.0 fields let error (ctx : ExecutionContext) = - GQLExecutionResult.ErrorAsync(ctx.ExecutionPlan.DocumentId, "Query complexity exceeds maximum threshold. Please reduce query complexity and try again.", None, ctx.Metadata) + GQLExecutionResult.ErrorAsync(ctx.ExecutionPlan.DocumentId, "Query complexity exceeds maximum threshold. Please reduce query complexity and try again.", ctx.Metadata) let (pass, totalWeight) = measureThreshold threshold ctx.ExecutionPlan.Fields let ctx = match reportToMetadata with diff --git a/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs b/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs index e830d2603..cd4a0f18e 100644 --- a/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs +++ b/src/FSharp.Data.GraphQL.Server.Middleware/SchemaDefinitions.fs @@ -104,9 +104,7 @@ module SchemaDefinitions = | ObjectValue x -> mapInput x | NullValue -> NoFilter |> Ok // TODO: Get union case - | _ -> Error [{ new IGQLError with - member _.Message = $"'ObjectListFilter' must be defined as object but got '{x.GetType ()}'" - member _.Exception = None }] + | _ -> Error [{ new IGQLError with member _.Message = $"'ObjectListFilter' must be defined as object but got '{x.GetType ()}'" }] let private coerceObjectListFilterValue (x : obj) : ObjectListFilter option = match x with diff --git a/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs b/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs index a2d14f397..f4defba80 100644 --- a/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs +++ b/src/FSharp.Data.GraphQL.Server/ErrorTypes.fs @@ -29,7 +29,6 @@ type internal CoercionError = { interface IGQLError with member this.Message = this.Message - member this.Exception = None interface IGQLErrorExtensions with @@ -79,7 +78,6 @@ type internal CoercionErrorWrapper = { interface IGQLError with member this.Message = this.InnerError.Message - member this.Exception = this.InnerError.Exception interface IGQLErrorExtensions with diff --git a/src/FSharp.Data.GraphQL.Server/Exceptions.fs b/src/FSharp.Data.GraphQL.Server/Exceptions.fs index f4198c99a..28ed75f94 100644 --- a/src/FSharp.Data.GraphQL.Server/Exceptions.fs +++ b/src/FSharp.Data.GraphQL.Server/Exceptions.fs @@ -21,7 +21,6 @@ type GQLMessageExceptionBase (errorKind, msg, [] extensions) = inherit GraphQLException (msg) interface IGQLError with member _.Message = msg - member this.Exception = Some this interface IGQLErrorExtensions with member _.Extensions = match extensions with diff --git a/src/FSharp.Data.GraphQL.Server/Executor.fs b/src/FSharp.Data.GraphQL.Server/Executor.fs index 32c0f969c..6ad5b8288 100644 --- a/src/FSharp.Data.GraphQL.Server/Executor.fs +++ b/src/FSharp.Data.GraphQL.Server/Executor.fs @@ -123,7 +123,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s return prepareOutput res with | :? GQLMessageException as ex -> return prepareOutput(GQLExecutionResult.Error (documentId, ex, executionPlan.Metadata)) - | ex -> return prepareOutput (GQLExecutionResult.Error(documentId, ex.ToString(), Some ex, executionPlan.Metadata)) // TODO: Handle better + | ex -> return prepareOutput (GQLExecutionResult.Error(documentId, ex.ToString(), executionPlan.Metadata)) // TODO: Handle better } let execute (executionPlan: ExecutionPlan, data: 'Root option, variables: ImmutableDictionary option) = @@ -143,7 +143,6 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s | Some m -> Ok m | None -> Error <| [ GQLProblemDetails.CreateWithKind ( "Operation to be executed is of type mutation, but no mutation root object was defined in current schema", - None, ErrorKind.Validation )] | Subscription -> @@ -151,7 +150,6 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s | Some s -> Ok <| upcast s | None -> Error <| [ GQLProblemDetails.CreateWithKind ( "Operation to be executed is of type subscription, but no subscription root object was defined in the current schema", - None, ErrorKind.Validation )] do! @@ -169,7 +167,6 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s return runMiddlewares (fun x -> x.PlanOperation) planningCtx planOperation | None -> return! Error <| [ GQLProblemDetails.CreateWithKind ( "No operation with specified name has been found for provided document", - None, ErrorKind.Validation )] } diff --git a/src/FSharp.Data.GraphQL.Server/IO.fs b/src/FSharp.Data.GraphQL.Server/IO.fs index 8d097f44b..b9fe6e73b 100644 --- a/src/FSharp.Data.GraphQL.Server/IO.fs +++ b/src/FSharp.Data.GraphQL.Server/IO.fs @@ -56,12 +56,12 @@ type GQLExecutionResult = GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.OfError error ], meta) static member Error(documentId, errors, meta) = GQLExecutionResult.RequestError(documentId, errors |> List.map GQLProblemDetails.OfError, meta) - static member Error(documentId, msg, ex : Exception option, meta) = - GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create (msg, ex) ], meta) + static member Error(documentId, msg, meta) = + GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create msg ], meta) static member Invalid(documentId, errors, meta) = GQLExecutionResult.RequestError(documentId, errors, meta) - static member ErrorAsync(documentId, msg : string, ex : Exception option, meta) = - AsyncVal.wrap (GQLExecutionResult.Error (documentId, msg, ex, meta)) + static member ErrorAsync(documentId, msg : string, meta) = + AsyncVal.wrap (GQLExecutionResult.Error (documentId, msg, meta)) static member ErrorAsync(documentId, error : IGQLError, meta) = AsyncVal.wrap (GQLExecutionResult.Error (documentId, error, meta)) diff --git a/src/FSharp.Data.GraphQL.Server/Schema.fs b/src/FSharp.Data.GraphQL.Server/Schema.fs index 7fa9b4887..ba44fdceb 100644 --- a/src/FSharp.Data.GraphQL.Server/Schema.fs +++ b/src/FSharp.Data.GraphQL.Server/Schema.fs @@ -133,9 +133,7 @@ type SchemaConfig = fun path ex -> match ex with | :? GQLMessageException as ex -> [ex] - | ex -> [{ new IGQLError with - member _.Message = ex.Message - member _.Exception = Some ex }] + | ex -> [{ new IGQLError with member _.Message = ex.Message }] SubscriptionProvider = SchemaConfig.DefaultSubscriptionProvider() LiveFieldSubscriptionProvider = SchemaConfig.DefaultLiveFieldSubscriptionProvider() JsonOptions = JsonFSharpOptions.Default().ToJsonSerializerOptions() } diff --git a/src/FSharp.Data.GraphQL.Server/Values.fs b/src/FSharp.Data.GraphQL.Server/Values.fs index ab46f6857..cddd8c0c7 100644 --- a/src/FSharp.Data.GraphQL.Server/Values.fs +++ b/src/FSharp.Data.GraphQL.Server/Values.fs @@ -209,9 +209,7 @@ let rec internal compileByType (inputObjectPath: FieldPath) (inputSource : Input return found else Debugger.Break() - return! Error [{ new IGQLError with - member _.Message = $"A variable '${variableName}' is not an object" - member _.Exception = None }] + return! Error [{ new IGQLError with member _.Message = $"A variable '${variableName}' is not an object" }] | false, _ -> return null } | _ -> Ok null @@ -269,9 +267,7 @@ let rec internal compileByType (inputObjectPath: FieldPath) (inputSource : Input | VariableName variableName -> match variables.TryGetValue variableName with | true, var -> Ok var - | false, _ -> Error [ { new IGQLError with - member _.Message = $"A variable '${variableName}' not found" - member _.Exception = None } ] + | false, _ -> Error [ { new IGQLError with member _.Message = $"A variable '${variableName}' not found" } ] | _ -> result { let! coerced = coerceEnumInput value diff --git a/src/FSharp.Data.GraphQL.Shared/Errors.fs b/src/FSharp.Data.GraphQL.Shared/Errors.fs index 39d15d367..a408f1d85 100644 --- a/src/FSharp.Data.GraphQL.Shared/Errors.fs +++ b/src/FSharp.Data.GraphQL.Shared/Errors.fs @@ -11,7 +11,6 @@ type internal FieldPath = obj list type IGQLError = abstract member Message : string with get - abstract member Exception : (Exception option) with get type internal ICoerceGQLError = inherit IGQLError @@ -67,12 +66,6 @@ type GQLProblemDetails = { [] Message : string - /// - /// The possible exception associated with this error. It won't be serialized. - /// - [] - Exception : Exception option - /// /// An array of fields path segments that that identify the specific field path in a GraphQL query where the resolution problem occurs. /// @@ -134,57 +127,50 @@ type GQLProblemDetails = { | :? IReadOnlyDictionary as extensions -> extensions | _ -> ReadOnlyDictionary mutableExtensions - static member internal CreateWithKind (message : string, ex : Exception option, kind : ErrorKind, ?path) = { + static member internal CreateWithKind (message, kind : ErrorKind, ?path) = { Message = message - Exception = ex Path = path |> Skippable.ofOption Locations = Skip Extensions = Dictionary 1 |> GQLProblemDetails.SetErrorKind kind |> Include } - static member Create (message : string, ex: Exception option, ?extensions : IReadOnlyDictionary) = { + static member Create (message, ?extensions : IReadOnlyDictionary) = { Message = message - Exception = ex Path = Skip Locations = Skip Extensions = extensions |> Skippable.ofOption } - static member Create (message : string, ex: Exception option, extensions : Skippable>) = { + static member Create (message, extensions) = { Message = message - Exception = ex Path = Skip Locations = Skip Extensions = extensions } - static member Create (message : string, ex: Exception option, path : FieldPath, ?extensions : IReadOnlyDictionary) = { + static member Create (message, path, ?extensions : IReadOnlyDictionary) = { Message = message - Exception = ex Path = Include path Locations = Skip Extensions = extensions |> Skippable.ofOption } - static member Create (message : string, ex: Exception option, path : FieldPath, extensions : Skippable>) = { + static member Create (message, path, extensions) = { Message = message - Exception = ex Path = Include path Locations = Skip Extensions = extensions } - static member Create (message : string, ex: Exception option, locations : GQLProblemLocation list, ?extensions : IReadOnlyDictionary) = { + static member Create (message, locations, ?extensions : IReadOnlyDictionary) = { Message = message - Exception = ex Path = Skip Locations = Include locations Extensions = extensions |> Skippable.ofOption } - static member Create (message : string, ex: Exception option, locations : GQLProblemLocation list, extensions : Skippable>) = { + static member Create (message, locations, extensions) = { Message = message - Exception = ex Path = Skip Locations = Include locations Extensions = extensions @@ -201,7 +187,7 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, error.Exception, extensions) + GQLProblemDetails.Create (message, extensions) static member OfFieldError (path : FieldPath) (error : IGQLError) = let extensions = @@ -214,7 +200,7 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, error.Exception, path, extensions) + GQLProblemDetails.Create (message, path, extensions) static member internal OfFieldExecutionError (path : FieldPath) (error : IGQLError) = let extensions = @@ -230,7 +216,7 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, error.Exception, path, extensions) + GQLProblemDetails.Create (message, path, extensions) override this.GetHashCode () = let extensionsHashCode = diff --git a/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs b/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs index 995d04d1f..5f5275b3d 100644 --- a/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs +++ b/src/FSharp.Data.GraphQL.Shared/SchemaDefinitions.fs @@ -31,9 +31,7 @@ module SchemaDefinitions = | NullValue -> $"Inline value 'null' cannot be converted into {destinationType}" | EnumValue value -> getMessage "enum" value | value -> raise <| NotSupportedException $"{value} cannot be passed as scalar input" - Error [{ new IGQLError with - member _.Message = message - member _.Exception = None }] + Error [{ new IGQLError with member _.Message = message }] member inputValue.GetCoerceRangeError(destinationType, minValue, maxValue) = let getMessage inputType value = $"Inline value '{value}' of type %s{inputType} cannot be converted into %s{destinationType} of range from {minValue} to {maxValue}" @@ -46,33 +44,23 @@ module SchemaDefinitions = | NullValue -> $"Inline value 'null' cannot be converted into {destinationType}" | EnumValue value -> getMessage "enum" value | value -> raise <| NotSupportedException $"{value} cannot be passed as scalar input" - Error [{ new IGQLError with - member _.Message = message - member _.Exception = None }] + Error [{ new IGQLError with member _.Message = message }] type JsonElement with member e.GetDeserializeError(destinationType, minValue, maxValue ) = let jsonValue = match e.ValueKind with JsonValueKind.String -> e.GetString() | _ -> e.GetRawText() - Error [{ new IGQLError with - member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType} of range from {minValue} to {maxValue}" - member _.Exception = None }] + Error [{ new IGQLError with member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType} of range from {minValue} to {maxValue}" }] member e.GetDeserializeError(destinationType) = let jsonValue = match e.ValueKind with JsonValueKind.String -> e.GetString() | _ -> e.GetRawText() - Error [{ new IGQLError with - member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType}" - member _.Exception = None }] + Error [{ new IGQLError with member _.Message = $"JSON value '{jsonValue}' of kind '{e.ValueKind}' cannot be deserialized into %s{destinationType}" }] let getParseRangeError (destinationType, minValue, maxValue) value = - Error [{ new IGQLError with - member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType} of range from {minValue} to {maxValue}" - member _.Exception = None }] + Error [{ new IGQLError with member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType} of range from {minValue} to {maxValue}" }] let getParseError destinationType value = - Error [{ new IGQLError with - member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType}" - member _.Exception = None }] + Error [{ new IGQLError with member _.Message = $"Inline value '%s{value}' cannot be parsed into %s{destinationType}" }] open System.Globalization @@ -381,9 +369,7 @@ module SchemaDefinitions = | VariableName variableName -> match variables.TryGetValue variableName with | true, value -> Ok value - | false, _ -> Error [{ new IGQLError with - member _.Message = $"A variable '$%s{variableName}' not found" - member _.Exception = None }] + | false, _ -> Error [{ new IGQLError with member _.Message = $"A variable '$%s{variableName}' not found" }] | v -> other v /// GraphQL type of int @@ -551,9 +537,7 @@ module SchemaDefinitions = coerceOutput : obj -> 'T option, ?description : string) : ScalarDefinition<'T> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with - member _.Message = msg - member _.Exception = None } |> List.singleton) + CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with member _.Message = msg } |> List.singleton) CoerceOutput = coerceOutput } /// @@ -567,9 +551,7 @@ module SchemaDefinitions = coerceOutput : obj -> 'T option, ?description : string) : ScalarDefinition<'T> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with - member _.Message = msg - member _.Exception = None })) + CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with member _.Message = msg })) CoerceOutput = coerceOutput } /// @@ -611,9 +593,7 @@ module SchemaDefinitions = coerceOutput : obj -> 'Primitive option, ?description : string) : ScalarDefinition<'Primitive, 'Wrapper> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with - member _.Message = msg - member _.Exception = None } |> List.singleton) + CoerceInput = coerceInput >> Result.mapError (fun msg -> { new IGQLError with member _.Message = msg } |> List.singleton) CoerceOutput = coerceOutput } /// @@ -627,9 +607,7 @@ module SchemaDefinitions = coerceOutput : obj -> 'Primitive option, ?description : string) : ScalarDefinition<'Primitive, 'Wrapper> = { Name = name Description = description - CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with - member _.Message = msg - member _.Exception = None })) + CoerceInput = coerceInput >> Result.mapError (List.map (fun msg -> { new IGQLError with member _.Message = msg })) CoerceOutput = coerceOutput } /// diff --git a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs index 491de1fa5..d97cb8bba 100644 --- a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs +++ b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs @@ -56,7 +56,6 @@ type AstError = [ { Message = message - Exception = None Path = path |> Skippable.ofOption |> Skippable.map List.rev Locations = Skip Extensions = diff --git a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs index d52b3c4e2..aaf31f99e 100644 --- a/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/MiddlewareTests.fs @@ -89,7 +89,7 @@ let execute (query : Document) = executor.AsyncExecute(query) |> sync let expectedErrors : GQLProblemDetails list = - [ GQLProblemDetails.Create ("Query complexity exceeds maximum threshold. Please reduce query complexity and try again.", None) ] + [ GQLProblemDetails.Create ("Query complexity exceeds maximum threshold. Please reduce query complexity and try again.") ] [] let ``Simple query: Should pass when below threshold``() = diff --git a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs index 32d9d80b3..39b0f1260 100644 --- a/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/SchemaTests.fs @@ -46,9 +46,7 @@ let ``Schema config should be able to override default error handling`` () = (fun path ex -> let i = idx idx <- idx + 1 - [ { new IGQLError with - member __.Message = i.ToString () - member __.Exception = None } ]) + [ { new IGQLError with member __.Message = i.ToString () } ]) } let TestType = Define.Object ( diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs index 5a0309f33..ecebe84e4 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputComplexTests.fs @@ -30,9 +30,7 @@ let TestComplexScalar = | InputParameterValue.InlineConstant (StringValue s) -> s if value = "SerializedValue" then Ok "DeserializedValue" - else Error { new IGQLError with - member _.Message = "" - member _.Exception = None }), + else Error { new IGQLError with member _.Message = "" }), coerceOutput = (fun value -> if value = upcast "DeserializedValue" then Some "SerializedValue" diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs index 19b6d5c63..d9bb21a16 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/InputObjectValidatorTests.fs @@ -21,9 +21,7 @@ open ErrorHelpers type InputRecord = { Country : string; ZipCode : string; City : string } let createSingleError message = - [{ new IGQLError with - member _.Message = message - member _.Exception = None }] + [{ new IGQLError with member _.Message = message }] let InputRecordType = Define.InputObject ( diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs index 4ebec160b..1bede223a 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.ValidString.fs @@ -122,9 +122,7 @@ module ValidationErrors = let toIGQLErrors (errors: ValidationErrors) : IGQLError list = errors |> ValidationErrors.toList - |> List.map (fun e -> { new IGQLError with - member _.Message = e - member _.Exception = None }) + |> List.map (fun e -> { new IGQLError with member _.Message = e }) module Operators = diff --git a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs index 6b3a354a0..1fb88fc2e 100644 --- a/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs +++ b/tests/FSharp.Data.GraphQL.Tests/Variables and Inputs/OptionalsNormalizationTests.fs @@ -148,9 +148,7 @@ open FSharp.Data.GraphQL.Validation.ValidationResult open ErrorHelpers let createSingleError message = - [{ new IGQLError with - member _.Message = message - member _.Exception = None }] + [{ new IGQLError with member _.Message = message }] type InputRecordNested = { HomeAddress : AddressRecord; WorkAddress : AddressRecord option; MailingAddress : AddressRecord voption } From 9ee25227edd40650e2d630a41d8b061908053613 Mon Sep 17 00:00:00 2001 From: valber Date: Tue, 9 Apr 2024 23:00:34 +0200 Subject: [PATCH 09/17] .AspNetCore: (2nd attempt): logging server exceptions with exception info too Note: this commit is a 2nd attempt after getting feedback from code review. There were two problems before: 1. The error message itself was not being logged, but only a mention that there was an error; 2. It was not possible to access the possibly original exception which led to the request processing error. However, it's extremely useful to log the error with the whole exception information including the stack trace of where the exception was thrown. However, these changes also introduce a breaking change as IGQLError and GQLProblemDetails now each have an additional mandatory member holding the possible exception which generated the error. These types (especiall GQLProblemDetails) are public and could be used by code using FSharp.Data.GraphQL. --- .../Giraffe/HttpHandlers.fs | 22 ++++++-- src/FSharp.Data.GraphQL.Server/Exceptions.fs | 2 + src/FSharp.Data.GraphQL.Server/Executor.fs | 2 +- src/FSharp.Data.GraphQL.Server/IO.fs | 4 ++ src/FSharp.Data.GraphQL.Server/Schema.fs | 4 +- src/FSharp.Data.GraphQL.Shared/Errors.fs | 55 ++++++++++++++++++- .../ValidationTypes.fs | 1 + 7 files changed, 80 insertions(+), 10 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index 6434951af..a1decc40d 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -139,11 +139,23 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> - logger.LogWarning( - $"Produced request error GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", - documentId, - metadata - ) + match errs |> List.choose (fun x -> x.Exception |> Option.ofValueOption) |> List.tryHead with + | Some ex -> + logger.LogError( + ex, + "Error while processing request that generated response with documentId '{documentId}'", + documentId + ) + | None -> + logger.LogWarning( + ( "Produced request error GraphQL response with:\n" + + "- documentId: '{documentId}'\n" + + "- error(s):\n {requestError}\n" + + "- metadata:\n {metadata}\n" ), + documentId, + errs, + metadata + ) GQLResponse.RequestError(documentId, errs) diff --git a/src/FSharp.Data.GraphQL.Server/Exceptions.fs b/src/FSharp.Data.GraphQL.Server/Exceptions.fs index 28ed75f94..ddf047652 100644 --- a/src/FSharp.Data.GraphQL.Server/Exceptions.fs +++ b/src/FSharp.Data.GraphQL.Server/Exceptions.fs @@ -21,6 +21,8 @@ type GQLMessageExceptionBase (errorKind, msg, [] extensions) = inherit GraphQLException (msg) interface IGQLError with member _.Message = msg + interface IGQLExceptionError with + member this.Exception = this interface IGQLErrorExtensions with member _.Extensions = match extensions with diff --git a/src/FSharp.Data.GraphQL.Server/Executor.fs b/src/FSharp.Data.GraphQL.Server/Executor.fs index 6ad5b8288..11c212675 100644 --- a/src/FSharp.Data.GraphQL.Server/Executor.fs +++ b/src/FSharp.Data.GraphQL.Server/Executor.fs @@ -123,7 +123,7 @@ type Executor<'Root>(schema: ISchema<'Root>, middlewares : IExecutorMiddleware s return prepareOutput res with | :? GQLMessageException as ex -> return prepareOutput(GQLExecutionResult.Error (documentId, ex, executionPlan.Metadata)) - | ex -> return prepareOutput (GQLExecutionResult.Error(documentId, ex.ToString(), executionPlan.Metadata)) // TODO: Handle better + | ex -> return prepareOutput (GQLExecutionResult.ErrorFromException(documentId, ex, executionPlan.Metadata)) } let execute (executionPlan: ExecutionPlan, data: 'Root option, variables: ImmutableDictionary option) = diff --git a/src/FSharp.Data.GraphQL.Server/IO.fs b/src/FSharp.Data.GraphQL.Server/IO.fs index b9fe6e73b..ef8e111a8 100644 --- a/src/FSharp.Data.GraphQL.Server/IO.fs +++ b/src/FSharp.Data.GraphQL.Server/IO.fs @@ -58,6 +58,10 @@ type GQLExecutionResult = GQLExecutionResult.RequestError(documentId, errors |> List.map GQLProblemDetails.OfError, meta) static member Error(documentId, msg, meta) = GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create msg ], meta) + + static member ErrorFromException(documentId : int, ex : Exception, meta : Metadata) = + GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.CreateOfException (ex.Message, ex) ], meta) + static member Invalid(documentId, errors, meta) = GQLExecutionResult.RequestError(documentId, errors, meta) static member ErrorAsync(documentId, msg : string, meta) = diff --git a/src/FSharp.Data.GraphQL.Server/Schema.fs b/src/FSharp.Data.GraphQL.Server/Schema.fs index ba44fdceb..10e04b81b 100644 --- a/src/FSharp.Data.GraphQL.Server/Schema.fs +++ b/src/FSharp.Data.GraphQL.Server/Schema.fs @@ -133,7 +133,9 @@ type SchemaConfig = fun path ex -> match ex with | :? GQLMessageException as ex -> [ex] - | ex -> [{ new IGQLError with member _.Message = ex.Message }] + | ex -> [{ new IGQLExceptionError with + member _.Message = ex.Message + member _.Exception = ex }] SubscriptionProvider = SchemaConfig.DefaultSubscriptionProvider() LiveFieldSubscriptionProvider = SchemaConfig.DefaultLiveFieldSubscriptionProvider() JsonOptions = JsonFSharpOptions.Default().ToJsonSerializerOptions() } diff --git a/src/FSharp.Data.GraphQL.Shared/Errors.fs b/src/FSharp.Data.GraphQL.Shared/Errors.fs index a408f1d85..133339269 100644 --- a/src/FSharp.Data.GraphQL.Shared/Errors.fs +++ b/src/FSharp.Data.GraphQL.Shared/Errors.fs @@ -12,6 +12,10 @@ type internal FieldPath = obj list type IGQLError = abstract member Message : string with get +type IGQLExceptionError = + inherit IGQLError + abstract member Exception : Exception with get + type internal ICoerceGQLError = inherit IGQLError abstract member VariableMessage : string with get @@ -66,6 +70,12 @@ type GQLProblemDetails = { [] Message : string + /// + /// The possible exception associated with this error. It won't be serialized. + /// + [] + Exception : Exception voption + /// /// An array of fields path segments that that identify the specific field path in a GraphQL query where the resolution problem occurs. /// @@ -129,6 +139,7 @@ type GQLProblemDetails = { static member internal CreateWithKind (message, kind : ErrorKind, ?path) = { Message = message + Exception = ValueNone Path = path |> Skippable.ofOption Locations = Skip Extensions = Dictionary 1 |> GQLProblemDetails.SetErrorKind kind |> Include @@ -136,6 +147,7 @@ type GQLProblemDetails = { static member Create (message, ?extensions : IReadOnlyDictionary) = { Message = message + Exception = ValueNone Path = Skip Locations = Skip Extensions = extensions |> Skippable.ofOption @@ -143,13 +155,23 @@ type GQLProblemDetails = { static member Create (message, extensions) = { Message = message + Exception = ValueNone Path = Skip Locations = Skip Extensions = extensions } + static member CreateOfException (message : string, ex : Exception, ?path : FieldPath, ?extensions : IReadOnlyDictionary) = { + Message = message + Exception = ValueSome ex + Path = path |> Skippable.ofOption + Locations = Skip + Extensions = extensions |> Skippable.ofOption + } + static member Create (message, path, ?extensions : IReadOnlyDictionary) = { Message = message + Exception = ValueNone Path = Include path Locations = Skip Extensions = extensions |> Skippable.ofOption @@ -157,6 +179,7 @@ type GQLProblemDetails = { static member Create (message, path, extensions) = { Message = message + Exception = ValueNone Path = Include path Locations = Skip Extensions = extensions @@ -164,6 +187,7 @@ type GQLProblemDetails = { static member Create (message, locations, ?extensions : IReadOnlyDictionary) = { Message = message + Exception = ValueNone Path = Skip Locations = Include locations Extensions = extensions |> Skippable.ofOption @@ -171,6 +195,7 @@ type GQLProblemDetails = { static member Create (message, locations, extensions) = { Message = message + Exception = ValueNone Path = Skip Locations = Include locations Extensions = extensions @@ -187,7 +212,15 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, extensions) + match error with + | :? IGQLExceptionError as exceptionError -> + match extensions with + | Include x -> + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, extensions = x) + | Skip -> + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception) + | _ -> + GQLProblemDetails.Create (message, extensions) static member OfFieldError (path : FieldPath) (error : IGQLError) = let extensions = @@ -200,7 +233,15 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, path, extensions) + match error with + | :? IGQLExceptionError as exceptionError -> + match extensions with + | Include x -> + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions = x) + | Skip -> + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path) + | _ -> + GQLProblemDetails.Create (message, path, extensions) static member internal OfFieldExecutionError (path : FieldPath) (error : IGQLError) = let extensions = @@ -216,7 +257,15 @@ type GQLProblemDetails = { | :? ICoerceGQLError as error -> error.VariableMessage + error.Message | _ -> error.Message - GQLProblemDetails.Create (message, path, extensions) + match error with + | :? IGQLExceptionError as exceptionError -> + match extensions with + | Include x -> + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions = x) + | Skip -> + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path) + | _ -> + GQLProblemDetails.Create (message, path, extensions) override this.GetHashCode () = let extensionsHashCode = diff --git a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs index d97cb8bba..1f86a40a4 100644 --- a/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs +++ b/src/FSharp.Data.GraphQL.Shared/ValidationTypes.fs @@ -56,6 +56,7 @@ type AstError = [ { Message = message + Exception = ValueNone Path = path |> Skippable.ofOption |> Skippable.map List.rev Locations = Skip Extensions = From 079cf5031b0c6df73a8acd109f166354ca2e8432 Mon Sep 17 00:00:00 2001 From: valber Date: Wed, 10 Apr 2024 22:35:20 +0200 Subject: [PATCH 10/17] .AspNetCore: for each possible exception logging it These changes are according to a request during code review. --- .../Giraffe/HttpHandlers.fs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index a1decc40d..9548a2c05 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -139,14 +139,19 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> - match errs |> List.choose (fun x -> x.Exception |> Option.ofValueOption) |> List.tryHead with - | Some ex -> - logger.LogError( - ex, - "Error while processing request that generated response with documentId '{documentId}'", - documentId - ) - | None -> + if errs |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) |> (not << Seq.isEmpty) + then + errs + |> List.choose(fun x -> x.Exception |> Option.ofValueOption) + |> List.iter + (fun ex -> + logger.LogError( + ex, + "Error while processing request that generated response with documentId '{documentId}'", + documentId + ) + ) + else logger.LogWarning( ( "Produced request error GraphQL response with:\n" + "- documentId: '{documentId}'\n" From 4c1a49096e2e18afa49284b1439b80dc27bbfeb6 Mon Sep 17 00:00:00 2001 From: valber Date: Wed, 10 Apr 2024 23:04:21 +0200 Subject: [PATCH 11/17] Removed duplicate fantomas config. value (was causing problems) --- .editorconfig | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.editorconfig b/.editorconfig index 1747b10b2..9f6b4d2d3 100644 --- a/.editorconfig +++ b/.editorconfig @@ -245,10 +245,6 @@ fsharp_multi_line_lambda_closing_newline=false # default false fsharp_keep_indent_in_branch=true -# multiline, nested expressions must be surrounded by blank lines -# default true -fsharp_blank_lines_around_nested_multiline_expressions=false - # whether a bar is placed before DU # false: type MyDU = Short of int # true: type MyDU = | Short of int From 273796ad88025fcc9597d907589a3d46c6d786cc Mon Sep 17 00:00:00 2001 From: valber Date: Wed, 10 Apr 2024 23:06:15 +0200 Subject: [PATCH 12/17] .AspNetCore: formatting HttpHandlers with Fantomas --- .../Giraffe/HttpHandlers.fs | 322 +++++++++--------- 1 file changed, 156 insertions(+), 166 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index 9548a2c05..35d7b4e05 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -21,48 +21,47 @@ type HttpHandler = HttpFunc -> HttpContext -> HttpFuncResult module HttpHandlers = - let [] internal IndentedOptionsName = "Indented" + [] + let internal IndentedOptionsName = "Indented" let rec private moduleType = getModuleType <@ moduleType @> - let ofTaskIResult ctx (taskRes: Task) : HttpFuncResult = task { - let! res = taskRes - do! res.ExecuteAsync(ctx) - return Some ctx - } + let ofTaskIResult ctx (taskRes : Task) : HttpFuncResult = + task { + let! res = taskRes + do! res.ExecuteAsync (ctx) + return Some ctx + } - let ofTaskIResult2 ctx (taskRes: Task>) : HttpFuncResult = - taskRes - |> TaskResult.defaultWith id - |> ofTaskIResult ctx + let ofTaskIResult2 ctx (taskRes : Task>) : HttpFuncResult = taskRes |> TaskResult.defaultWith id |> ofTaskIResult ctx let private handleGraphQL<'Root> (next : HttpFunc) (ctx : HttpContext) = let sp = ctx.RequestServices let logger = sp.CreateLogger moduleType - let options = sp.GetRequiredService>>() + let options = sp.GetRequiredService>> () let toResponse { DocumentId = documentId; Content = content; Metadata = metadata } = let serializeIndented value = let jsonSerializerOptions = options.Get(IndentedOptionsName).SerializerOptions - JsonSerializer.Serialize(value, jsonSerializerOptions) + JsonSerializer.Serialize (value, jsonSerializerOptions) match content with - | Direct(data, errs) -> - logger.LogDebug( + | Direct (data, errs) -> + logger.LogDebug ( $"Produced direct GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace($"GraphQL response data:\n:{{data}}", serializeIndented data) + logger.LogTrace ($"GraphQL response data:\n:{{data}}", serializeIndented data) - GQLResponse.Direct(documentId, data, errs) - | Deferred(data, errs, deferred) -> - logger.LogDebug( + GQLResponse.Direct (documentId, data, errs) + | Deferred (data, errs, deferred) -> + logger.LogDebug ( $"Produced deferred GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata @@ -71,41 +70,32 @@ module HttpHandlers = if logger.IsEnabled LogLevel.Debug then deferred |> Observable.add (function - | DeferredResult(data, path) -> - logger.LogDebug( - "Produced GraphQL deferred result for path: {path}", - path |> Seq.map string |> Seq.toArray |> Path.Join - ) + | DeferredResult (data, path) -> + logger.LogDebug ("Produced GraphQL deferred result for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( - $"GraphQL deferred data:\n{{data}}", - serializeIndented data - ) - | DeferredErrors(null, errors, path) -> - logger.LogDebug( - "Produced GraphQL deferred errors for path: {path}", - path |> Seq.map string |> Seq.toArray |> Path.Join - ) + logger.LogTrace ($"GraphQL deferred data:\n{{data}}", serializeIndented data) + | DeferredErrors (null, errors, path) -> + logger.LogDebug ("Produced GraphQL deferred errors for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace($"GraphQL deferred errors:\n{{errors}}", errors) - | DeferredErrors(data, errors, path) -> - logger.LogDebug( + logger.LogTrace ($"GraphQL deferred errors:\n{{errors}}", errors) + | DeferredErrors (data, errors, path) -> + logger.LogDebug ( "Produced GraphQL deferred result with errors for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( + logger.LogTrace ( $"GraphQL deferred errors:\n{{errors}}\nGraphQL deferred data:\n{{data}}", errors, serializeIndented data )) - GQLResponse.Direct(documentId, data, errs) + GQLResponse.Direct (documentId, data, errs) | Stream stream -> - logger.LogDebug( + logger.LogDebug ( $"Produced stream GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata @@ -115,23 +105,20 @@ module HttpHandlers = stream |> Observable.add (function | SubscriptionResult data -> - logger.LogDebug("Produced GraphQL subscription result") + logger.LogDebug ("Produced GraphQL subscription result") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( - $"GraphQL subscription data:\n{{data}}", - serializeIndented data - ) - | SubscriptionErrors(null, errors) -> - logger.LogDebug("Produced GraphQL subscription errors") + logger.LogTrace ($"GraphQL subscription data:\n{{data}}", serializeIndented data) + | SubscriptionErrors (null, errors) -> + logger.LogDebug ("Produced GraphQL subscription errors") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace($"GraphQL subscription errors:\n{{errors}}", errors) - | SubscriptionErrors(data, errors) -> - logger.LogDebug("Produced GraphQL subscription result with errors") + logger.LogTrace ($"GraphQL subscription errors:\n{{errors}}", errors) + | SubscriptionErrors (data, errors) -> + logger.LogDebug ("Produced GraphQL subscription result with errors") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( + logger.LogTrace ( $"GraphQL subscription errors:\n{{errors}}\nGraphQL deferred data:\n{{data}}", errors, serializeIndented data @@ -139,141 +126,144 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> - if errs |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) |> (not << Seq.isEmpty) - then - errs - |> List.choose(fun x -> x.Exception |> Option.ofValueOption) - |> List.iter - (fun ex -> - logger.LogError( - ex, - "Error while processing request that generated response with documentId '{documentId}'", - documentId - ) - ) - else - logger.LogWarning( - ( "Produced request error GraphQL response with:\n" - + "- documentId: '{documentId}'\n" - + "- error(s):\n {requestError}\n" - + "- metadata:\n {metadata}\n" ), - documentId, - errs, - metadata - ) - GQLResponse.RequestError(documentId, errs) + if errs + |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) + |> (not << Seq.isEmpty) then + errs + |> List.choose (fun x -> x.Exception |> Option.ofValueOption) + |> List.iter (fun ex -> + logger.LogError (ex, "Error while processing request that generated response with documentId '{documentId}'", documentId)) + else + logger.LogWarning ( + ("Produced request error GraphQL response with:\n" + + "- documentId: '{documentId}'\n" + + "- error(s):\n {requestError}\n" + + "- metadata:\n {metadata}\n"), + documentId, + errs, + metadata + ) + + GQLResponse.RequestError (documentId, errs) /// Checks if the request contains a body - let checkIfHasBody (request: HttpRequest) = task { - if request.Body.CanSeek then - return (request.Body.Length > 0L) - else - request.EnableBuffering() - let body = request.Body - let buffer = Array.zeroCreate 1 - let! bytesRead = body.ReadAsync(buffer, 0, 1) - body.Seek(0, SeekOrigin.Begin) |> ignore - return bytesRead > 0 - } + let checkIfHasBody (request : HttpRequest) = + task { + if request.Body.CanSeek then + return (request.Body.Length > 0L) + else + request.EnableBuffering () + let body = request.Body + let buffer = Array.zeroCreate 1 + let! bytesRead = body.ReadAsync (buffer, 0, 1) + body.Seek (0, SeekOrigin.Begin) |> ignore + return bytesRead > 0 + } /// Check if the request is an introspection query /// by first checking on such properties as `GET` method or `empty request body` /// and lastly by parsing document AST for introspection operation definition. /// /// Result of check of - let checkOperationType (ctx: HttpContext) = taskResult { - - let checkAnonymousFieldsOnly (ctx: HttpContext) = taskResult { - let! gqlRequest = ctx.TryBindJsonAsync(GQLRequestContent.expectedJSON) - let! ast = Parser.parseOrIResult ctx.Request.Path.Value gqlRequest.Query - let operationName = gqlRequest.OperationName |> Skippable.toOption - - let createParsedContent() = { - Query = gqlRequest.Query - Ast = ast - OperationName = gqlRequest.OperationName - Variables = gqlRequest.Variables - } - if ast.IsEmpty then - logger.LogTrace( - "Request is not GET, but 'query' field is an empty string. Must be an introspection query" - ) + let checkOperationType (ctx : HttpContext) = + taskResult { + + let checkAnonymousFieldsOnly (ctx : HttpContext) = + taskResult { + let! gqlRequest = ctx.TryBindJsonAsync (GQLRequestContent.expectedJSON) + let! ast = Parser.parseOrIResult ctx.Request.Path.Value gqlRequest.Query + let operationName = gqlRequest.OperationName |> Skippable.toOption + + let createParsedContent () = + { Query = gqlRequest.Query + Ast = ast + OperationName = gqlRequest.OperationName + Variables = gqlRequest.Variables } + if ast.IsEmpty then + logger.LogTrace ("Request is not GET, but 'query' field is an empty string. Must be an introspection query") + return IntrospectionQuery <| ValueNone + else + match Ast.findOperationByName operationName ast with + | None -> + logger.LogTrace "Document has no operation" + return IntrospectionQuery <| ValueNone + | Some op -> + if not (op.OperationType = Ast.Query) then + logger.LogTrace "Document operation is not of type Query" + return createParsedContent () |> OperationQuery + else + let hasNonMetaFields = + Ast.containsFieldsBeyond + Ast.metaTypeFields + (fun x -> logger.LogTrace ($"Operation Selection in Field with name: {{fieldName}}", x.Name)) + (fun _ -> logger.LogTrace "Operation Selection is non-Field type") + op + + if hasNonMetaFields then + return createParsedContent () |> OperationQuery + else + return IntrospectionQuery <| ValueSome ast + } + + let request = ctx.Request + + if HttpMethods.Get = request.Method then + logger.LogTrace ("Request is GET. Must be an introspection query") return IntrospectionQuery <| ValueNone else - match Ast.findOperationByName operationName ast with - | None -> - logger.LogTrace "Document has no operation" + let! hasBody = checkIfHasBody request + + if not hasBody then + logger.LogTrace ("Request is not GET, but has no body. Must be an introspection query") return IntrospectionQuery <| ValueNone - | Some op -> - if not (op.OperationType = Ast.Query) then - logger.LogTrace "Document operation is not of type Query" - return createParsedContent () |> OperationQuery - else - let hasNonMetaFields = - Ast.containsFieldsBeyond - Ast.metaTypeFields - (fun x -> - logger.LogTrace($"Operation Selection in Field with name: {{fieldName}}", x.Name)) - (fun _ -> logger.LogTrace "Operation Selection is non-Field type") - op - - if hasNonMetaFields then - return createParsedContent() |> OperationQuery - else - return IntrospectionQuery <| ValueSome ast + else + return! checkAnonymousFieldsOnly ctx } - let request = ctx.Request - - if HttpMethods.Get = request.Method then - logger.LogTrace("Request is GET. Must be an introspection query") - return IntrospectionQuery <| ValueNone - else - let! hasBody = checkIfHasBody request - - if not hasBody then - logger.LogTrace("Request is not GET, but has no body. Must be an introspection query") - return IntrospectionQuery <| ValueNone - else - return! checkAnonymousFieldsOnly ctx - } - /// Execute default or custom introspection query - let executeIntrospectionQuery (executor: Executor<_>) (ast: Ast.Document voption) = task { - let! result = - match ast with - | ValueNone -> executor.AsyncExecute IntrospectionQuery.Definition - | ValueSome ast -> executor.AsyncExecute ast - - let response = result |> toResponse - return Results.Ok response - } + let executeIntrospectionQuery (executor : Executor<_>) (ast : Ast.Document voption) = + task { + let! result = + match ast with + | ValueNone -> executor.AsyncExecute IntrospectionQuery.Definition + | ValueSome ast -> executor.AsyncExecute ast + + let response = result |> toResponse + return Results.Ok response + } /// Execute the operation for given request - let executeOperation (executor: Executor<_>) content = task { - let operationName = content.OperationName |> Skippable.filter (not << isNull) |> Skippable.toOption - let variables = content.Variables |> Skippable.filter (not << isNull) |> Skippable.toOption - - operationName - |> Option.iter (fun on -> logger.LogTrace("GraphQL operation name: '{operationName}'", on)) - - logger.LogTrace($"Executing GraphQL query:\n{{query}}", content.Query) - - variables - |> Option.iter (fun v -> logger.LogTrace($"GraphQL variables:\n{{variables}}", v)) - - let root = options.CurrentValue.RootFactory ctx - - let! result = - Async.StartAsTask( - executor.AsyncExecute(content.Ast, root, ?variables = variables, ?operationName = operationName), - cancellationToken = ctx.RequestAborted - ) + let executeOperation (executor : Executor<_>) content = + task { + let operationName = + content.OperationName + |> Skippable.filter (not << isNull) + |> Skippable.toOption + let variables = + content.Variables + |> Skippable.filter (not << isNull) + |> Skippable.toOption + + operationName + |> Option.iter (fun on -> logger.LogTrace ("GraphQL operation name: '{operationName}'", on)) + + logger.LogTrace ($"Executing GraphQL query:\n{{query}}", content.Query) + + variables + |> Option.iter (fun v -> logger.LogTrace ($"GraphQL variables:\n{{variables}}", v)) + + let root = options.CurrentValue.RootFactory ctx + + let! result = + Async.StartAsTask ( + executor.AsyncExecute (content.Ast, root, ?variables = variables, ?operationName = operationName), + cancellationToken = ctx.RequestAborted + ) - let response = result |> toResponse - return Results.Ok response - } + let response = result |> toResponse + return Results.Ok response + } if ctx.RequestAborted.IsCancellationRequested then Task.FromResult None From 11e3c3802467babb1c47333dfe3d0fbf952d5b19 Mon Sep 17 00:00:00 2001 From: valber Date: Thu, 11 Apr 2024 19:30:22 +0200 Subject: [PATCH 13/17] Revert ".AspNetCore: formatting HttpHandlers with Fantomas" This reverts commit 273796ad88025fcc9597d907589a3d46c6d786cc. --- .../Giraffe/HttpHandlers.fs | 322 +++++++++--------- 1 file changed, 166 insertions(+), 156 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index 35d7b4e05..9548a2c05 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -21,47 +21,48 @@ type HttpHandler = HttpFunc -> HttpContext -> HttpFuncResult module HttpHandlers = - [] - let internal IndentedOptionsName = "Indented" + let [] internal IndentedOptionsName = "Indented" let rec private moduleType = getModuleType <@ moduleType @> - let ofTaskIResult ctx (taskRes : Task) : HttpFuncResult = - task { - let! res = taskRes - do! res.ExecuteAsync (ctx) - return Some ctx - } + let ofTaskIResult ctx (taskRes: Task) : HttpFuncResult = task { + let! res = taskRes + do! res.ExecuteAsync(ctx) + return Some ctx + } - let ofTaskIResult2 ctx (taskRes : Task>) : HttpFuncResult = taskRes |> TaskResult.defaultWith id |> ofTaskIResult ctx + let ofTaskIResult2 ctx (taskRes: Task>) : HttpFuncResult = + taskRes + |> TaskResult.defaultWith id + |> ofTaskIResult ctx let private handleGraphQL<'Root> (next : HttpFunc) (ctx : HttpContext) = let sp = ctx.RequestServices let logger = sp.CreateLogger moduleType - let options = sp.GetRequiredService>> () + let options = sp.GetRequiredService>>() let toResponse { DocumentId = documentId; Content = content; Metadata = metadata } = let serializeIndented value = let jsonSerializerOptions = options.Get(IndentedOptionsName).SerializerOptions - JsonSerializer.Serialize (value, jsonSerializerOptions) + JsonSerializer.Serialize(value, jsonSerializerOptions) match content with - | Direct (data, errs) -> - logger.LogDebug ( + | Direct(data, errs) -> + logger.LogDebug( $"Produced direct GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace ($"GraphQL response data:\n:{{data}}", serializeIndented data) + logger.LogTrace($"GraphQL response data:\n:{{data}}", serializeIndented data) - GQLResponse.Direct (documentId, data, errs) - | Deferred (data, errs, deferred) -> - logger.LogDebug ( + GQLResponse.Direct(documentId, data, errs) + | Deferred(data, errs, deferred) -> + logger.LogDebug( $"Produced deferred GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata @@ -70,32 +71,41 @@ module HttpHandlers = if logger.IsEnabled LogLevel.Debug then deferred |> Observable.add (function - | DeferredResult (data, path) -> - logger.LogDebug ("Produced GraphQL deferred result for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join) + | DeferredResult(data, path) -> + logger.LogDebug( + "Produced GraphQL deferred result for path: {path}", + path |> Seq.map string |> Seq.toArray |> Path.Join + ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace ($"GraphQL deferred data:\n{{data}}", serializeIndented data) - | DeferredErrors (null, errors, path) -> - logger.LogDebug ("Produced GraphQL deferred errors for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join) + logger.LogTrace( + $"GraphQL deferred data:\n{{data}}", + serializeIndented data + ) + | DeferredErrors(null, errors, path) -> + logger.LogDebug( + "Produced GraphQL deferred errors for path: {path}", + path |> Seq.map string |> Seq.toArray |> Path.Join + ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace ($"GraphQL deferred errors:\n{{errors}}", errors) - | DeferredErrors (data, errors, path) -> - logger.LogDebug ( + logger.LogTrace($"GraphQL deferred errors:\n{{errors}}", errors) + | DeferredErrors(data, errors, path) -> + logger.LogDebug( "Produced GraphQL deferred result with errors for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace ( + logger.LogTrace( $"GraphQL deferred errors:\n{{errors}}\nGraphQL deferred data:\n{{data}}", errors, serializeIndented data )) - GQLResponse.Direct (documentId, data, errs) + GQLResponse.Direct(documentId, data, errs) | Stream stream -> - logger.LogDebug ( + logger.LogDebug( $"Produced stream GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata @@ -105,20 +115,23 @@ module HttpHandlers = stream |> Observable.add (function | SubscriptionResult data -> - logger.LogDebug ("Produced GraphQL subscription result") + logger.LogDebug("Produced GraphQL subscription result") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace ($"GraphQL subscription data:\n{{data}}", serializeIndented data) - | SubscriptionErrors (null, errors) -> - logger.LogDebug ("Produced GraphQL subscription errors") + logger.LogTrace( + $"GraphQL subscription data:\n{{data}}", + serializeIndented data + ) + | SubscriptionErrors(null, errors) -> + logger.LogDebug("Produced GraphQL subscription errors") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace ($"GraphQL subscription errors:\n{{errors}}", errors) - | SubscriptionErrors (data, errors) -> - logger.LogDebug ("Produced GraphQL subscription result with errors") + logger.LogTrace($"GraphQL subscription errors:\n{{errors}}", errors) + | SubscriptionErrors(data, errors) -> + logger.LogDebug("Produced GraphQL subscription result with errors") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace ( + logger.LogTrace( $"GraphQL subscription errors:\n{{errors}}\nGraphQL deferred data:\n{{data}}", errors, serializeIndented data @@ -126,144 +139,141 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> + if errs |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) |> (not << Seq.isEmpty) + then + errs + |> List.choose(fun x -> x.Exception |> Option.ofValueOption) + |> List.iter + (fun ex -> + logger.LogError( + ex, + "Error while processing request that generated response with documentId '{documentId}'", + documentId + ) + ) + else + logger.LogWarning( + ( "Produced request error GraphQL response with:\n" + + "- documentId: '{documentId}'\n" + + "- error(s):\n {requestError}\n" + + "- metadata:\n {metadata}\n" ), + documentId, + errs, + metadata + ) - if errs - |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) - |> (not << Seq.isEmpty) then - errs - |> List.choose (fun x -> x.Exception |> Option.ofValueOption) - |> List.iter (fun ex -> - logger.LogError (ex, "Error while processing request that generated response with documentId '{documentId}'", documentId)) - else - logger.LogWarning ( - ("Produced request error GraphQL response with:\n" - + "- documentId: '{documentId}'\n" - + "- error(s):\n {requestError}\n" - + "- metadata:\n {metadata}\n"), - documentId, - errs, - metadata - ) - - GQLResponse.RequestError (documentId, errs) + GQLResponse.RequestError(documentId, errs) /// Checks if the request contains a body - let checkIfHasBody (request : HttpRequest) = - task { - if request.Body.CanSeek then - return (request.Body.Length > 0L) - else - request.EnableBuffering () - let body = request.Body - let buffer = Array.zeroCreate 1 - let! bytesRead = body.ReadAsync (buffer, 0, 1) - body.Seek (0, SeekOrigin.Begin) |> ignore - return bytesRead > 0 - } + let checkIfHasBody (request: HttpRequest) = task { + if request.Body.CanSeek then + return (request.Body.Length > 0L) + else + request.EnableBuffering() + let body = request.Body + let buffer = Array.zeroCreate 1 + let! bytesRead = body.ReadAsync(buffer, 0, 1) + body.Seek(0, SeekOrigin.Begin) |> ignore + return bytesRead > 0 + } /// Check if the request is an introspection query /// by first checking on such properties as `GET` method or `empty request body` /// and lastly by parsing document AST for introspection operation definition. /// /// Result of check of - let checkOperationType (ctx : HttpContext) = - taskResult { - - let checkAnonymousFieldsOnly (ctx : HttpContext) = - taskResult { - let! gqlRequest = ctx.TryBindJsonAsync (GQLRequestContent.expectedJSON) - let! ast = Parser.parseOrIResult ctx.Request.Path.Value gqlRequest.Query - let operationName = gqlRequest.OperationName |> Skippable.toOption - - let createParsedContent () = - { Query = gqlRequest.Query - Ast = ast - OperationName = gqlRequest.OperationName - Variables = gqlRequest.Variables } - if ast.IsEmpty then - logger.LogTrace ("Request is not GET, but 'query' field is an empty string. Must be an introspection query") - return IntrospectionQuery <| ValueNone - else - match Ast.findOperationByName operationName ast with - | None -> - logger.LogTrace "Document has no operation" - return IntrospectionQuery <| ValueNone - | Some op -> - if not (op.OperationType = Ast.Query) then - logger.LogTrace "Document operation is not of type Query" - return createParsedContent () |> OperationQuery - else - let hasNonMetaFields = - Ast.containsFieldsBeyond - Ast.metaTypeFields - (fun x -> logger.LogTrace ($"Operation Selection in Field with name: {{fieldName}}", x.Name)) - (fun _ -> logger.LogTrace "Operation Selection is non-Field type") - op - - if hasNonMetaFields then - return createParsedContent () |> OperationQuery - else - return IntrospectionQuery <| ValueSome ast - } - - let request = ctx.Request - - if HttpMethods.Get = request.Method then - logger.LogTrace ("Request is GET. Must be an introspection query") + let checkOperationType (ctx: HttpContext) = taskResult { + + let checkAnonymousFieldsOnly (ctx: HttpContext) = taskResult { + let! gqlRequest = ctx.TryBindJsonAsync(GQLRequestContent.expectedJSON) + let! ast = Parser.parseOrIResult ctx.Request.Path.Value gqlRequest.Query + let operationName = gqlRequest.OperationName |> Skippable.toOption + + let createParsedContent() = { + Query = gqlRequest.Query + Ast = ast + OperationName = gqlRequest.OperationName + Variables = gqlRequest.Variables + } + if ast.IsEmpty then + logger.LogTrace( + "Request is not GET, but 'query' field is an empty string. Must be an introspection query" + ) return IntrospectionQuery <| ValueNone else - let! hasBody = checkIfHasBody request - - if not hasBody then - logger.LogTrace ("Request is not GET, but has no body. Must be an introspection query") + match Ast.findOperationByName operationName ast with + | None -> + logger.LogTrace "Document has no operation" return IntrospectionQuery <| ValueNone - else - return! checkAnonymousFieldsOnly ctx + | Some op -> + if not (op.OperationType = Ast.Query) then + logger.LogTrace "Document operation is not of type Query" + return createParsedContent () |> OperationQuery + else + let hasNonMetaFields = + Ast.containsFieldsBeyond + Ast.metaTypeFields + (fun x -> + logger.LogTrace($"Operation Selection in Field with name: {{fieldName}}", x.Name)) + (fun _ -> logger.LogTrace "Operation Selection is non-Field type") + op + + if hasNonMetaFields then + return createParsedContent() |> OperationQuery + else + return IntrospectionQuery <| ValueSome ast } + let request = ctx.Request + + if HttpMethods.Get = request.Method then + logger.LogTrace("Request is GET. Must be an introspection query") + return IntrospectionQuery <| ValueNone + else + let! hasBody = checkIfHasBody request + + if not hasBody then + logger.LogTrace("Request is not GET, but has no body. Must be an introspection query") + return IntrospectionQuery <| ValueNone + else + return! checkAnonymousFieldsOnly ctx + } + /// Execute default or custom introspection query - let executeIntrospectionQuery (executor : Executor<_>) (ast : Ast.Document voption) = - task { - let! result = - match ast with - | ValueNone -> executor.AsyncExecute IntrospectionQuery.Definition - | ValueSome ast -> executor.AsyncExecute ast - - let response = result |> toResponse - return Results.Ok response - } + let executeIntrospectionQuery (executor: Executor<_>) (ast: Ast.Document voption) = task { + let! result = + match ast with + | ValueNone -> executor.AsyncExecute IntrospectionQuery.Definition + | ValueSome ast -> executor.AsyncExecute ast + + let response = result |> toResponse + return Results.Ok response + } /// Execute the operation for given request - let executeOperation (executor : Executor<_>) content = - task { - let operationName = - content.OperationName - |> Skippable.filter (not << isNull) - |> Skippable.toOption - let variables = - content.Variables - |> Skippable.filter (not << isNull) - |> Skippable.toOption - - operationName - |> Option.iter (fun on -> logger.LogTrace ("GraphQL operation name: '{operationName}'", on)) - - logger.LogTrace ($"Executing GraphQL query:\n{{query}}", content.Query) - - variables - |> Option.iter (fun v -> logger.LogTrace ($"GraphQL variables:\n{{variables}}", v)) - - let root = options.CurrentValue.RootFactory ctx - - let! result = - Async.StartAsTask ( - executor.AsyncExecute (content.Ast, root, ?variables = variables, ?operationName = operationName), - cancellationToken = ctx.RequestAborted - ) + let executeOperation (executor: Executor<_>) content = task { + let operationName = content.OperationName |> Skippable.filter (not << isNull) |> Skippable.toOption + let variables = content.Variables |> Skippable.filter (not << isNull) |> Skippable.toOption - let response = result |> toResponse - return Results.Ok response - } + operationName + |> Option.iter (fun on -> logger.LogTrace("GraphQL operation name: '{operationName}'", on)) + + logger.LogTrace($"Executing GraphQL query:\n{{query}}", content.Query) + + variables + |> Option.iter (fun v -> logger.LogTrace($"GraphQL variables:\n{{variables}}", v)) + + let root = options.CurrentValue.RootFactory ctx + + let! result = + Async.StartAsTask( + executor.AsyncExecute(content.Ast, root, ?variables = variables, ?operationName = operationName), + cancellationToken = ctx.RequestAborted + ) + + let response = result |> toResponse + return Results.Ok response + } if ctx.RequestAborted.IsCancellationRequested then Task.FromResult None From 36d71cab8ae21876c2a86d8d2f28d196b467f10a Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 11 Apr 2024 03:45:25 +0400 Subject: [PATCH 14/17] Reduced allocations in `GQLProblemDetails` creation --- src/FSharp.Data.GraphQL.Shared/Errors.fs | 49 +++++++++++------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Shared/Errors.fs b/src/FSharp.Data.GraphQL.Shared/Errors.fs index 133339269..1dcd3db6f 100644 --- a/src/FSharp.Data.GraphQL.Shared/Errors.fs +++ b/src/FSharp.Data.GraphQL.Shared/Errors.fs @@ -5,6 +5,7 @@ open System.Collections.Generic open System.Collections.ObjectModel open System.Linq open System.Runtime +open System.Runtime.InteropServices open System.Text.Json.Serialization type internal FieldPath = obj list @@ -145,15 +146,15 @@ type GQLProblemDetails = { Extensions = Dictionary 1 |> GQLProblemDetails.SetErrorKind kind |> Include } - static member Create (message, ?extensions : IReadOnlyDictionary) = { + static member Create (message, extensions : IReadOnlyDictionary) = { Message = message Exception = ValueNone Path = Skip Locations = Skip - Extensions = extensions |> Skippable.ofOption + Extensions = Include extensions } - static member Create (message, extensions) = { + static member Create (message, [] extensions) = { Message = message Exception = ValueNone Path = Skip @@ -161,23 +162,31 @@ type GQLProblemDetails = { Extensions = extensions } - static member CreateOfException (message : string, ex : Exception, ?path : FieldPath, ?extensions : IReadOnlyDictionary) = { + static member CreateOfException (message : string, ex : Exception, [] path : FieldPath Skippable, [] extensions : IReadOnlyDictionary Skippable) = { Message = message Exception = ValueSome ex - Path = path |> Skippable.ofOption + Path = path + Locations = Skip + Extensions = extensions + } + + static member CreateOfException (message : string, ex : Exception, path : FieldPath, [] extensions : IReadOnlyDictionary Skippable) = { + Message = message + Exception = ValueSome ex + Path = Include path Locations = Skip - Extensions = extensions |> Skippable.ofOption + Extensions = extensions } - static member Create (message, path, ?extensions : IReadOnlyDictionary) = { + static member Create (message, path, extensions : IReadOnlyDictionary) = { Message = message Exception = ValueNone Path = Include path Locations = Skip - Extensions = extensions |> Skippable.ofOption + Extensions = Include extensions } - static member Create (message, path, extensions) = { + static member Create (message, path, [] extensions) = { Message = message Exception = ValueNone Path = Include path @@ -185,12 +194,12 @@ type GQLProblemDetails = { Extensions = extensions } - static member Create (message, locations, ?extensions : IReadOnlyDictionary) = { + static member Create (message, locations, extensions : IReadOnlyDictionary) = { Message = message Exception = ValueNone Path = Skip Locations = Include locations - Extensions = extensions |> Skippable.ofOption + Extensions = Include extensions } static member Create (message, locations, extensions) = { @@ -214,11 +223,7 @@ type GQLProblemDetails = { match error with | :? IGQLExceptionError as exceptionError -> - match extensions with - | Include x -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, extensions = x) - | Skip -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception) + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, extensions = extensions) | _ -> GQLProblemDetails.Create (message, extensions) @@ -235,11 +240,7 @@ type GQLProblemDetails = { match error with | :? IGQLExceptionError as exceptionError -> - match extensions with - | Include x -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions = x) - | Skip -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path) + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions) | _ -> GQLProblemDetails.Create (message, path, extensions) @@ -259,11 +260,7 @@ type GQLProblemDetails = { match error with | :? IGQLExceptionError as exceptionError -> - match extensions with - | Include x -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions = x) - | Skip -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path) + GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions) | _ -> GQLProblemDetails.Create (message, path, extensions) From 9973bef41af2cd2389f47fc0990920ba0f08d722 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 11 Apr 2024 03:45:58 +0400 Subject: [PATCH 15/17] Formatted `HttpHandlers` correctly --- .../Giraffe/HttpHandlers.fs | 140 ++++++++---------- 1 file changed, 61 insertions(+), 79 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index 9548a2c05..50204f4bb 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -25,9 +25,9 @@ module HttpHandlers = let rec private moduleType = getModuleType <@ moduleType @> - let ofTaskIResult ctx (taskRes: Task) : HttpFuncResult = task { + let ofTaskIResult ctx (taskRes : Task) : HttpFuncResult = task { let! res = taskRes - do! res.ExecuteAsync(ctx) + do! res.ExecuteAsync (ctx) return Some ctx } @@ -41,28 +41,28 @@ module HttpHandlers = let logger = sp.CreateLogger moduleType - let options = sp.GetRequiredService>>() + let options = sp.GetRequiredService>> () let toResponse { DocumentId = documentId; Content = content; Metadata = metadata } = let serializeIndented value = let jsonSerializerOptions = options.Get(IndentedOptionsName).SerializerOptions - JsonSerializer.Serialize(value, jsonSerializerOptions) + JsonSerializer.Serialize (value, jsonSerializerOptions) match content with - | Direct(data, errs) -> - logger.LogDebug( + | Direct (data, errs) -> + logger.LogDebug ( $"Produced direct GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace($"GraphQL response data:\n:{{data}}", serializeIndented data) + logger.LogTrace ($"GraphQL response data:\n:{{data}}", serializeIndented data) - GQLResponse.Direct(documentId, data, errs) - | Deferred(data, errs, deferred) -> - logger.LogDebug( + GQLResponse.Direct (documentId, data, errs) + | Deferred (data, errs, deferred) -> + logger.LogDebug ( $"Produced deferred GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata @@ -71,41 +71,32 @@ module HttpHandlers = if logger.IsEnabled LogLevel.Debug then deferred |> Observable.add (function - | DeferredResult(data, path) -> - logger.LogDebug( - "Produced GraphQL deferred result for path: {path}", - path |> Seq.map string |> Seq.toArray |> Path.Join - ) + | DeferredResult (data, path) -> + logger.LogDebug ("Produced GraphQL deferred result for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( - $"GraphQL deferred data:\n{{data}}", - serializeIndented data - ) - | DeferredErrors(null, errors, path) -> - logger.LogDebug( - "Produced GraphQL deferred errors for path: {path}", - path |> Seq.map string |> Seq.toArray |> Path.Join - ) + logger.LogTrace ($"GraphQL deferred data:\n{{data}}", serializeIndented data) + | DeferredErrors (null, errors, path) -> + logger.LogDebug ("Produced GraphQL deferred errors for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace($"GraphQL deferred errors:\n{{errors}}", errors) - | DeferredErrors(data, errors, path) -> - logger.LogDebug( + logger.LogTrace ($"GraphQL deferred errors:\n{{errors}}", errors) + | DeferredErrors (data, errors, path) -> + logger.LogDebug ( "Produced GraphQL deferred result with errors for path: {path}", path |> Seq.map string |> Seq.toArray |> Path.Join ) if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( + logger.LogTrace ( $"GraphQL deferred errors:\n{{errors}}\nGraphQL deferred data:\n{{data}}", errors, serializeIndented data )) - GQLResponse.Direct(documentId, data, errs) + GQLResponse.Direct (documentId, data, errs) | Stream stream -> - logger.LogDebug( + logger.LogDebug ( $"Produced stream GraphQL response with documentId = '{{documentId}}' and metadata:\n{{metadata}}", documentId, metadata @@ -115,23 +106,20 @@ module HttpHandlers = stream |> Observable.add (function | SubscriptionResult data -> - logger.LogDebug("Produced GraphQL subscription result") + logger.LogDebug ("Produced GraphQL subscription result") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( - $"GraphQL subscription data:\n{{data}}", - serializeIndented data - ) - | SubscriptionErrors(null, errors) -> - logger.LogDebug("Produced GraphQL subscription errors") + logger.LogTrace ($"GraphQL subscription data:\n{{data}}", serializeIndented data) + | SubscriptionErrors (null, errors) -> + logger.LogDebug ("Produced GraphQL subscription errors") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace($"GraphQL subscription errors:\n{{errors}}", errors) - | SubscriptionErrors(data, errors) -> - logger.LogDebug("Produced GraphQL subscription result with errors") + logger.LogTrace ($"GraphQL subscription errors:\n{{errors}}", errors) + | SubscriptionErrors (data, errors) -> + logger.LogDebug ("Produced GraphQL subscription result with errors") if logger.IsEnabled LogLevel.Trace then - logger.LogTrace( + logger.LogTrace ( $"GraphQL subscription errors:\n{{errors}}\nGraphQL deferred data:\n{{data}}", errors, serializeIndented data @@ -139,41 +127,38 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> - if errs |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) |> (not << Seq.isEmpty) + if + errs + |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) + |> (not << Seq.isEmpty) then errs - |> List.choose(fun x -> x.Exception |> Option.ofValueOption) - |> List.iter - (fun ex -> - logger.LogError( - ex, - "Error while processing request that generated response with documentId '{documentId}'", - documentId - ) - ) + |> List.choose (fun x -> x.Exception |> Option.ofValueOption) + |> List.iter (fun ex -> + logger.LogError (ex, "Error while processing request that generated response with documentId '{documentId}'", documentId)) else - logger.LogWarning( - ( "Produced request error GraphQL response with:\n" - + "- documentId: '{documentId}'\n" - + "- error(s):\n {requestError}\n" - + "- metadata:\n {metadata}\n" ), + logger.LogWarning ( + ("Produced request error GraphQL response with:\n" + + "- documentId: '{documentId}'\n" + + "- error(s):\n {requestError}\n" + + "- metadata:\n {metadata}\n"), documentId, errs, metadata ) - GQLResponse.RequestError(documentId, errs) + GQLResponse.RequestError (documentId, errs) /// Checks if the request contains a body - let checkIfHasBody (request: HttpRequest) = task { + let checkIfHasBody (request : HttpRequest) = task { if request.Body.CanSeek then return (request.Body.Length > 0L) else - request.EnableBuffering() + request.EnableBuffering () let body = request.Body let buffer = Array.zeroCreate 1 - let! bytesRead = body.ReadAsync(buffer, 0, 1) - body.Seek(0, SeekOrigin.Begin) |> ignore + let! bytesRead = body.ReadAsync (buffer, 0, 1) + body.Seek (0, SeekOrigin.Begin) |> ignore return bytesRead > 0 } @@ -182,23 +167,21 @@ module HttpHandlers = /// and lastly by parsing document AST for introspection operation definition. /// /// Result of check of - let checkOperationType (ctx: HttpContext) = taskResult { + let checkOperationType (ctx : HttpContext) = taskResult { - let checkAnonymousFieldsOnly (ctx: HttpContext) = taskResult { - let! gqlRequest = ctx.TryBindJsonAsync(GQLRequestContent.expectedJSON) + let checkAnonymousFieldsOnly (ctx : HttpContext) = taskResult { + let! gqlRequest = ctx.TryBindJsonAsync (GQLRequestContent.expectedJSON) let! ast = Parser.parseOrIResult ctx.Request.Path.Value gqlRequest.Query let operationName = gqlRequest.OperationName |> Skippable.toOption - let createParsedContent() = { + let createParsedContent () = { Query = gqlRequest.Query Ast = ast OperationName = gqlRequest.OperationName Variables = gqlRequest.Variables } if ast.IsEmpty then - logger.LogTrace( - "Request is not GET, but 'query' field is an empty string. Must be an introspection query" - ) + logger.LogTrace ("Request is not GET, but 'query' field is an empty string. Must be an introspection query") return IntrospectionQuery <| ValueNone else match Ast.findOperationByName operationName ast with @@ -213,13 +196,12 @@ module HttpHandlers = let hasNonMetaFields = Ast.containsFieldsBeyond Ast.metaTypeFields - (fun x -> - logger.LogTrace($"Operation Selection in Field with name: {{fieldName}}", x.Name)) + (fun x -> logger.LogTrace ($"Operation Selection in Field with name: {{fieldName}}", x.Name)) (fun _ -> logger.LogTrace "Operation Selection is non-Field type") op if hasNonMetaFields then - return createParsedContent() |> OperationQuery + return createParsedContent () |> OperationQuery else return IntrospectionQuery <| ValueSome ast } @@ -227,20 +209,20 @@ module HttpHandlers = let request = ctx.Request if HttpMethods.Get = request.Method then - logger.LogTrace("Request is GET. Must be an introspection query") + logger.LogTrace ("Request is GET. Must be an introspection query") return IntrospectionQuery <| ValueNone else let! hasBody = checkIfHasBody request if not hasBody then - logger.LogTrace("Request is not GET, but has no body. Must be an introspection query") + logger.LogTrace ("Request is not GET, but has no body. Must be an introspection query") return IntrospectionQuery <| ValueNone else return! checkAnonymousFieldsOnly ctx } /// Execute default or custom introspection query - let executeIntrospectionQuery (executor: Executor<_>) (ast: Ast.Document voption) = task { + let executeIntrospectionQuery (executor : Executor<_>) (ast : Ast.Document voption) = task { let! result = match ast with | ValueNone -> executor.AsyncExecute IntrospectionQuery.Definition @@ -256,18 +238,18 @@ module HttpHandlers = let variables = content.Variables |> Skippable.filter (not << isNull) |> Skippable.toOption operationName - |> Option.iter (fun on -> logger.LogTrace("GraphQL operation name: '{operationName}'", on)) + |> Option.iter (fun on -> logger.LogTrace ("GraphQL operation name: '{operationName}'", on)) - logger.LogTrace($"Executing GraphQL query:\n{{query}}", content.Query) + logger.LogTrace ($"Executing GraphQL query:\n{{query}}", content.Query) variables - |> Option.iter (fun v -> logger.LogTrace($"GraphQL variables:\n{{variables}}", v)) + |> Option.iter (fun v -> logger.LogTrace ($"GraphQL variables:\n{{variables}}", v)) let root = options.CurrentValue.RootFactory ctx let! result = - Async.StartAsTask( - executor.AsyncExecute(content.Ast, root, ?variables = variables, ?operationName = operationName), + Async.StartAsTask ( + executor.AsyncExecute (content.Ast, root, ?variables = variables, ?operationName = operationName), cancellationToken = ctx.RequestAborted ) From 6d77fe884ec966f738bcd90fd675f1227b6d082b Mon Sep 17 00:00:00 2001 From: valber Date: Thu, 11 Apr 2024 21:29:23 +0200 Subject: [PATCH 16/17] "CreateOfException" -> "Create" --- src/FSharp.Data.GraphQL.Server/IO.fs | 2 +- src/FSharp.Data.GraphQL.Shared/Errors.fs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server/IO.fs b/src/FSharp.Data.GraphQL.Server/IO.fs index ef8e111a8..463d8c3a1 100644 --- a/src/FSharp.Data.GraphQL.Server/IO.fs +++ b/src/FSharp.Data.GraphQL.Server/IO.fs @@ -60,7 +60,7 @@ type GQLExecutionResult = GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create msg ], meta) static member ErrorFromException(documentId : int, ex : Exception, meta : Metadata) = - GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.CreateOfException (ex.Message, ex) ], meta) + GQLExecutionResult.RequestError(documentId, [ GQLProblemDetails.Create (ex.Message, ex) ], meta) static member Invalid(documentId, errors, meta) = GQLExecutionResult.RequestError(documentId, errors, meta) diff --git a/src/FSharp.Data.GraphQL.Shared/Errors.fs b/src/FSharp.Data.GraphQL.Shared/Errors.fs index 1dcd3db6f..0c65adb1d 100644 --- a/src/FSharp.Data.GraphQL.Shared/Errors.fs +++ b/src/FSharp.Data.GraphQL.Shared/Errors.fs @@ -162,7 +162,7 @@ type GQLProblemDetails = { Extensions = extensions } - static member CreateOfException (message : string, ex : Exception, [] path : FieldPath Skippable, [] extensions : IReadOnlyDictionary Skippable) = { + static member Create (message : string, ex : Exception, [] path : FieldPath Skippable, [] extensions : IReadOnlyDictionary Skippable) = { Message = message Exception = ValueSome ex Path = path @@ -170,7 +170,7 @@ type GQLProblemDetails = { Extensions = extensions } - static member CreateOfException (message : string, ex : Exception, path : FieldPath, [] extensions : IReadOnlyDictionary Skippable) = { + static member Create (message : string, ex : Exception, path : FieldPath, [] extensions : IReadOnlyDictionary Skippable) = { Message = message Exception = ValueSome ex Path = Include path @@ -223,7 +223,7 @@ type GQLProblemDetails = { match error with | :? IGQLExceptionError as exceptionError -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, extensions = extensions) + GQLProblemDetails.Create(exceptionError.Message, exceptionError.Exception, extensions = extensions) | _ -> GQLProblemDetails.Create (message, extensions) @@ -240,7 +240,7 @@ type GQLProblemDetails = { match error with | :? IGQLExceptionError as exceptionError -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions) + GQLProblemDetails.Create(exceptionError.Message, exceptionError.Exception, path, extensions) | _ -> GQLProblemDetails.Create (message, path, extensions) @@ -260,7 +260,7 @@ type GQLProblemDetails = { match error with | :? IGQLExceptionError as exceptionError -> - GQLProblemDetails.CreateOfException(exceptionError.Message, exceptionError.Exception, path, extensions) + GQLProblemDetails.Create(exceptionError.Message, exceptionError.Exception, path, extensions) | _ -> GQLProblemDetails.Create (message, path, extensions) From fef559bc8673630e46eefcd5e604f9a2a9ecadf9 Mon Sep 17 00:00:00 2001 From: valber Date: Fri, 12 Apr 2024 22:34:41 +0200 Subject: [PATCH 17/17] .AspNetCore: more memory-efficient exception logging Co-authored-by: Andrii Chebukin --- .../Giraffe/HttpHandlers.fs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs index 50204f4bb..14ad62f3b 100644 --- a/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs +++ b/src/FSharp.Data.GraphQL.Server.AspNetCore/Giraffe/HttpHandlers.fs @@ -127,16 +127,17 @@ module HttpHandlers = GQLResponse.Stream documentId | RequestError errs -> - if + let noExceptionsFound = errs - |> Seq.choose (fun x -> x.Exception |> Option.ofValueOption) - |> (not << Seq.isEmpty) - then - errs - |> List.choose (fun x -> x.Exception |> Option.ofValueOption) - |> List.iter (fun ex -> - logger.LogError (ex, "Error while processing request that generated response with documentId '{documentId}'", documentId)) - else + |> Seq.map + (fun x -> + x.Exception |> ValueOption.iter (fun ex -> + logger.LogError (ex, "Error while processing request that generated response with documentId '{documentId}'", documentId) + ) + x.Exception.IsNone + ) + |> Seq.forall id + if noExceptionsFound then logger.LogWarning ( ("Produced request error GraphQL response with:\n" + "- documentId: '{documentId}'\n"