Clarify error handling

[BjRo]

Given the following query

query profileRefQuery {
  me { 
    profile_visits {     
      collection {
       number_of_visits
        visitor_id
        visitor {
          ...profileFragment
        }
      }
      total
    }
  }
  
  profile_refs(ids: [1]){
    ... profileFragment
  }
}

on the following type system

  Me = GraphQL::ObjectType.define do
    name "me"
   
    field :profile_ref do
      type Types::Profiles::ProfileRef
      resolve Resolvers::Profiles::ProfileRef
    end
  end

  QueryType = GraphQL::ObjectType.define do
    name "Query"

    field :profile_refs do
      type types[!Types::Profiles::ProfileRef]
      argument :ids, !types.ID.to_list_type

      resolve ->(obj, args, context) {
        Promise.all(args["ids"].map { |id| Resolvers::Profiles::ProfileRef.call(obj, {"id" => id}, context) })
      }
    end

    field :me do
      type Me
      resolve ->(_,_,_){ {} }
    end
  end

how would I reasonably indicate errors? My Batch::Loader is calling into a different system via HTTP/Rest, which can of course have timeouts.

Currently I raise an GraphQL::ExecutionError from my perform(ids) method when it runs into a timeout. That sort of works.

The only nasty side effect of this is that in the response all paths are wrong (because they point to the resolved promise).

{
  "data": {
    "me": {
      "profile_visits": {
        "collection": [
          {
            "number_of_visits": 2,
            "visitor_id": "2",
            "visitor": null
          },
          {
            "number_of_visits": 2,
            "visitor_id": "15",
            "visitor": null
          },
          {
            "number_of_visits": 1,
            "visitor_id": "2",
            "visitor": null
          },
          {
            "number_of_visits": 1,
            "visitor_id": "15",
            "visitor": null
          }
        ],
        "total": 4
      }
    },
    "profile_refs": null
  },
  "errors": [
    {
      "message": "BACKEND_TIMEOUT",
      "locations": [
        {
          "line": 29,
          "column": 3
        }
      ],
      "path": [
        "profile_refs"
      ],
      "detail": {}
    },
    {
      "message": "BACKEND_TIMEOUT",
      "locations": [
        {
          "line": 29,
          "column": 3
        }
      ],
      "path": [
        "profile_refs"
      ],
      "detail": {}
    },
    {
      "message": "BACKEND_TIMEOUT",
      "locations": [
        {
          "line": 29,
          "column": 3
        }
      ],
      "path": [
        "profile_refs"
      ],
      "detail": {}
    },
    {
      "message": "BACKEND_TIMEOUT",
      "locations": [
        {
          "line": 29,
          "column": 3
        }
      ],
      "path": [
        "profile_refs"
      ],
      "detail": {}
    },
    {
      "message": "BACKEND_TIMEOUT",
      "locations": [
        {
          "line": 29,
          "column": 3
        }
      ],
      "path": [
        "profile_refs"
      ],
      "detail": {}
    }
  ]
}

Which basically means that a client can't detect that resolving the visitor has failed.

How would you handle this? Any suggestions?

[rmosolgo]

What if, instead of raising an error from the loader, you raised it from the resolve function, for example:

      resolve ->(obj, args, context) {
        promises = args["ids"].map { |id| 
          Resolvers::Profiles::ProfileRef.call(obj, {"id" => id}, context).then { |result|
            if result.is_a?(GraphQL::ExecutionError) 
              raise(result)
            else 
              result 
            end 
          }
        }
        Promise.all(promises)
      }

notice that using it that way would mean calling fulfill(id, GraphQL::ExecutionError.new("...")) so that the error is returned as the result and the resolver can handle it.

(I think that graphql-ruby will support that, if it doesn't it's a bug!)

[dylanahsmith]

Unhandled exceptions in the Loader's perform method will be used to reject the promise, so calling sync on that promise will re-raise the exception. You're resolve proc could can use promise.rb chaining methods to rescue the promise (e.g. Promise#then can be given both a fulfill and reject proc).

We try to avoid doing external HTTP requests inline, so don't really have the same issue. We use graphql-batch to talk to internal datastores, where issues making requests to them are an internal error.

If the exception on the promise itself isn't handled, then graphql-ruby will call .sync on it and handle the error. If it is a GraphQL::ExecutionError, then it should have error information associated with that field, otherwise that sounds like a graphql-ruby bug.

The only nasty side effect of this is that in the response all paths are wrong (because they point to the resolved promise).

This isn't clear at all. How do the paths point to the promise? The graphql error information has graphql field names and the location information refers to the GraphQL query.

Which basically means that a client can't detect that resolving the visitor has failed.

You issue description doesn't mention how the visitor is resolved, but this seems like a part you think is the bug. This definitely needs clarification. If you could demonstrate the bug with a simple example, then it would be much easier to tell you what is going on.

[BjRo]

@rmosolgo spot on. My problem primary results in not raising the GraphQL::ExecutionError at the right spot. When I raise it in the then callback, the paths are correct again.

@dylanahsmith No further action necessary. I solved it with @rmosolgo's help.

Thank you both for your help!