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

Render error when unable to deserialize resource #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JoeWoodward
Copy link

Adds config option to set what happens when unable to deserialize payload

  # config.jsonapi_payload_malformed = -> {
  #   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: :bad_request
  # }

Able to override on controller by controller basis
Tested!

@beauby
Copy link
Member

beauby commented Apr 5, 2019

Thanks for all your work @JoeWoodward, and sorry not to have replied earlier.
I would lean towards mimicking what happens when a broken JSON payload is set with content-type application/json, which is, IIRC, to raise an exception, which can be rescued by the controller. What do you think?

@JoeWoodward
Copy link
Author

No worries @beauby, open source is hard work. It's like a second job :D. I'm happy you made these gems in the first place, they're really good.

I was trying to avoid using exceptions for flow control as I don't believe it really adds any benefit, I believe most libraries use it as they can't be sure where they are in the flow so it's easier to just blow an exception and let the developer handle it. In our case we do have access and control of the controller so IMO makes more sense to actually render an error

I think this solution has another benefit too, if we raise an exceptions which is not handled the server will return a 500 error (which I just realized is currently not handled so probably also needs a PR) which is not very helpful when you're trying to use a json api. I actually implemented this for that exact reason, the mobile devs were complaining that they weren't sure if it was because of their code or mine when they were getting 500 errors when my controller actions where trying to access the nil deserialized object.

You made me realize something, if the config is nil then I am returning in this PR. I think that should actually raise an error

https://jsonapi.org/format/#errors-processing from this link, the spec does permit us responding with 4XX errors

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

Successfully merging this pull request may close these issues.

2 participants