HTTParty::Response fakes nil

[lesliev]

HTTParty::Response sometimes looks nil but isn't. This makes for some interesting consequences and strange code in order to deal with them:

[1] pry(#Salesforce::Rest)> response => nil [2] pry(#Salesforce::Rest)> response.code => 413 [4] pry(#Salesforce::Rest)> response.class => HTTParty::Response [11] pry(#Salesforce::Rest)> response == nil => false [12] pry(#Salesforce::Rest)> response.nil? => true [13] pry(#Salesforce::Rest)> !!response => true

So in my code I have: if !((200..299).include?(response.code)) if response.nil? raise "SF ERROR #{response.code}" # fake nil, not a bug else raise "SF ERROR '#{response.message}': #{response.first['message']}" end end

As I see it there are two problems:

1. Response#inspect returns 'nil'
2. Response#nil? somehow returns true

I don't know why (2) happens, something to do with Ruby's internal implementation of .nil?, which is defined in Object.

Perhaps you can suggest an improvement for my error detecting code.

[messick]

Both of these happen because of the following protected method in /lib/httparty/response.rb:

def method_missing(name, *args, &block)
  if parsed_response.respond_to?(name)
    parsed_response.send(name, *args, &block)
  elsif response.respond_to?(name)
    response.send(name, *args, &block)
  else
    super
  end
end

This means that practically every method gets tried against parsed_response. This is why response == nil and response.nil? can give opposite results. The former is checking the response object for nil, the latter is actually checking the response.parsed_response for nil.

This overriding of method_missing is causing all kinds of problems. #290 shows that besides the response.nil? returning a "wrong" result sometimes, having malformed JSON inside they response can causel methods like .is_a?, .nil?, to blow up since they are being run against parsed_response and not the actual response object. Is there a reason for the overriding of method_missing @jnunemaker?

[jnunemaker]

I barely remember. This was from a long time ago. I believe the goal was to add a proxy to the parsed response in order to expose the status code and headers, without breaking backwards compatibility.

[messick]

Seems like a better solution might be to explicitly implement the methods that are needed, rather than doing the shotgun method_missing approach. I'll try to take a look at it later, because I'm sure there are lots more people out here having problems because of this.

[simonhildebrandt]

I just got bitten by this bug, but in a different way - trying to use Object#tap on a response object yields the parsed_response instead, which was a spectacularly confusing problem to debug. :)

My first instinct was to reverse the order (check the response object for the method before the parsed_response) but I also agree with @messick - a whitelist or actual proxy methods might behave more predictably.

[steveklebanoff]

@cjlarose and I just got bitten by an issue caused by method_missing that took a long time to debug as well. +1 for whitelisting or actual proxy methods to avoid this confusion for other developers.

[JonMidhir]

If we could build a list of the methods that have to be delegated elsewhere for the class to be backwards compatible I'd be happy to do the work to whitelist those in the method_missing method.

The class would need to inherit from Object rather than BasicObject though to respond to implement things like #tap, #nil? and #is_a?.