Fork me on GitHub
#ring-swagger
<
2018-08-22
>
jdt11:08:39

As discussed some days ago (https://clojurians.slack.com/archives/C06GSN6R2/p1534530179000100) ('m using Muuntaja 0.6.0-alpha3). However it's caused some unit tests break. I'm here to ask if it's a bug or a feature, since I know nothing of msgpack standards. Basically the unit test that is breaking does a msgpack.core/unpack from [clojure-msgpack 1.2.1] on the response from the compojure-api app, and the returned data is no longer compatible. It results in a java.lang.ClassCastException: java.lang.Byte cannot be cast to [B. So what the new msgpack code packing isn't what the old client msgpack code wants to see. I don't know if there's a standard that's supposed to ensure the format of the data doesn't change, but I'm hesitant to go changing client code to use a new msgpack tool when the old one has worked all this time.

jdt11:08:17

time to read up on msgpack I guess, but could use some advice on whether this is a new alpha muuntaja bug, or whether I really should be upgrading the msgpack client.

jdt11:08:01

My impression is that my client msgpack (unpack) code shouldn't be breaking, but perhaps the argument can be made that msgpack.core is old and broken and so it's time to upgrade.

jdt11:08:34

Hmmm, 1.2.1 is the latest and greates of msgpack.core. Of course this may not be a msgpack issue at all, it could be the muuntaja and compojure-api alpha-23 upgrade have ... altered ... the encoding of what is returned by the route.

jdt11:08:44

The route in question declares a prismatic schema return type of

(s/defschema DocumentLayout
  (sweet/describe (s/pred bytes?) "Protocol buffer binary encoded data."))

jdt11:08:09

The only change in the broken APIs is the upgrade to compojure-api alpha23 and muuntaja 0.6.0-alpha3, and elimination of this line from the api format options:

-   :formats (m/create (-> m/default-options (msgpack-format/with-msgpack-format)))
Since that msgpack-format support is no longer present. I didn't see the need to declare any formats because the current defaults are supposed to include msgpack. Or do I need something afterall?

jdt11:08:22

Client headers are (mock/header "accept" "application/msgpack")

jdt12:08:36

Will re-examine the defaults, could have sworn current muuntaja said msgpack was a default supported format.

jdt12:08:39

Did the obsoleted with-msgpack-format support "application/msgpack" where the new msgpack defaults do not?

jdt12:08:15

The problem is not anything about the request body, the problem is the client breaks decoding the HTTP response from the compojure-api app

jdt12:08:56

So it's asking for "application/msgpack" as the "accept" header, which worked before, but now results in unpacking exceptions, perhaps because the server response isn't being encoded the way it was before.

jdt13:08:39

We had no special code to support that header, with-msgpack-format and the old muuntaja seems like it must have been supporting it before but not now.

jdt13:08:43

Or such is my guess.

jdt13:08:22

I'm not sure which other parts of the breaking changes in changelog might apply here, unless it's a need to set the Content-Type where we weren't before, re:

**BREAKING**: if a "Content-Type" header is set in a response, the body will not be encoded, unless `:muuntaja/encode` is set

jdt14:08:47

Interesting, if I change the client "accept" header from "application/msgpack" to "application/transit+msgpack", the content is sent back differently (but not correctly in either case). As far as I can tell we do NOT set any Content Type in the response.