Pass headers to process response* functions #118
Conversation
chewbranca
force-pushed
the
pass-headers-to-process-response
branch
2 times, most recently
from
d0abd99 to
c74ec66
Compare
|
I dropped out the commit for The test suite is failing in three places for me, but it fails the same three tests on master and on this branch. |
|
Yeah the three failures in https://travis-ci.org/myfreeweb/httpotion/jobs/313695533 are the same I'm getting on master and on this branch. |
|
https tests are broken, don't pay attention to them :) |
lib/httpotion.ex
Outdated
| status_code: process_status_code(status_code), | ||
| headers: process_response_headers(headers), | ||
| body: process_response_body(body) | ||
| body: process_response_body(headers, body) |
There was a problem hiding this comment.
The headers were just processed with process_response_headers(headers), I'm not sure raw headers should be used here. Maybe extract process_response_headers(headers) into a variable and use it both in the returned headers: and in the argument to this?
lib/httpotion.ex
Outdated
| status_code: process_status_code(status_code), | ||
| headers: process_response_headers(headers), | ||
| body: process_response_body(body) | ||
| body: process_response_body(headers, body) |
|
Switched to using the processed headers in ce64a04, let me know if you have a preference on the temp var name or if that's sufficient. |
lib/httpotion.ex
Outdated
| def handle_response(response) do | ||
| case response do | ||
| { :ok, status_code, headers, body, _ } -> | ||
| headers1 = process_response_headers(headers) |
There was a problem hiding this comment.
Resist the Erlang! You can rebind headers here rather than using versioned variable names.
There was a problem hiding this comment.
Yeah but we know what it's actually doing so rebinding feels dirty... but hey, that's why I asked if @myfreeweb has a preference, I can switch to rebinding if preferred.
There was a problem hiding this comment.
Something like processed_headers would be better than headers1. Rebinding is fine too I guess.
chewbranca
force-pushed
the
pass-headers-to-process-response
branch
from
ce64a04 to
0f60cf0
Compare
|
@myfreeweb alright, I've changed the variable name to |
|
Great, thanks! |
|
@myfreeweb np! Any chance you could push out a new release with this in it when you get a moment? |
There were couple of hacks in test/elixir/lib/couch.ex We've got changes needed to remove them into httpotion 3.1.3. The changes were introduced in: - valpackett/httpotion#118 - valpackett/httpotion#130
There were couple of hacks in test/elixir/lib/couch.ex We've got changes needed to remove them into httpotion 3.1.3. The changes were introduced in: - valpackett/httpotion#118 - valpackett/httpotion#130
3 participants
The
Content-Typeresponse header can be crucial for properly decoding the response. This PR creates two arity variants ofprocess_response_bodyandprocess_response_chunkfor passing in the response headers, with default implementations falling back to the standard behavior by way of the one arity versions.