Return JSON by default for all API responses/errors

[DJTB]

Confirmed (at least partially) non-json routes: DELETE synonym/reading/:id/ POST auth/password/reset/ POST auth/password/reset/confirm/ POST contact/ POST review/:id/correct/ (403 error response) POST review/:id/incorrect/(403 error response)

[tadgh]

Currently there are some endpoints that return pure html, or null response bodies. We want there to at least be empty JSON at every route. Ideally though, in case of error, an "error" field which indicates what happened (if it makes sense from a security perspective)

[DJTB]

This might be useful to standardise all responses: https://github.com/django-json-api/django-rest-framework-json-api Following this format: http://jsonapi.org/format/

Could be overkill at the moment and require API rewriting for both of us rather than just modifying what you already have though.

However, it might combine well with writing your WK V2 API changes. Up to you to investigate/decide.

[tadgh]

Just checked over that library, and the spec, and it would be a pretty big overhaul. For now, I'm just gonna fix up the endpoints that aren't behaving nicely, but we should keep thius in mind if it ever becomes important to follow the exact JSON API spec

[DJTB]

Yeah no worries, was just throwing it out there.

Note: Obviously I'll have to deploy my changes to handle the modified endpoints at the same time as you for this

[tadgh]

Hm, you'll need to make changes to accomodate just the regular endpoint change? e.g. contact form now returning a {"detail": "Successfully sent"} body?

[tadgh]

Also, the RFC for response codes specifically states to return no Response Body upon returning a 204, which is what DRF returns during deletion. So i'll be able to fix up everything but the deletes, which will continue to return 204. I have added a default renderer which adds dummy data in the case that none is provided by the endpoint

[DJTB]

I'll need to make some changes, but actually it should probably be safe. The issue before was calling a json resolver on non-json which would error when parsing plain text etc since the json syntax was "wrong". Though calling the plain text resolver on json should just return a literal string of the json.

I'll check locally and let you know.

[tadgh]

Sounds good. For now, anything that returns 204 will have no response body, but everything else should at least have some empty json now.

[tadgh]

Let me know when, and I'll deploy

[DJTB]

So only DELETEs will be empty / no json? What about 500? I think that was text with <h1>something something</h1>

[tadgh]

I don't see that mentioned in the original ticket. the 403s mentioned are taken care of, is there a specific 500? or any 500?

[DJTB]

I think any. Sentry will catch the failed json parsing if it does occur, so I can update you if something was missed.

[tadgh]

kk. I'll write a test to check a 500 anyhow. Sadly if its an NGINX 5XX, i don't think i have as much control over that... I'll have to find out.

[DJTB]

Don't worry about it - it's not like there will be different messages for the 500. I'll handle that status code separately.