Add http reason phrase to cache #134
Conversation
| end | ||
|
|
||
| it 'merges the reason phrase' do | ||
| expect(response.reason_phrase).to eq('Success') if response.respond_to?(:reason_phrase) |
There was a problem hiding this comment.
This guard is a bit of a smell, but is needed for the specs to pass with older versions of Faraday (~>0.8.0). Would love your input for if there is a better way to write this spec.
There was a problem hiding this comment.
Just to understand it... In this case you're specifying the :reason_phrase in the spec's subject. Why it would break the spec in older versions of Faraday?
There was a problem hiding this comment.
@garaujodev ah, there are two separate types here. subject is a Faraday::HttpCache::Response and response is a Faraday::Response (and is built via subject.to_response(env)).
The issue this guard is solving for is that Faraday::Response#reason_phrase was added after Faraday 0.8.11 was released (docs for 0.8.11 here). So, when the specs are run in CI with Faraday ~> 0.8.0, a NoMethodError is raised.
There was a problem hiding this comment.
Got it. Thanks for the PR and the explanation ❤️
|
@wevtimoteo thanks for the review on this! Do you have a schedule for when new gem versions get published? |
Hi @ethanis, I think @garaujodev is planning to release it as |
|
@ethanis done ✔️ Thanks for all ❤️💜💙 |
3 participants
Description
This adds the reason phrase from the http response to the data stored in the cache. This is useful information to be able to inspect the underlying causes for unsuccessful (non 20x status code) responses that are read from the cache.
There should be no issues with backwards compatibility as
hash[:reason_phrase]will simply returnnilif it hadn't been previously stored in the cache.Hows this tested?
Specs ✅