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

HTTParty response cache is broken #764

Open
seanneale opened this issue Oct 7, 2022 · 1 comment
Open

HTTParty response cache is broken #764

seanneale opened this issue Oct 7, 2022 · 1 comment

Comments

@seanneale
Copy link

seanneale commented Oct 7, 2022

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:

  • last_uri
  • raw_request
  • last_response

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)
  1. 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
Copy link
Owner

This seems related to #714 by @baberthal.

I have a fix in #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.

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

No branches or pull requests

2 participants