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

Error when use retry and cache middleware #1238

Closed
isakovp opened this issue Jan 18, 2021 · 5 comments
Closed

Error when use retry and cache middleware #1238

isakovp opened this issue Jan 18, 2021 · 5 comments

Comments

@isakovp
Copy link

isakovp commented Jan 18, 2021

Basic Info

  • Faraday Version: 0.15.4
  • Ruby Version: 2.7.1

Issue description

Error when use retry and cache middleware

NoMethodError: undefined method 'fetch' for nil:NilClass
  from faraday_middleware/response_middleware.rb:78:in 'preserve_raw?'
  from faraday_middleware/response_middleware.rb:39:in 'process_response'
  from faraday_middleware/response_middleware.rb:33:in 'block in call'
  from faraday/response.rb:61:in 'on_complete'
  from faraday_middleware/response_middleware.rb:31:in 'call'
  from faraday_middleware/request/encode_json.rb:24:in 'call'
  from faraday/request/authorization.rb:37:in 'call'
  from faraday/response.rb:8:in 'call'
  from faraday/request/retry.rb:128:in 'call'
  from faraday/rack_builder.rb:143:in 'build_response'
  from faraday/connection.rb:387:in 'run_request'
  from faraday/connection.rb:138:in 'get'

Steps to reproduce

  1. Use cache middleware and use Redis as cache
  2. Use retry middleware and retry 500 error (as example API rate limit)
  3. Start 2 rake tasks that use the same Faraday.get method

Job 1 try to do get but failed with 500 error
Job 2 do get and response cached
Job 1 retried and get response from cache, but env[:response_headers] not null, it contains headers from first call and this code ignored
https://github.com/lostisland/faraday_middleware/blob/master/lib/faraday_middleware/response/caching.rb#L115

As temporary fix we use retry_block option for retry middleware
retry_block: Proc.new { |env| env[:response_headers] = nil }

@iMacTia
Copy link
Member

iMacTia commented Jan 18, 2021

Hi @isakovp, could you please share your Faraday connection setup so we can see all the middleware involved and their order in the stack? thanks!

@isakovp
Copy link
Author

isakovp commented Jan 19, 2021

Faraday.new(options.merge({ url: ENDPOINT_URL,
                            request: { timeout: 120.second.to_i },
                            headers: { 'User-Agent' => 'appw.import2.com' } })) do |connection|
  connection.request :retry, max: 5, interval: 5.seconds, backoff_factor: 2, exceptions: [
      Faraday::Errors::TooManyRequests, # retry api limits
      Faraday::Error::ConnectionFailed, Faraday::Error::SSLError, Errno::EPIPE, # connection errors
      Errno::ETIMEDOUT, 'Timeout::Error', Faraday::Error::TimeoutError, Faraday::Errors::Timeout, # timeouts
      Faraday::Errors::InternalServerError, Faraday::Errors::BadGatewayError, Faraday::Errors::ServiceUnavailable, Faraday::Errors::GatewayTimeout, # server side errors (5xx)
      Faraday::Errors::Unauthorized, # retry unauthorized calls -> API token expired
      Faraday::Error::ParsingError
  ]

  connection.use Faraday::Response::AdvancedRaiseError
  connection.use Faraday::Request::AccessTokenAuthorization, token

  connection.request :json
  connection.response :json, content_type: 'application/json'

  connection.response(:caching, cache) if cache_enabled && cache.present?
  connection.response(:logger, Rails.logger, { headers: true, bodies: true }) if http_debug_output?
  connection.adapter Faraday.default_adapter
end

@iMacTia
Copy link
Member

iMacTia commented Jan 19, 2021

Thanks @isakovp, the first thing I notice is that the retry middleware is at the very top of the stack, is that due to some particular reason? My suggestion would be to have the cache middleware on top instead, since you want to short-circuit the request if cached. The retry middleware instead makes more sense towards the bottom, I'd suggest to place it just before the logging middleware in your particular case.
The suggested stack order would be this:

connection.response :caching
connection.use Faraday::Response::AdvancedRaiseError
connection.use Faraday::Request::AccessTokenAuthorization
connection.request :json
connection.response :json
connection.request :retry
connection.response :logger
connection.adapter Faraday.default_adapter

Could you give this setup a try and let me know if it solves your issue?

@isakovp
Copy link
Author

isakovp commented Jan 20, 2021

Thanks, @iMacTia it solved my issue, but the code in Caching#finalize_response looks sucks

@olleolleolle
Copy link
Member

Sounds good, @isakovp, does that resolve the issue?

@iMacTia iMacTia closed this as completed Apr 11, 2021
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

3 participants