search

graphiti-api / graphiti-rails

MIT License
54 stars 28 forks source link

Provide a nice API to use inside of `rescue_from` #12

Closed wagenet closed 5 years ago

wagenet commented 5 years ago

rescue_from bypasses all default error handling. Ideally, we'd provide a nice API to call from this.

richmolj commented 5 years ago

I wonder if we took the current code

rescue_from Exception do |e|
  handle_exception(e, show_raw_error: !!current_user.internal)
end

And threw a deprecation. Then document that customizing behavior is done with register_exception instead of rescue_from (so we can make more adjustments to the resulting payload, and do so with a class-based interface), and handle everything with ActionDispatch (which would grab the relevant registered exception and handle appropriately).

Does this sound feasible or am I off base?

wagenet commented 5 years ago

I like the general idea, the main issue is that in the current implementation we only go through the GraphitiError handlers for JSON:API by default, with other content types being opt-in. So there could easily be a case where someone registers an exception but uses a different content type and it never gets used.

wagenet commented 5 years ago

See #19 for a better overview of the problem.