Handling Errors : Exception inheritance hierarchy

[jfrancoist]

Referring to this line : https://github.com/rmosolgo/graphql-ruby/blob/master/lib/graphql/schema/rescue_middleware.rb#L43

Is there any particular reason why the rescue_from would only match the exact type and not match anything in the inheritance hierarchy ? This is how it currently works in ruby, if I rescue StandardError I would catch anything from ArgumentError to RuntimeError

This is my scenario :

We have custom validation for input and system that will throw a custom exception if something is wrong. But at the same time we would also like to catch StandardError and send a masked output, because we don't want the user to see those error like "ActiveRecord::NotFound"

The code would be something like that :

MySchema.rescue_from(StandardError) { |error| ErrorHandler.handle(error) }

class ErrorHander
  def self.handle(error)
    if error.is_a?(CustomError)
      Logger.warn(error.message)
      error.message
    else
      Logger.error(error.message)
      "internal error"
    end
  end  
end

I do understand that I can just add this line for the CustomError, but I still need to log those in our logger and I rather not have different actions doing those. MySchema.rescue_from(CustomError) { |error| error.message }

Do you have any suggestion for this scenario ?

[rmosolgo]

It's just a shortcoming of the implementation, right there on the line you found! I agree that it would make more sense to work like ruby's rescue. It would be interesting to see how ActionController implements error handling, I wonder if we can take a page out of their book.

Do you have any suggestion for this scenario ?

To be honest, I have doubts about the future of middleware as a whole. There are two problems:

• It's not robust enough to handle some important use cases (see #186)
• It adds overhead to every field resolution, even when it makes no sense to do so

So in terms of suggestions, I suggest custom error handling, something like this:

# Wrap field resolver `resolve_func` with a handler for `error_superclass`. 
# `RescueFrom` instances are valid field resolvers too. 
class RescueFrom 
  def initialize(error_superclass, resolve_func) 
    @error_superclass = error_superclass 
    @resolve_func = resolve_func
  end 

  def call(obj, args, ctx) 
    @resolve_func.call(obj, args, ctx) 
  rescue @error_superclass 
    # Your error handling logic here:
    # - return an instance of `GraphQL::ExecutionError` 
    # - or, return nil:
    nil 
  end 
end 

Then, apply it to fields on an opt-in basis:

field :create_post, PostType, 
  # Wrap the resolve function with `RescueFrom.new(err_class, ...)`
  resolve RescueFrom.new(ActiveRecord::RecordInvalid, -> (obj, args, ctx) { ... })
end 

This way, you get error handling with proper Ruby semantics and no overhead! I'm rewriting some docs now, I think I'll copy this code right into them ... !

[jfrancoist]

Thanks for the reply.

In our case, since we always doing some type of database access in our queries and mutations. It would be better to check all the fields. It feels kinda repeating the the same code when it should be handled at a higher level, but that is specific to our scenario. If it was somewhere else this solution would have been much preferred.

I read a bit of the discussion about deprecating middleware, and I think the proposition of using of instrumentation sound pretty cool. Unfortunately for now I think I will add a new middleware and point out that we should move away from it as soon as there is a solid alternative in place.

I have looked into the ActionController and even though it's written that i will go through the hierarchy L28 is does seems to be doing so using the cause field L120. So it does kind it's been implemented right now by checking exact type first and if there is a cause type it will also check for those.

[jfrancoist]

The middleware does not give out what I would have excepted. I tried your proposal and it seems fine. I've altered it a bit for our need, since I will always be catching StandardError.

class Rescuable 
  def initialize(resolve_func, error_superclass: StandardError) 
    @error_superclass = error_superclass 
    @resolve_func = resolve_func
  end 

  def call(obj, args, ctx) 
    @resolve_func.call(obj, args, ctx) 
  rescue @error_superclass 
    # Your error handling logic here:
    # - return an instance of `GraphQL::ExecutionError` 
    # - or, return nil:
    nil 
  end 
end 

field :create_post, PostType, 
  # Wrap the resolve function with `RescueFrom.new(err_class, ...)`
  resolve Rescuable.new -> (obj, args, ctx) { ... }
end 

Makes it a little bit less verbose.

Thanks for the help.

[rmosolgo]

Glad it worked out! I'll make sure to add this to the docs soon.

[dblock]

https://github.com/rmosolgo/graphql-ruby/pull/1035 is related