jsonapi-rails icon indicating copy to clipboard operation
jsonapi-rails copied to clipboard

If no JSON API payload was found should render JSON API compliant error

Open JoeWoodward opened this issue 7 years ago • 7 comments

Currently have strong params in place and when a request comes through without a compliant structure I end up raising ActionController::ParameterMissing because my strong params doesn't have anything to work from.

How should I be handling this? Should the gem maybe handle this?

JoeWoodward avatar Jan 31 '18 10:01 JoeWoodward

For now I'm monkey patching and rendering a jsonapi_error when the parsing fails. Not sure if this is the correct thing to do. The docs state that we MAY return 403 and can include errors.

# frozen_string_literal: false
# Override the default failure behaviour.
# With this patch if hash.nil? the controller will render jsonapi_errors
require 'jsonapi/rails/controller'

module Extensions
  module JSONAPI
    module Rails
      module Controller
        module Deserialization
          extend ActiveSupport::Concern

          class_methods do
            def deserializable_resource(key, options = {}, &block)
              options = options.dup
              klass = options.delete(:class) ||
                      Class.new(::JSONAPI::Rails::DeserializableResource, &block)

              before_action(options) do |controller|
                hash = controller.params.to_unsafe_hash[:_jsonapi]
                if hash.nil?
                  ::JSONAPI::Rails.logger.warn do
                    "Unable to deserialize #{key} because no JSON API payload w" \
                    "as found. (#{controller.controller_name}##{params[:action]})"
                  end
                  controller.render jsonapi_errors: {
                    title: 'Non-compliant Request Body',
                    detail: 'The request was not formatted in compliance with ' \
                      'the application/vnd.api+json spec',
                    links: { about: 'http://jsonapi.org/format/' }
                  }, status: 403
                  next
                end

                ActiveSupport::Notifications
                  .instrument('parse.jsonapi-rails',
                              key: key, payload: hash, class: klass) do
                  ::JSONAPI::Parser::Resource.parse!(hash)
                  resource = klass.new(hash[:data])
                  controller.request.env[JSONAPI_POINTERS_KEY] =
                    resource.reverse_mapping
                  controller.params[key.to_sym] = resource.to_hash
                end
              end
            end
          end
        end
      end
    end
  end
end

JSONAPI::Rails::Controller.include(
  Extensions::JSONAPI::Rails::Controller::Deserialization
)

I feel like this should probably be configurable though

JoeWoodward avatar Feb 01 '18 03:02 JoeWoodward

Before rendering jsonapi_errors should also check if the current Content-Type is actually application/vnd.api+json

JoeWoodward avatar Feb 01 '18 07:02 JoeWoodward

Agreed that the lib should at least raise a custom exception so that people can rescue from it with a sensible json:api error. Would you mind issuing a PR for this?

beauby avatar Feb 18 '18 12:02 beauby

@beauby any suggestion on how to add a status and links to the error response?


if event.save
  render jsonapi: event, status: :created
else
  render jsonapi_errors: event.errors, status: :unprocessable_entity
end
{
    "errors": [
        {
            "title": "Invalid user",
            "detail": "user must exist",
            "source": {}
        }
    ],
    "jsonapi": {
        "version": "1.0"
    }
}

status and links are the attributes required by JSON API Spec.

veelenga avatar Aug 14 '18 22:08 veelenga

Didn't see this response. I will try to come up with an elegant solution

JoeWoodward avatar Sep 02 '18 08:09 JoeWoodward

Maybe rather than using an exception to handle the flow control the controller should have a customizable error response.

i.e.

In the config file you can set the default error response hash for this situation config[:jsonapi_payload_error_hash]

Then we add a hook to the controller that would call controller.jsonapi_invalid_payload_error

Which could look something like

def jsonapi_payload_error
  render_error_for_action = "jsonapi_payload_error_for_#{controller.action_name}"
  return public_send(render_error_for_action) if controller.responds_to?(render_error_for_action)
  render jsonapi_errors: JSONAPI::Rails.config[:jsonapi_payload_error_hash]
end

What do you think @beauby

JoeWoodward avatar Sep 02 '18 08:09 JoeWoodward

@veelenga your question is related to this https://github.com/jsonapi-rb/jsonapi-rails/issues/86, it's not related to the issue outlined here.

However, one note is that none of the errors attributes are actually required, they are optional

An error object MAY have the following members:

That said I think there should be a way to map these values. I will try to conceive a means of doing so

JoeWoodward avatar Sep 02 '18 09:09 JoeWoodward