improve load_application_object_failed

[rmosolgo]

When loads: ... fails to find an object, or when it finds an object that doesn't resolve to the loads: ... type, it calls load_application_object_failed(err).

Currently, the code assumes that load_application_object_failed(err) will raise an error, halting control flow in the caller.

However, if you want to use errors-as-data in your mutation, this doesn't work, because you don't want to raise an error here. So, we should:

• Support errors-as-data when loads: ... fails
• Make sure the docs describe the expected behavior of that method
• Improve the runtime behavior when those expectations aren't met (currently the method just keeps running with nil, resulting in weird behavior)
• Make sure errors from that method hit normal rescue_from handlers

[joelzwarrington]

Hi @rmosolgo, wanted to give some insight into how we're currently using (or abusing) load_application_object_failed.

We're currently transitioning from integer IDs to global IDs, and doing so in a gradual approach and allowing consumers to opt in. When doing this, we can expect to receive both global and non-global IDs for arguments. This has led us to discover the loads keyword for arguments, and it works well when provided a global ID (as the schema level object_from_id method can find the object), but doesn't work well with the integer IDs.

To assist with the transition we've looked at implementing load_application_object_failed to find the object using the following:

# method added to base mutation and resolver classes
def load_application_object_failed(error)
  type_class = error.argument.loads
  type_class.model.find_by(id: error.id)
end

Types which are implementing global IDs have a model added, which is an ActiveRecord class. This appeared to work well - however we also noticed some issues which would be blocking for us to use this approach. The authorized? method isn't called on the type even when it might be returned from load_application_object_failed. We noticed that this else block doesn't get triggered: https://github.com/rmosolgo/graphql-ruby/blob/master/lib/graphql/schema/member/has_arguments.rb#L290

However, the mutation would still resolve, even if not authorized, and the user would only hit unauthorized errors when getting the fields returned from the mutation.

---

Should we be using this approach?

[joelzwarrington]

Wanted to add on to my previous comment, after finding out about the authorized? issue, we have used a work-around to get around this if anyone is in a similar ⛵

• remove load_application_object_failed and don't override the method
• augment our BaseArgument to handle both cases using prepare so that a global ID is always returned

class BaseArgument < GraphQL::Schema::Argument
  def initialize(*args, prepare: nil, loads: nil, **kwargs, &block)
    if loads.present?
      prepare = ->(value, _context) {
        return value if global_id? # this check isn't important, but assume it's being done in some way
        
        object = loads.model.find_by(id: value)
        type = loads

        return if object.nil?

        global_id(object, type)
     }
   end

   super(*args, prepare: prepare, loads: loads, **kwargs, &block)
end

This way object_from_id will always be a global ID and an object should be found regardless of an integer or global ID is being used.

[rodloboz]

Hi @rmosolgo , have you had a change to come back to this issue?

In our GraphQL API we return most errors as userErrors (errors as data), except the ones caused by failed loads because these raise an error and it is handled in the top level errors.

I've been trying to find a way to hook into load_application_object_failed but instead of raising an error or continuing the normal execution of the mutation resolver, to return a payload like this:

{
  event: nil,
  user_errors: {
    message: "Could not find Event",
    code: "NOT_FOUND,
    path: ["input", "eventId"]
  }
}

That would be added in the response data under the appropriate mutation field.

Is there a workaround for this problem or is this behaviour currently not possible?

[rmosolgo]

Hey, after a while, I finally got a PR for this: #4667. After that PR, any object returned from load_application_object_failed is passed along as if it had been loaded :+1: