Rspec-rails: calling .to_json on double.as_null_object causes stack level too deep in rails 4.1+

[Avemark]

Having this issue in rails 4.2 on my own application. promised @samphippen I'd write an issue.

at https://github.com/chav02/rspec_to_json I've prepared a minimal rails application that showcases this for rails 4.0.x (working) and 4.1.x (splosions!)

[JonRowe]

I actually wonder if this is a Rails bug, I can see how this would happen, for example if Rails when trying to convert our double to_json and kept trying until it gets something enumerable this would be the result...

[cupakromer]

The root cause is from this monkey patch from rails in lib/active_support/core_ext/object/json.rb:

class Object
  def as_json(options = nil) #:nodoc:
    if respond_to?(:to_hash)
      to_hash.as_json(options)
    else
      instance_values.as_json(options)
    end
  end
end

Since this is a spy it responds to all messages. However, since there are no responses set it simply returns itself. When as_json is called the first time, this code checks if the spy responds to a hash. It does. It asks for the hash, but doesn't get a hash, it gets the spy back. Which it then attempts to convert to json again. But since it's the same object, we simply recurse in the same method.Thus, we get an infinite loop.

However in Rails 4.0 the implementation was very different and is defined in lib/active_support/core_ext/object/to_json.rb:

def to_json(options = nil)
  ActiveSupport::JSON.encode(self, options)
end

It looks like we will need to add our own monkey patch to a spy to handle this change.

[cupakromer]

Locally this fixed it for me but I'm not sure that's really the correct solution:

RSpec::Mocks::Double.class_exec do
  undef_method(:to_json)
end

[myronmarston]

Locally this fixed it for me but I'm not sure that's really the correct solution:

Given that we document null object doubles (and spies) as responding to all methods, that doesn't seem like a good solution.

Instead, maybe rspec-rails could define to_json to return something reasonable? For example, what did Rails 4.0 return for double.as_null_object.to_json? You may even just want to use the Rails 4.0 implementation (ActiveSupport::JSON.encode(self, options)).

[JonRowe]

Would simply implementing to_json to return self work? That would match our old behaviour but short-circuit the Rails monkey patch?

[cupakromer]

Given that we document null object doubles (and spies) as responding to all methods, that doesn't seem like a good solution.

Good point since spies use the same class as doubles this would break doubles.

Instead, maybe rspec-rails could define to_json to return something reasonable?

Would simply implementing to_json to return self work?

Defining our own method does prevents the exception, but doesn't allow the spy to check the method:

require 'rails_helper'

RSpec::Mocks::Double.class_exec do
  def to_json
    self
  end
end

RSpec.describe 'Attempting to call to_json on a double' do

  it 'does not cause a stack trace' do
    a = spy('A')
    a.to_json
    expect(a).to have_received(:to_json)
  end

end

It looks like this is because we do not record the message except in method_missing. Since we've defined the method, or it's defined by ActiveSupport on Object, it never gets recorded.

Also, only patching RSpec::Mocks::Double won't fix object doubles. Interestingly, instance and class doubles don't cause the exception without a patch.

[JonRowe]

It looks like this is because we do not record the message except in method_missing. Since we've defined the method, or it's defined by ActiveSupport on Object, it never gets recorded.

Yes but that's an easy fix if we're defining our own method...

[cupakromer]

This seems to fix both and allow for capturing messages:

module NullObjectToJson
  def respond_to_missing?(message, include_all)
    message == :to_json || super
  end

  def method_missing(message, *args, &block)
    proxy = __mock_proxy
    if proxy.null_object? && message == :to_json
      proxy.record_message_received(message, *args, &block)
      return "null"
    end
    super
  end
end

RSpec::Mocks::Double.class_exec do
  undef_method(:to_json)
  include NullObjectToJson
end

RSpec::Mocks::ObjectVerifyingDouble.class_exec do
  undef_method(:to_json)
  include NullObjectToJson
end

[JonRowe]

I think we could/should fix this up stream by defining to_json when we make a double a null object, rather than un defining the method here... :/

[myronmarston]

I think we could/should fix this up stream by defining to_json when we make a double a null object, rather than un defining the method here... :/

This issue seems 100% rails specific (as it's really only a problem on some Rails versions) so I'd rather not pollute rspec-mocks with this. IMO, it belongs here in rspec-rails.

[cupakromer]

I agree defining a custom to_json based on how I wrote the method_missing is much simpler. However, doesn't that break spies on BasicObject?

[JonRowe]

Not if it's done only when a null object?

[myronmarston]

However, doesn't that break spies on BasicObject?

Not sure what you mean? None of spy, instance_spy, class_spy or object_spy use BasicObject...

[cupakromer]

This prints out the custom to_json response, but then fails the expectation:

  it "doesn't pollute basic object" do
    d = instance_spy(BasicObject)
    p d.to_json
    expect(d).to have_received(:to_json)
  end

  1) Attempting to call to_json on a double doesn't pollute basic object
     Failure/Error: expect(d).to have_received(:to_json)
       BasicObject does not implement: to_json

I would have expected the call to d.to_json to throw the exception.

I ran that with this patch:

  ::RSpec::Mocks::TestDouble.module_exec do
    def to_json(*args, &block)
      return super unless null_object?
      __mock_proxy.record_message_received(:to_json, *args, &block)
      "null"
    end
  end

[Avemark]

I'd suggest doing this on the as_json level rather than to_json. I don't see the sense in fixing this problem for to_json but leaving the as_json method vulnerable. I know I for one use the as_json method sometimes, even if I can't specifically remember doing it in a spec.

[cupakromer]

@chav02 as the model author you write / implement the as_json method. However, it is rare for one to call as_json directly, instead you call to_json, or let Rails. That handles calling as_json on the object and recursive objects.

Even if we implemented an as_json you still would not be able to set an expectation that to_json was called on a spy due to the Rails monkey patch.

[JonRowe]

Would it though? Defining as_json (or undefining the existing and using method missing) should have the same "fix" effect?

[cupakromer]

@chav02 I think i see what you are referring to. We need to patch both methods. PR inbound.

[JonathanFrias]

I ran into this issue, here is a workaround that worked for me:

    the_mock = double('object').as_null_object
    the_mock.stub(:as_json) { the_mock }