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

Expect and the "Accept:" header #54

Open
rtfeldman opened this issue Dec 27, 2018 · 1 comment
Open

Expect and the "Accept:" header #54

rtfeldman opened this issue Dec 27, 2018 · 1 comment

Comments

@rtfeldman
Copy link
Member

Motivation

Our Rails back-end uses the Accept HTTP request header to determine what to send back to clients. It defaults to sending HTML unless told otherwise.

When we make a HTTP request that expects a JSON response, if the user's authentication session has expired, here's what happens:

  • Rails detects this and sends a 401 response
  • Because it did not see an Accept header, it defaults to sending back HTML - a fully rendered "You need to log in" HTML page
  • Because the Elm request expected a JSON response, it tries to run a JSON decoder on this HTML, which of course blows up because HTML is not JSON

In the NoRedInk/elm-rails package, we explicitly set the Accept header on all JSON requests to application/json, which prevents this.

However, sometimes we use other packages that do HTTP, and if they do not set this header, and we don't remember to override it to set it properly, we end up with production bugs when users are not authenticated.

Proposal

Http.Body stores a String for what Content-Type header should be set in the request. Http.jsonBody uses this to set Content-Type: application/json on any request that has a JSON body.

Http.Expect could similarly store a String for what Accept header should be set in the request. Http.expectJson could use this to set Accept: application/json on any request that expects a JSON response.

This would address this problem for anyone using elm/http with Rails or other servers that use the Accept header to make decisions about what format to use for responses.

@ymtszw
Copy link

ymtszw commented Aug 15, 2019

I think this is reasonable and sound way of content-negotiation. We should definitely achieve this symmetry.

While I was updating my elm-http-xml package (https://github.com/ymtszw/elm-http-xml) to support elm/[email protected], I've found Http.Request data type was no more.
In older version of elm-http-xml, I've been supplying Accept: application/xml header at this layer.
But in 2.x of elm/http, there is no suitable interface to atomically add Accept header AND introduce corresponding decoding functions (in my case via Xml.Decode.Decoder type).

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

No branches or pull requests

2 participants