Skip to content

Conversation

@Nitrodist
Copy link
Contributor

The body attribute would be set with the body from the response once a
request was completed. The other attributes of the request (url,
headers, etc.) are not clobbered because they don't share the same keys.

In the case of the headers, the work had already been done to separate
them out into separate attributes (response_headers vs.
request_headers). This does similar work.

Fixes #297

Depends on #389 since this breaks untested functionality in the Retry middleware. Once that has been merged, then this pull request needs to be modified so that the small amount of work from Nitrodist@3348e99 goes into it as well.

The body attribute would be set with the body from the response once a
request was completed. The other attributes of the request (url,
headers, etc.) are not clobbered because they don't share the same keys.

In the case of the headers, the work had already been done to separate
them out into separate attributes (response_headers vs.
request_headers). This does similar work.

Fixes lostisland#297
@Nitrodist
Copy link
Contributor Author

This probably warrants a major release since it's changing body to either response_body or request_body. Lots of code that uses Faraday will probably break with this.

Thoughts?

@sferik
Copy link
Member

sferik commented Jul 16, 2014

This is a big API change, so it needs input from @technoweenie and/or @mislav. Ideally, we could find a solution that does break things so badly.

@Nitrodist
Copy link
Contributor Author

I don't know much about the versioning the project is on, but according to the roadmap[0], the 0.9 version sounds like it's supposed to make these kind of big changes and 1.0 is the version that won't make big changes.

Granted, this was 2 years ago, but I think it still holds true. Maybe this is the feature that defines the release to 1.0? ⬜ 🐋

[0] - https://github.com/lostisland/faraday/wiki/Roadmap

@mislav
Copy link
Contributor

mislav commented Oct 6, 2015

I agree that this changes too much.

I get how a single body property can be ambiguous, especially as env[:body] where it holds request body during request phase and response body during respose. However, I don't think we need to go as far to redefine every body accessor in the codebase. I'm fine with API such as req.body = 'REQUEST' and response.body #=> 'RESPONSE'.

I would be more in favor of saving original request body in env so it isn't trumped over during response. Something like this was attempted in #517.

Right now there are no official plans for what goes in 1.0, but whenever there is a major release next, I don't want to change any more APIs needlessly because people already had a rought transition from 0.8 to 0.9 and I don't want to get them on a similar rollercoaster again.

@mislav mislav closed this Oct 6, 2015
@mislav mislav added the feature label Oct 6, 2015
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 this pull request may close these issues.

POST body is lost with Retry middleware

3 participants