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

Allow customizing the content type for errors #82

Open
ansman opened this issue Nov 18, 2013 · 10 comments
Open

Allow customizing the content type for errors #82

ansman opened this issue Nov 18, 2013 · 10 comments

Comments

@ansman
Copy link
Contributor

ansman commented Nov 18, 2013

Currently all default responses are returned as text/plain.

I'd love to be able to have some way of rendering responses as JSON instead.

@ordnungswidrig
Copy link
Member

After content-negotiation has happened the responses will send as the negotiated content types. I'm currently thinking about how to specify media-types for error responses, especially before content negotation has happened.

You can always send the media type in a custom handler, either as a ring response wrapping it with "liberator.representation/ring-response" or by forcing the media type like this:

(defresource foo
  :exists? false
  :handle-not-found (fn [ctx] (as-response {:error :not-found}
                              (assoc-in ctx [:representation :media-type] "application/json")))

@ansman
Copy link
Contributor Author

ansman commented Nov 18, 2013

I'd rather not override every single error.

Also, it would be nice to be able to customize the format of the JSON returned.

But perhaps I should just create a ring middleware that looks for plan/text requests and converts them to application/json?

@ordnungswidrig
Copy link
Member

So how should the JSON then look like in general?

@ansman
Copy link
Contributor Author

ansman commented Nov 19, 2013

Well that's the tricky part, not sure if there is one size that fits all here.

Though the format I'm currently using is

{
  "message": "<message>"
}

@ordnungswidrig
Copy link
Member

Unfortunately, there is no one-size-fits-all. Future changes in the representation generation will allow you to override the JSON generation in one place, and, thus, allow you to render a String as the above JSON representation.

@ansman
Copy link
Contributor Author

ansman commented Nov 20, 2013

That would be awesome!

@timvisher
Copy link

Not sure if I should open a new issue for this.

Consider the following:

(require '[ring.mock.request :as r])
((ANY "/" [] (liberator/resource
              :exists? (fn [ctx] false)
              :available-media-types ["application/json"]))
 (r/request :get "/"))
;; {:status 404, :headers {"Content-Type" "text/plain"}, :body "Resource not found."}

((ANY "/" [] (liberator/resource
              :handle-ok (fn [ctx] "charnock")
              :available-media-types ["application/json"]))
 (r/request :get "/"))
;; {:status 200, :headers {"Vary" "Accept", "Content-Type" "application/json;charset=UTF-8"}, :body "charnock"}

This behavior is unexpected to me. Notice that the body of neither of those responses is valid json. I would've expected the response of both to be "\"the thing\"".

Perhaps the path forward would be to attempt to json-str the response if it isn't already valid json? The weirdness is needing to support passing a json str back to the client, but that's why you could check it for validity if it's a string and otherwise call json-str on it.

Raising an exception would be better than lying about the content-type in any case. :)

@ordnungswidrig
Copy link
Member

@timvisher I don't get it. Maybe you missed the part that should generate "\"the string\""?

@ordnungswidrig
Copy link
Member

@timvisher Ok, I think now I got it :-) This is indented behaviour. Liberator will return a string result from a handler as the response body. This decision had been made for convenience as it covers most of the use cases for text bases media types. If you want to work around this, then you can specify your own function at :as-response which converts the handler result to a ring response.

The documentation at http://clojure-liberator.github.io/liberator/doc/representations.html should cover all the cases. Feel free to ask if you still have a question.

@timvisher
Copy link

@ordnungswidrig I guess my point is that liberator isn't obeying the content-type here. So to truly get a resource with only json available, I need to specify the media type and then for a certain class of responses override them to actually return json while for others the json stringifying is done for me.

Like I said, you can't just blindly encode all strings that get returned to you a second time because that would break people's ability to change arbitrary things into a json string.

That said, I think would be entirely reasonable to out of the box validate that the json response is in fact valid json and if it isn't, attempt to encode it. If that fails, an exception would be thrown and a 500 would happen at the server where it belongs, rather than forcing all clients to handle the case where they might be being lied to about the body. It seems like a very reasonable default and then of course the overrides you already mentioned could be used where something different than the default is needed.

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

No branches or pull requests

3 participants