FSharp.Data.GraphQL icon indicating copy to clipboard operation
FSharp.Data.GraphQL copied to clipboard

Return errors from railway programming model

Open xperiandri opened this issue 5 years ago • 4 comments

Description

We use Chessie and resolver function returns a result. Then I want to unwrap that result and return value or errors as GraphQL response like this:

/// If Ok, returns value and sets messages as GraphQL operation errors
/// If Bad, returns value and sets errors as GraphQL operation errors
let handleMessages<'TSuccess,'TMessage> (ctx : ResolveFieldContext) (result : Result<'TSuccess,'TMessage>) : 'TSuccess =
    let setErrors messages =
        let ex = GraphQLException (ctx.ExecutionInfo.Identifier)
        messages |> Seq.iteri (fun m i -> ex.Data.Add (i, m))
        ctx.AddError ex
    match result with
    | Ok (value, messages) -> setErrors (messages); value
    | Bad messages -> setErrors (messages); Unchecked.defaultof<'TSuccess>

However as long as type is not nullable, returning null produces error.

Repro steps

  1. Create resolver that returns Result type of Chassie library
  2. Use my function to unwrap a result

Expected behavior

Ability to handle Result type and pass errors without raising an exception.

Actual behavior

nullResolverError is called and GraphQLException is returned

Known workarounds

Raise exception after resolver execution

Related information

  • FSharp.Data.GraphQL.Server 1.0.5

xperiandri avatar Mar 30 '20 19:03 xperiandri

What's the type of 'TSuccess in this case? I believe this is by design as to not break the GraphQL contract. If you have a non-nullable resolver throw an exception the behaviour is the engine will propagate the error up the tree until it encounters the first parent that's nullable and nullify the result. This is in order to abide by the contract.

johnberzy-bazinga avatar Mar 30 '20 21:03 johnberzy-bazinga

@johnberzy-bazinga what do you think about switching to Result<'TSuccess,Error list> for coercion functions and for resolvers?

xperiandri avatar Apr 03 '20 21:04 xperiandri

@xperiandri I think that makes sense. Let's see if the spec has anything to say about this. It might be that invalid inputs are fatal errors so execution should halt. But at the very least we should handle the exception and set the error path to the root of the query. If change to the Result type it's a breaking change, so we'll need to bump the version.

johnberzy-bazinga avatar Apr 03 '20 22:04 johnberzy-bazinga

@johnberzy-bazinga what do you think about such signature of coerce functions?

abstract CoerceInput : Value -> Result<struct(obj * string seq), string seq>
abstract CoerceValue : obj -> Result<struct(obj * string seq), string seq>

or

abstract CoerceInput : Value -> Result<struct(obj * obj seq), obj seq>
abstract CoerceValue : obj -> Result<struct(obj * obj seq), obj seq>

As some warnings can also be produced for successfully validated inputs as it is done in https://github.com/fsprojects/Chessie

xperiandri avatar Apr 13 '20 12:04 xperiandri

Implemented in #418

xperiandri avatar Nov 05 '23 23:11 xperiandri