Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Return errors from railway programming model #274

Closed
xperiandri opened this issue Mar 30, 2020 · 5 comments
Closed

Return errors from railway programming model #274

xperiandri opened this issue Mar 30, 2020 · 5 comments

Comments

@xperiandri
Copy link
Collaborator

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
@johnberzy-bazinga
Copy link
Contributor

johnberzy-bazinga commented Mar 30, 2020

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.

@xperiandri
Copy link
Collaborator Author

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

@johnberzy-bazinga
Copy link
Contributor

@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.

@xperiandri
Copy link
Collaborator Author

xperiandri commented Apr 13, 2020

@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
Copy link
Collaborator Author

Implemented in #418

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants