graphiti-api / graphiti_errors

MIT License
1 stars 8 forks source link

Format errors using the requested format. #15

Closed Ben-Fenner closed 2 years ago

Ben-Fenner commented 2 years ago

We are implementing an API for a client who insists on using XML.

This is working okay, but exceptions/errors are returned formatted as JSON. Can the errors be formatted using the same format that the request was asking for?

Looking through the code, this seems to be where the magic happens, but that's as far as I got: https://github.com/graphiti-api/graphiti_errors/blob/master/lib/graphiti_errors.rb#L66

According to the documentation, I don't think a custom handler can accomplish what I'm after? https://www.graphiti.dev/guides/concepts/error-handling#advanced

(It looks like there are plans to move completely to rescue_registry. Will that make this ticket obsolete?)

richmolj commented 2 years ago

XML support is definitely tacked on, and even vanilla JSON errors are subpar. If you can get it working I'd be happy to merge, but don't think I will ever have this use case myself (please god). I don't think a custom handler or rescue_regisitry should be relevant - those are really more about catching the error, handling it, and the shape of the response...but as you point out that's different from the rendering format. There MIGHT be something in graphiti_rails as well since that registeres renderers and connects with jsonapi_errors

Ben-Fenner commented 2 years ago

I'll look into the feasibility of submitting a pull request and get back to you. :+1:

Ben-Fenner commented 2 years ago

I am not really sure how the request.format would be referenced while rendering an error to figure out which format to render it in. I'm sure it could be done, but implementing this with a PR right now does not seem feasible for me.

Instead, I've hobbled together a very poor solution by creating what is essentially a before_render controller filter and applied the logic there. If anyone is morbidly curious, it looks like this inside my Api::ApplicationController:

  def render(*args)
    requested_format = request.format.to_s.split('application/').last
    requested_format = 'jsonapi' if ['apijson', 'vnd.api+json'].include?(requested_format)
    rendered_format  = args&.first&.first&.first&.to_s
    rendered_format  = 'jsonapi' if rendered_format == 'prefixes'

    if rendered_format != requested_format
      # A response was about to be rendered that doesn't match the format requested.
      # Likely we are rendering an error which Graphiti always formats as JSON, but the user requested XML or JSONAPI.
      # So format the response accordingly.

      payload = args[0][:json]
      if requested_format == 'xml'
        super(xml: payload) && return
      elsif requested_format == 'jsonapi'
        super(jsonapi: payload) && return
      end
    end

    super
  end

The request_format parsing and rendered_format parsing both leave a lot to be desired. But whatever you do, don't try to use String#slice! when parsing as it causes all sorts of buggy issues (in Rails 4.2 anyway). And for reasons I don't understand the calls to super somehow loop back and invoke this render method recursively when I don't think it should. If not handled properly, infinite loops are possible. The JSON:API response behaves properly on the client end, but the content is simply the original JSON, nothing special. So that could use some work. I feel properly gross about all this, so no need to pile on. :wink:

All regular requests and responses seem to pass through well. Errors are returned in the format they were requested (although JSON:API stuff is simply JSON content wrapped in a JSON:API MIME type), and non-supported formats still raise an ActionController::UnknownFormat error. I've registered that error now with the exception handler, which returns as JSON. This is the only case now where an error will be returned blindly as JSON, as there was no known format to align with in the request.

Maybe one day we can convince our main API user to work in JSON. :rofl:

richmolj commented 2 years ago

Appreciate the update and sharing the hack in case someone else is ever unlucky enough to have the XML requirement. Sorry we couldn't figure out a more elegant solution but glad it's at least working!

Ben-Fenner commented 2 years ago

For anyone in the future trying to use the hack above, we've slimmed it way down and removed a 2nd loop that was happening only during JSON:API formatting.

So for Rails 4.2.0 this is our current solution:

  def render(*args, &block)
    requested_format = request.format.to_s
    response_format  = response.content_type

    if requested_format == 'application/xml' && response_format != 'application/xml'
      payload = args.first[:json]
      super(xml: payload, &block) && return
    elsif requested_format == Graphiti::Renderer::CONTENT_TYPE && response_format != Graphiti::Renderer::CONTENT_TYPE
      payload = args.first[:json]
      super(json: payload, content_type: Graphiti::Renderer::CONTENT_TYPE, &block) && return
    end
    super(*args, &block)
  end

The XML stuff was always easy enough, it's the JSON:API stuff that's problematic. Because it is not a typically understood format to render it needs special treatment. We decided to set the response content type ourselves (so the client/browser treats it properly) while rendering the result as JSON (so Ruby symbols are converted to strings). This avoids the second (recursive) loop and the need to protect against it.

Keep in mind this still doesn't actually return JSON:API, just JSON with a MIME type of JSON:API. So you'll need to serialize things yourself if you want real JSON:API.

If you're serving HTML through your API controllers, you might need to augment this code a little bit. Maybe. I haven't checked.

Cheers! :rofl: