Clojurians Log v2

Do you have thoughts on fixing this? It seems like there could be code in the wild that is depending on this behavior. Technically it is a bug fix but functionally could be a breaking change.

I think fixing it in ring-http-response and marking it as a breaking change would probably be right

In your first code block "content-type" is being assoced directly into the response map, and not the :headers map of the response.. which seems wrong to me.

.. and what do you mean by "works" and "does not work"?

good catch. Let me check it again and I will verify if the behavior I’m seeing is still happening. I am probably picking up default behavior currently with the “working” example (which will explain in a moment)

ooof I'm blind, thanks for spotting that @ramblurr

yeah, it was picking up default but with correct code, seeing same behavior. Here are the details:

> I am probably picking up default behavior currently with the “working” exampl Yes, I'm still not sure what wasn't working, but I do know that muuntaja defaults to json.

Here is the corrected function

(defn handler
  "Handler function for the `/health` end-point"
  [request]
  (-> {:status "healthy" :message "All good, Captain!"}
      (resp/ok)
      (update :headers assoc "content-type" "application/json")))

(defn handler
  "Handler function for the `/health` end-point"
  [request]
  (-> {:status "healthy" :message "All good, Captain!"}
      (resp/ok)
      (resp/content-type "application/json")))

Yea that is likely what is happening.

My guess as to why this hasn't been raised before is that I suspect most people do not use helpers like resp/content-type preferring to manipulate the response map directly.

yes, that is my guess as well, or they are just not setting content type and getting the default, which is almost always going to be JSON.

suffice to say, fixing it is a breaking change.

if you are using muuntaja in the "normal" way, you shouldnt be setting the content type yourself

that's one of the whole purposes of muuntaja, to negotiate the content type and then serialize the response body into the negotiated format (whether that be json or otherwise). muuntaja will set the content-type header.

Is it standard that the negotiated response type is the same as the request type?

try omitting the setting of the content type in your response entirely, then curl your endpoint with -H "Accept: application/edn" or -H "Accept: application/json" or -H "accept: application/transit+json"

if muuntaja is correctly configured, then you will see the response body encoded in the requested format

Cool. I will test that out. I’m pretty confident it will work and is configured correctly as the incorrect function you pointed out above was likely picking up defaults since the content-type was not being set into the headers map.

Thank you for your help.

the bug in ring-http-response still exists, even though it doesn't impact you

I can see that it gets fixed

Is it standard that the negotiated response type is the same as the request type?I can't easily answer yes or no to that question because "request type" is ambiguous. Rather, I will say that the client should send an Accept: header that lists the content types it requests, usually the first one is the preferred one. The server then parses that header, compares it to an internal list of what it supports (with muuntaja this is json, edn, transit, etc) and then uses that type for the response. Muuntaja automates that all for you, so you just return clojure data structures in your :body and muuntaja does the rest.

> the bug in ring-http-response still exists, even though it doesn't impact you yea indeed. and just looking at that ns, I see other instances of Title Cased headers too, not just Content-Type

tracked here: https://github.com/metosin/ring-http-response/issues/35

yep... looks like muuntaja only uses lower-case https://github.com/metosin/muuntaja/blob/master/modules/muuntaja/src/muuntaja/core.clj#L79-L84

I wonder how this isn't a more widespread problem in ring. I've never seen a case-insensitive-header-lookup helper fn

ah, the ring spec says header keys should be lower-cased: https://github.com/ring-clojure/ring/blob/master/SPEC.md#headers-1

so definitely a bug in ring-http-response

huh interesting

turns out the spec was wrong! and the ring 1.x spec doesn't have anything to say about the case of the headers

so the bug is probably https://github.com/metosin/muuntaja/blob/master/modules/muuntaja/src/muuntaja/core.clj#L79-L84 for expecting lower case headers

That is a more stable change. Expanding what is acceptable rather than narrowing it.

Indeed, though the metosin folks really care about perf and probably wouldn't like to make that change because of the extra overhead.