More refactoring

lib/graphiti/rails.rb

@@ -17,11 +17,16 @@ def self.included(_klass)
 
 require "graphiti/rails/railtie"
 
-# The standard Rails ShowExceptions middleware handles exceptions and either
-# displays them (in test and development, by default) or calls the exceptions_app
-# to render in a user-friendly format. Unfortunately, the default exceptions_app
-# does not handle Graphiti errors as specifically as we would prefer. To handle this,
-# we wrap the default app and handle those errors ourselves.
+# The standard Rails ShowExceptions middleware passes exceptions on to `config.exceptions_app`.
+# However, the default exceptions_app (PublicExceptions) doesn't handle JSON API responses
+# correctly, so we wrap it ourselves to ensure correct handling.
+#
+# One potential problem here is if people want to handle the JSON API responses themselves.
+# In that case, we might actually want to extend PublicExceptions insteand and then let people use
+# their own handling when they overwite the exceptions app. The downside to this is that people
+# setting their own exceptions app may not know that they should handle JSON API specially.
+#
+# If we continue with this approach, we should see about getting an API into Rails for this.
 ActionDispatch::ShowExceptions.class_eval do
   alias_method :initialize_without_graphiti, :initialize
   def initialize(*args)
@@ -30,6 +35,33 @@ def initialize(*args)
   end
 end
 
+# Since we have more information for JSON API errors from Graphiti we hijack the defaults
+# and do do the better rendering. Ideally, we'd have a hook in Rails for this.
+#
+# Set `config.debug_exception_response_format = :api` for this to work correctly.
+#
+# TODO: Investigate whether dev tools will render a returned HTML formatted error correctly.
+# If so, there may be some benefit in not changing `debug_exception_response_format` since
+# an HTML formatted error might be nicer to read than JSON formatted. However, we should
+# still handle the JSON API case correctly when `debug_exception_response_format == :api`.
+ActionDispatch::DebugExceptions.class_eval do
+  private
+
+  alias_method :render_for_api_request_without_graphiti, :render_for_api_request
+  def render_for_api_request(content_type, wrapper)
+    return super unless content_type == :jsonapi
+
+    # TODO: Make this configurable, also handle invalid requests correctly
+    handler = Graphiti::Rails::ExceptionHandler.new
+    handler.show_raw_error = true
+
+    json   = exception_klass.error_payload(e)
+    status = exception_klass.status_code(e)
+
+    render(status, json.to_json, content_type)
+  end
+end
+
 ActiveSupport.on_load(:action_controller) do
   include Graphiti::Rails::Context
   include Graphiti::Rails::Errors

lib/graphiti/rails/exceptions_app.rb

@@ -2,8 +2,7 @@ module Graphiti
   module Rails
     # Similar function to GraphitiErrors#handle_exceptions but more Railsy
     #
-    # This Rack app looks for Graphiti exceptions and, if registered, will render them in
-    # in a nicer format. Also, if the request is JSON API then we clean it up for more uniform display.
+    # This Rack app looks for JSON API requests then renders them in a more correct format.
     # Other exceptions are passed on to the app specified in the Rails app's `config.exceptions_app`.
     class ExceptionsApp
       def initialize(app)
@@ -12,26 +11,22 @@ def initialize(app)
 
       def call(env)
         request = ActionDispatch::Request.new(env)
-
-        begin
-          content_type = request.formats.first
-        rescue Mime::Type::InvalidMimeType
-          content_type = Mime[:text]
-        end
-
+        content_type = request.formats.first rescue nil
         exception = request.get_header "action_dispatch.exception"
         context = request.get_header "graphiti.error_lookup_context"
 
-        # Render handled exceptions, also ensure all JSONAPI responses are in a machine readable format
-        if context && (context.registered_graphiti_exception?(exception) || content_type == :jsonapi)
-          exception_klass = context.graphiti_exception_handler_for(exception)
+        # Ensure all JSON API responses are in the correct format
+        if content_type == :jsonapi
+          exception_klass = context&.graphiti_exception_handler_for(exception) || GraphitiErrors::ExceptionHandler.new
+
+          # TODO: Do we actually want to log?
           exception_klass.log(exception)
 
           payload = exception_klass.error_payload(exception)
           status = exception_klass.status_code(exception)
           body = payload.to_json
 
-          [status, { "Content-Type" => "application/json; charset=#{ActionDispatch::Response.default_charset}",
+          [status, { "Content-Type" => "#{content_type}; charset=#{ActionDispatch::Response.default_charset}",
             "Content-Length" => body.bytesize.to_s }, [body]]
         else
           @app.call(env)

lib/graphiti/rails/exceptions_handler.rb

@@ -0,0 +1,17 @@
+module Graphiti
+  module Rails
+    class ExceptionHandler < GraphitiErrors::ExceptionHandler
+      # Since we're registering the errors with ActionDispatch, we can also use that
+      # for getting the status code.
+      def status_code(error)
+        ActionDispatch.ExceptionWrapper.status_code_for_exception(error.class.name)
+      end
+
+      def detail(error)
+        if status_code(error) >= 500
+          default_detail
+        end
+      end
+    end
+  end
+end

lib/graphiti/rails/railtie.rb

@@ -1,9 +1,51 @@
 # frozen_string_literal: true
 
-# TODO: Remove this if we don't end up needing it
 module Graphiti
   module Rails
     class Railtie < ::Rails::Railtie
+      config.action_dispatch.rescue_responses.merge!(
+        # Graphiti::Errors::AdapterNotImplemented,
+        # Graphiti::Errors::SideloadConfig,
+        # Graphiti::Errors::Remote,
+        # Graphiti::Errors::AroundCallbackProc,
+        # Graphiti::Errors::RemoteWrite,
+        # Graphiti::Errors::UnsupportedOperator,
+        # Graphiti::Errors::UnwritableRelationship,
+        # Graphiti::Errors::SingularSideload,
+        # Graphiti::Errors::UnsupportedSort,
+        # Graphiti::Errors::ExtraAttributeNotFound,
+        # Graphiti::Errors::InvalidFilterValue,
+        # Graphiti::Errors::MissingEnumAllowList,
+        # Graphiti::Errors::InvalidLink,
+        # Graphiti::Errors::SingularFilter,
+        # Graphiti::Errors::Unlinkable,
+        # Graphiti::Errors::SideloadParamsError,
+        # Graphiti::Errors::SideloadQueryBuildingError,
+        # Graphiti::Errors::SideloadAssignError,
+        # Graphiti::Errors::AttributeError,
+        # Graphiti::Errors::InvalidJSONArray,
+        # Graphiti::Errors::InvalidEndpoint,
+        # Graphiti::Errors::InvalidType,
+        # Graphiti::Errors::ResourceEndpointConflict,
+        # Graphiti::Errors::PolymorphicResourceChildNotFound,
+        # Graphiti::Errors::ValidationError,
+        # Graphiti::Errors::ImplicitFilterTypeMissing,
+        # Graphiti::Errors::ImplicitSortTypeMissing,
+        # Graphiti::Errors::TypecastFailed,
+        # Graphiti::Errors::ModelNotFound,
+        # Graphiti::Errors::TypeNotFound,
+        # Graphiti::Errors::PolymorphicSideloadTypeNotFound,
+        # Graphiti::Errors::PolymorphicSideloadChildNotFound,
+        # Graphiti::Errors::MissingSideloadFilter,
+        # Graphiti::Errors::MissingDependentFilter,
+        # Graphiti::Errors::ResourceNotFound
+        # Graphiti::Errors::UnsupportedPagination,
+        # Graphiti::Errors::UnsupportedPageSize,
+        # Graphiti::Errors::InvalidInclude,
+        # Graphiti::Errors::StatNotFound,
+        Graphiti::Errors::RecordNotFound => 404
+        # Graphiti::Errors::RequiredFilter
+      )
     end
   end
 end