search

Vacasa / drf-jsonapi

JSON API with Django Rest Framework
MIT License
3 stars 5 forks source link

Some responses have the wrong Content-Type header #3

Open efryvc opened 6 years ago

efryvc commented 6 years ago

Describe the bug 4xx and 5xx errors in particular have been noted

To Reproduce Steps to reproduce the behavior:

eg. go to / get a response with a content-type of text/html

Expected behavior Content-type header on ALL non-empty jsonapi responses should be application/vnd.api+json

chrisbrantley commented 5 years ago

@efryvc after looking into this I'm not convinced this is a bug.

I've tested responses from the API and 4xx responses DO return with the correct content-type of application/vnd.api+json as long as the Accept header is either not set (default value) or is set to application/vnd.api+json.

However, if you try to visit an invalid url like your example of /<resource_that_does_not_exist> that does not have a route the API is never invoked and therefore you get the default content type according to settings.py. The default is text/html. You can override this value to whatever you want: https://docs.djangoproject.com/en/2.1/ref/settings/#default-content-type

For apps that only expose API endpoints it should be fine to set DEFAULT_CONTENT_TYPE ="application/vnd.api+json". If your app is a mixed service and handles API requests as well as serving up html that may not be appropriate.

To me this issue seems outside of the scope of this package.

efryvc commented 5 years ago

@chrisbrantley I see where you're coming from.

Part of me wants to put that DEFAULT_CONTENT_TYPE into drf-jsonapi/settings.py (if there was one), though since it would be incorrect for your mixed service example, it's not a big part.

It might be worth calling this out in the README. I know that we noticed it while testing, and spent a few cycles trying to solve it, and something in the docs would be helpful to people who don't know about this setting in Django.

chrisbrantley commented 5 years ago

@efryvc Great idea. I'm in the process of updating the README now so I'll add that.

efryvc commented 5 years ago

+1

On Jan 28, 2019, at 7:26 PM, Chris Brantley notifications@github.com wrote:

@efryvc Great idea. I'm in the process of updating the README now so I'll add that.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.