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

Wrong headers definition in withDecoder #328

Closed
valeriobelli opened this issue Oct 10, 2020 · 1 comment · Fixed by #329
Closed

Wrong headers definition in withDecoder #328

valeriobelli opened this issue Oct 10, 2020 · 1 comment · Fixed by #329
Assignees
Labels
Milestone

Comments

@valeriobelli
Copy link

Is your feature request related to a problem? Please describe.
Inside the withDecoder combinator is used the withHeaders to inject automatically the headers:

  • Accept: application/json
  • Content-Type: application/json

The ratio is clear and it's to indicate to a backend service the needs of a JSON-like response body to be decoded. Actually, this need is indicated using the Accept request header, as described by https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept.

Therefore, the Content-Type: application/json that is injected automatically ends to be useless since a request can be sent also with other types of data that should/could be defined by the end-user. Moreover, it can also be a source of bugs if it's put at the end of a functional pipeline, as in the following example:

pipe(
  request,
  withHeaders({ 
    'Content-Type': 'application/x-www-form-urlencoded;charset=UTF-8'
  }),
  withDecoder(codec),
)

Describe the solution you'd like

The solution I thought is pretty simple: removing the Content-Type from the withHeaders combinator used inside withDecoder combinator moving the responsibility of setting the media type with Content-Type to the end-user.

@StefanoMagrassi StefanoMagrassi self-assigned this Oct 13, 2020
@StefanoMagrassi StefanoMagrassi changed the title Wrong headers definition in withDecoder Wrong headers definition in withDecoder Oct 13, 2020
@StefanoMagrassi
Copy link
Contributor

@DrugoLeb thank you for your hint.

I changed the tag of the issue into "bug" because, as you pointed out, the Content-Type header on request side can be other than application/json and a different value provided by the user could be wrongly overriden

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

Successfully merging a pull request may close this issue.

2 participants