GraphQL::Execution::Interpreter and rescue_from compatibility

[JanStevens]

Hello,

As documented here the new GraphQL::Execution::Interpreter does not work with the rescue_from middleware. I think the rescue_from add valuable error handeling that allows your endpoint to always return a 200 and indicate the errors. The following examples are based of the graphql-errors gem which we currently use https://github.com/exAspArk/graphql-errors

The most common example is catching ActiveRecord::RecordNotFound and replace them with nil so whenever you try to fetch something that does not exist you return a 200 with nothing in it.

  rescue_from ActiveRecord::RecordNotFound do |_exception|
    nil
  end

Another example is to error standardisation, for example in our GraphQL api we always call update! or save! this raises a ActiveRecord::RecordInvalid which we capture with rescue_from and standarize the error response:

rescue_from ActiveRecord::RecordInvalid do |exception|
  RecordInvalidError.from_exception(exception)
end

class RecordInvalidError < GraphQL::ExecutionError
  def to_h
    super.merge(problems: extensions[:errors])
  end

  def self.from_exception(exception)
    record = exception.record
    errors = record.errors.map do |attr|
      {
        path: [attr.to_s.camelize(:lower)],
        explanation: record.errors.full_messages_for(attr)
      }
    end
    new(record.errors.full_messages.join("\n"), extensions: { errors: errors })
  end
end

Ofcourse one can say that every mutation / resolver should deal with the errors themself, but this means that you always have to go for a if ...save else do_with_errors end kind of thing which isn't that pretty nor ensures consistency.

Finally sometimes (even when you know its bad) you have some deeply nested code where you want to brake out of so your only resort is to raise an exception, this does not mean you want to return back a 500 but rather a customised message.

If anyone else has other use cases then please do share,

Regards

[masakazutakewaka-stt]

A bit off topic but as far as I know you can't pass a error class object to rescue_from like @JanStevens displayed above. ( So I wrote a small patch that enables to do this )

[JanStevens]

@masakazutakewaka-stt Yes indeed, I based myself off this gem we currently use: https://github.com/exAspArk/graphql-errors

I'll update the issue since I think that graphql-errors should be in graphql-ruby

[masakazutakewaka-stt]

@JanStevens Cool gem (It seems it doesn't support class-based API though) . I looked it up and totally agree with you. It would be really awesome if graphql-ruby gets to support behavior like this ↓ where unhandled errors are rescued with an custom error class object.

rescue_from StandardError do |e|
 CustomError.new(e)
end

[joshforbes]

Just want to throw in on this issue. We use rescue_from to handle RecordNotFound errors (with the graphql-errors gem).

[blevine]

@rmosolgo, thoughts on this?

[RealSilo]

Do you have any updates?

[rmosolgo]

Hi, sorry I haven't written back sooner 😞 ! My thoughts are:

• I agree, graphql-ruby should include better error handling. I think graphql-errors's features are great, and something like that would be a great addition to graphql-ruby.
• I don't have time to work on it yet. My current big project is Simplify schema boot #2363 , and after that, I'll probably look at supporting true parallelism, in the style of Lazy Concurrency Per Evaluation Layer #1981. But, I think I could work on errors after that -- it's probably the next-most important thing after that, and definitely something that should be in good order before rolling over to graphql-ruby 2.0.

🤔 Maybe we could make graphql-errors support the new runtime. I see right now it uses a field instrumenter, which is no longer supported. It would almost work as a FieldExtension:

graphql-ruby/lib/graphql/schema/field_extension.rb

Lines 42 to 51 in 272e36e

	# @param object [Object] The object the field is being resolved on
	# @param arguments [Hash] Ruby keyword arguments for resolving this field
	# @param context [Query::Context] the context for this query
	# @yieldparam object [Object] The object to continue resolving the field on
	# @yieldparam arguments [Hash] The keyword arguments to continue resolving with
	# @yieldparam memo [Object] Any extension-specific value which will be passed to {#after_resolve} later
	# @return [Object] The return value for this field.
	def resolve(object:, arguments:, context:)
	yield(object, arguments, nil)
	end

But I'm not sure exactly how to rescue errors from promises there. I will make some time for an investigation on that and write back here!

[raysuelzer]

Any updates or thoughts on this? It seems like this PR is very close to working in the graphql-errors repo.

https://github.com/exAspArk/graphql-errors/pull/18/files

[thiago-sydow]

Hi! I'm in the same situation, enabled the new interpreter to have subscriptions and my rescue_from stopped working. I'm doing a workaround that works but I'm wondering if could be a performance impact to do like this, or if it can be worked on top of that (I saw the tracers for platform analytics rely on the key to execute something or not.

What I'm doing right now is simply on schema.rb:

tracer ErrorHandlingTracer

and the class is:

  class ErrorHandlingTracer
    def self.trace(key, data)
      yield
    rescue Sequel::ValidationFailed => error_1
      GraphQL::ExecutionError.new(error_1.message)
    rescue Some other error => error_2
      extensions = error_2.extra_data.is_a?(Hash) ? error_2.extra_data : {}
      GraphQL::ExecutionError.new(error_2.message, extensions: extensions)
    end
  end

[rmosolgo]

@thiago-sydow , I think that's a great solution, thanks for sharing! I don't think there will be much performance impact for it.

[rmosolgo]

Hi, thanks to all who shared their thoughts here. I've added some new error handling which you can try on master: #2458

If you give a try and encounter a problem, please open a new issue!