Graphql-ruby: RFC: Re-raise errors with additional GraphQL backtrace

[rmosolgo]

I just learned you can provide a backtrace when raising an error as the third argument to raise(error_or_class, message, backtrace). So, you could amend a backtrace before raising an error, for example:

def raise_error
  raise KeyError, "Something bad happened"
end

begin
  raise_error
rescue
  backtrace_additions = ["(added):1", "(added:2)"]
  raise $!, $!.message, backtrace_additions + $!.backtrace
end
# KeyError: Something bad happened
#   from (added):1
#   from (added:2)
#   from (irb):2:in `raise_error'
#   from (irb):6
#   from /usr/bin/irb:12:in `<main>'

So, could we track graphql execution and _add_ entries to the backtrace when errors come from graphql resolve functions?

[theorygeek]

Are you thinking that we would add things like the path in the query?

query MyQuery {
  node(id: "...") { someField { someOtherField } }
}
# maybe other queries in the document too...

# Show the name of the operation, and the path:
#       in someOtherField
#       in someField[3]
#       in node(id: "...")
#   in MyQuery
#   from (irb):2:in `raise_error'
#   from (irb):6
#   from /usr/bin/irb:12:in `<main>'

[rmosolgo]

Yeah exactly! Maybe it could be optional while we try it out, eg

# lib/graphql/error.rb
  def backtrace 
    if GraphQL.path_backtrace && @context 
      backtrace_from_context(@context) + super 
    else 
      super 
    end 
  end 

[rmosolgo]

Here's another approach to modifying the backtrace:

https://github.com/ko1/pretty_backtrace

The approach looks like:

• Add a handler for raised errors
• Use the debug inspector to backtrack through the RubyVM stack
• Gather the local variables and inspect them
• Rebuild the stack trace with new metadata

[eapache]

Don't have a strong opinion which approach to use, but something along these lines would be super-useful. Figuring out which chain of selections/fields resulted in a specific error is one of the most annoying parts of debugging our schema right now.

[xuorig]

Interesting idea. Is it worth it in a world where resolvers are methods on objects? The reason why the backtrace is not super clear right now is the heavy proc usage during execution, I have a feeling it will become if the definition changes in the following weeks / months 💭

[eapache]

I think it's still worth it. Even when resolvers are methods, the ruby trace alone won't tell you how you got there, e.g. consider the following totally made-up query:

{
  myObject { foo }
  myListOfObjects(first: 10) { edge { node { foo } } }
}

Even if you know the exception occurred in the method for foo, it probably looks something like

foo:123
someInternalGemMethod:xxx
someInternalGemMethod2:xxx
someInternalGemMethod3:xxx
someInternalGemMethod4:xxx

It would still be really helpful to know whether the exception came from myObject or from myListOfObjects and if from the list, from which one.

[xuorig]

Good point.

As long as we add to it and not modify it / delete from it , I think it would provide great value 👍

[rmosolgo]

trying in #946

I didn't use RubyVM::DebugInspector like koichi's gem above because it's MRI-only, and I know some folks use JRuby.