jnunemaker / httparty

:tada: Makes http fun again!
MIT License
5.81k stars 964 forks source link

HTTParty response cache is broken #764

Open seanneale opened 2 years ago

seanneale commented 2 years ago

Rails 6.0.5.1 Ruby 2.7.6

We have recently upgraded HTTParty to v0.20.0 and found that the cached response is missing some information from the response.request:

We require raw_request for our logging, so have reverted to v0.19.1

Scripts to reproduce the issue:

  1. Script closer to the root cause
    url = "https://avatars.githubusercontent.com/u/12815721?s=200&v=4"
    r = HTTParty.get(url);nil
    r.request.raw_body
    # => nil
    Marshal.load(Marshal.dump(r)).request.raw_body
    # expected: nil
    # actual: NoMethodError (undefined method `body' for nil:NilClass)
  2. Script closer to our scenario
    
    # Enable cache in the local
    make bash-devel
    rails dev:cache
    bin/rails c

url = "https://avatars.githubusercontent.com/u/12815721?s=200&v=4" ​

set cache

r_2 = Rails.cache.fetch("sean_test", expires_in: 5.minutes, race_condition_ttl: 5, force: true) do r = HTTParty.get(url) end ​

test if body available (it will be)

r_2.request.raw_body ​

fetch cache

r_2_cache = Rails.cache.fetch("sean_test", expires_in: 5.minutes, race_condition_ttl: 5) do HTTParty.get(url) end; nil ​

test if body available (it won't be)

r_2_cache.request.raw_body

jnunemaker commented 2 years ago

This seems related to https://github.com/jnunemaker/httparty/pull/714 by @baberthal.

I have a fix in https://github.com/jnunemaker/httparty/pull/767.

I think the best thing is to serialize all the ivars and then re-initialize them but this should at least solve this issue and not cause backwards compat issues (hopefully).

@seanneale That said, I would not recommend digging into internals like you are. One of the things you are accessing is just an ivar which if not given a method should really be treated as private. I'll think on that PR and see if anything else should go in. Should be able to merge soonish.