Gracefully handle responses with no content type header

[mightybyte]

I've been playing with trasa recently (really liking it so far), and the first route I tried to implement failed to return a valid response because the server happened to not supply a content type header in the request. It seems like this behavior severely limits the usefulness of trasa-client for scraping arbitrary websites. I'm willing to put together a pull request to fix this but I thought I'd ask what you think would be the right solution before I do so. Any ideas?

[andrewthad]

For what it's worth, I've not been using this library lately since I've not been doing any GHC in the browser anymore. I've ended up settling on simpler, less type-safe solutions. That aside, I would take a PR for this. Here are my thoughts:

dispatchWith takes a Maybe Content as its argument. This is fine and should be Nothing when a request body is empty. However, over in Server.hs, the way that Content-Type is looked up causes a missing Content-Type to result in a Nothing body. One simple possibility is to make serveWith accept a default Content-Type argument, and then any missing Content-Type would just get this default one assigned to the content instead. I like this option because its simple and noninvasive, but if you've got any other suggestions, I'm all ears.

[chessai]

FWIW, as another maintainer of this library, I still use it, in and out of production. Nothing further to add other than glad you like it!

On Mon, Dec 21, 2020, 09:09 Andrew Martin notifications@github.com wrote:

For what it's worth, I've not been using this library lately since I've not been doing any GHC in the browser anymore. I've ended up settling on simpler, less type-safe solutions. That aside, I would take a PR for this. Here are my thoughts:

dispatchWith https://github.com/haskell-trasa/trasa/blob/fb70005a93c26b6d3dfe1956ccb06e64adf4f7e0/trasa/src/Trasa/Core.hs#L516 takes a Maybe Content as its argument. This is fine and should be Nothing when a request body is empty. However, over in Server.hs, the way that Content-Type is looked up https://github.com/haskell-trasa/trasa/blob/fb70005a93c26b6d3dfe1956ccb06e64adf4f7e0/trasa-server/src/Trasa/Server.hs#L101-L102 causes a missing Content-Type to result in a Nothing body. One simple possibility is to make serveWith accept a default Content-Type argument, and then any missing Content-Type would just get this default one assigned to the content instead. I like this option because its simple and noninvasive, but if you've got any other suggestions, I'm all ears.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/haskell-trasa/trasa/issues/21#issuecomment-749021083, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEOIX27HAPUFCB7GEKC2YVDSV5QI7ANCNFSM4VCIUA6A .

[mightybyte]

Here's my first stab at it that I threw together over the weekend which changes the MediaType field inside Content to a Maybe MediaType. What do you think about this approach?

https://github.com/mightybyte/trasa/compare/master...mightybyte:optional-response-content-type

[andrewthad]

I think that seems fine. My only criticism is about a small part of the implementation:

go (BodyDecoding medias dec:decs) mayMedia bod = let media = fromMaybe (NE.head medias) mayMedia in
  case any (flip mediaMatches media) medias of

It's slightly confusing to pick a default and then perform a lookup that is guaranteed to match the head of the list. I think you could leave the go function alone and case on media before even calling go. If it's Nothing, then there's no need to walk through the handlers (that is, no need to call go). Just pick the first one instead of calling go.

[mightybyte]

Ahh yes, that makes sense. I pushed a fix and opened a draft PR. How does that look?