Usage with the batch loader

[DamirSvrtan]

Hi @exAspArk! Thank you for your great work regarding the GraphQL stuff!

I've been trying to add this gem in combination with your other gem https://github.com/exAspArk/batch-loader but don't have much luck. The errors that are supposed to be rescued just get propagated.

It works well with fields that don't have batch loaders, the ones that have just propagate the error and it never gets rescued.

Looking at this code:

    def instrument(_type, field)
      old_resolve_proc = field.resolve_proc
      new_resolve_proc = ->(object, arguments, context) do
        begin
          old_resolve_proc.call(object, arguments, context)
        rescue => exception
          if handler = find_handler(exception)
            handler.call(exception, object, arguments, context)
          else
            raise exception
          end
        end
      end

      field.redefine { resolve(new_resolve_proc) }
    end

In the begin rescue block, without the batch loader, the error gets raised and it gets rescued perfectly fine.

With the batch loader, the old_resolve_proc.call(object, arguments, context) just returns an instance of the batch loader and that is it (the error gets raised elsewhere later on).

Have you encountered this error so far?

[exAspArk]

Hi @DamirSvrtan!

That's a very interesting case. graphql-errors can't get the error since the lazy objects were not resolved yet. Resolving them will start producing N+1 queries. Two potential ideas:

1. Handle exceptions in BatchLoader blocks by reusing the handlers (not ideal):

BatchLoader.for(id).batch do |ids, loader|
  ...
rescue => e                 # no need to use begin since Ruby 2.5
  GraphQL::Errors.handle(e) # somehow get the handler
end

2. Handle exceptions when graphql-ruby gem resolves lazy objects.

Need to investigate whether it is possible.

[DamirSvrtan]

Yes it is possible to do this in a nice way!

As you have redefined the resolve here: https://github.com/exAspArk/graphql-errors/blob/master/lib/graphql/errors.rb#L38 you can redefine the lazy_resolve as well.

I can make a PR!

[exAspArk]

@DamirSvrtan would be amazing if you can open a PR!

I just started working on it, was able to get failing tests #13 :) Feel free to "reuse" it if you want

🔴 👉 💚

[DamirSvrtan]

Sure thing, I can continue off of your branch on a fork that I can build!

[exAspArk]

Fixed in #14 💛