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

Version is missing in the response headers #70

Open
nicolaa opened this issue Apr 2, 2019 · 10 comments
Open

Version is missing in the response headers #70

nicolaa opened this issue Apr 2, 2019 · 10 comments

Comments

@nicolaa
Copy link

nicolaa commented Apr 2, 2019

Problem:
ApiVersionConstraint requires clients to send Accept request headers that specify the API version, for example: Accept: "application/json; version=2". However, the API response does not match this expected behavior.

Solution:
Add a default header to Api::ApiController that responds with the correct version.

For example: Content-Type: "application/json; version=2"

@Aliciawyse
Copy link

Hey there @nicolaa!

I'm working on this issue now (with a mentor through a program called RubyMe). I'll be sending a PR very soon! :-)

Aliciawyse added a commit to Aliciawyse/stitches that referenced this issue Jun 10, 2019
In issue stitchfix#70 we see that response headers do not respond with the correct version.

This adds a rack middleware that overrides the Content-Type header to show the version.

Co-authored-by: Juan Carlos Ruiz<[email protected]>
@brettfishman
Copy link
Contributor

Hi @nicolaa - can you say a bit more about why this is a problem?

@nicolaa
Copy link
Author

nicolaa commented Jun 11, 2019

@brettfishman This actually came up when one of our service consumers wrote pact expectations for header values. They were calling the v2 endpoint and specifying application/json; version=2 for content-type and accept headers in the request, and expecting version=2 to appear in the content type response header. Since this is something we don't actually read in the application, we just removed those expectations; however, it did reveal that stitches is not propagating the information correctly.

@brettfishman
Copy link
Contributor

brettfishman commented Jun 11, 2019

@nicolaa Ah, got it. Thanks for adding the additional context. I agree that it seems appropriate for the server to acknowledge the version returned in the Content-Type response header.

UPDATE: In light of section 3.1.1.5 in the RFC for Content-Type I'm re-thinking this a bit. I'm going to sleep on it.

@brettfishman
Copy link
Contributor

@nicolaa So after thinking on this a bit more, and conferring with @bwebster on it, I'm not sure we are convinced that this is an issue worth solving at this point for a couple reasons:

  1. API consumers, as far as I know, currently do not depend upon the Content-Type header in the response to do any verification of the version returned. I'm not saying that this will always be true, but we do not currently have a use-case out there in the wild.

  2. As noted above, the specification for this header talks about it containing a media type (e.g. application/json) along with how it is encoded (e.g. charset=utf8). The example you gave above (Content-Type: "application/json; version=2" omits the charset=utf8 part, and I haven't found evidence to suggest this is standard, or at least an acceptable alternative.

Would you be able to point me to a source that supports your suggested approach?

@nicolaa
Copy link
Author

nicolaa commented Jun 13, 2019

@brettfishman I think you may be right. I'm not sure the version info should be tied to the Content-Type header either. HOWEVER, we do attach it to both the Content-Type and Accept headers in the Request, and there is no corresponding Accept header in the Response, so it feels like there is still a gap somewhere. I think most REST standards propose a customer header like Accept-Version to handle this.

I don't feel super strongly about this either way, but in the example i was referring to, the v1 and v2 response bodies are different, so I do think that there should be some way in the Response to differentiate what "Type" of response body we are sending.

@brettfishman
Copy link
Contributor

I do think that there should be some way in the Response to differentiate what "Type" of response body we are sending

@nicolaa I agree with this statement. I don't have bandwidth at the moment to research other potential solutions, but would welcome a proposal for how to achieve this. I don't think the current proposal totally works, but I'm happy to revisit my POV if a compelling solution is found.

@nicolaa
Copy link
Author

nicolaa commented Jun 13, 2019

@brettfishman TBH, I don't think a perfect solution exists. These are known limitations of HTTP. We are really trying to build on top of an infrastructure that wasn't designed for our needs.

@brettfishman
Copy link
Contributor

@nicolaa yeah, agreed. To be clear, I'm definitely not seeking a perfect solution.

@Aliciawyse
Copy link

Aliciawyse commented Jun 13, 2019 via email

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

Successfully merging a pull request may close this issue.

3 participants