Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
JoeWoodward opened this issue Jan 31, 2018 · 7 comments

Comments

@JoeWoodward
Copy link

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
Copy link
Author

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
Copy link
Author

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

@beauby
Copy link
Member

beauby commented Feb 18, 2018

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?

@veelenga
Copy link

veelenga commented Aug 14, 2018

@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.

@JoeWoodward
Copy link
Author

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

@JoeWoodward
Copy link
Author

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
Copy link
Author

@veelenga your question is related to this #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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants