NoMethodError: undefined method [] for nil:NilClass in Gzip

[anaprimawaty]

The line case response_env[:response_headers][CONTENT_ENCODING] (gzip.rb#L26) raises NoMethodError: undefined method [] for nil:NilClass sporadically because response_env[:response_headers] is nil.

We currently handle this by raising a custom error NilResponseHeaders and retrying the request. Happy to push a fix upstream, but was wondering if it's preferable to assume there's no compression and skip the decompression instead?

[ruckus]

Interesting. Thanks for the heads-up and sleuthing.

was wondering if it's preferable to assume there's no compression and skip the decompression instead?

Yes this makes sense. I'm looking at that file and its not clear to me what the refactor would be. I think we'd need to write another method which just passes thru the raw body as-is - another method being the 2nd (block) argument passed to reset_body. I think ....

[anaprimawaty]

Ah.. do you mean something like this https://github.com/lostisland/faraday_middleware/commit/e95c96b90e5c1e45297c5a89fcd87352fe229c67 ? One issue could be that reset_body still attempts to access the response headers e.g. env[:response_headers][CONTENT_LENGTH], which would result in the same error.

Thoughts on just breaking if we detect there's nil response headers?

def call(env)
  env[:request_headers][ACCEPT_ENCODING] ||= SUPPORTED_ENCODINGS
  @app.call(env).on_complete do |response_env|
    break if response_env[:response_headers].nil?

    case response_env[:response_headers][CONTENT_ENCODING]
    when 'gzip'
      reset_body(response_env, &method(:uncompress_gzip))
    when 'deflate'
      reset_body(response_env, &method(:inflate))
    when 'br'
      reset_body(response_env, &method(:brotli_inflate))
    end
  end
end

[ruckus]

@anaprimawaty hah yeah that faraday commit is exactly what I pictured in my head. And you're right we'd also need to protect against headers being nil.

I think your suggestion of just breaking on commit is suitable. Would you be able to put that in a PR?

Thanks a ton for your work on this.