By
on Rails, Bug

Issues of Control Flow logic in Respond To Blocks

The following code was written by my boss to simulate an issue arising with a production app that I've been working on at work.

class RacersController < ApplicationController  
  def race
    @racer = Racer.first
    @was_in_if_block, @was_outside_if_block = false, false

    respond_to do |format|
      if @racer.name == 'john'
        format.json do
          render :json => @racer
          @was_in_if_block = true
          return true
        end
      end

      @was_outside_if_block = true
      format.json { render :json => @racer.name = "foobar" }
      format.html { @racer }
    end
  end
end  

The Problem

Can you identify any issues lurking within this code?

Assuming that the responses is a json response and that @racer.name is indeed 'john',
the if condition should execute which would lead one to believe that @was_in_if_block would evaluate to true followed by the the rest of the code not to execute.

However, if this code was spec'ed out(which indeed it is, to simulate the issue, https://github.com/alisyed/RailsAnomalies) we would have observed that @was_outside_if_block was being assigned true as well. In other words the return isn't working as expected.

The underlying reason

Lets delve into Rails code!

The respond_to method invokes the important retrieve_collector_from_mimes(mimes, &block) which first evaluates the particular type of mime_types the controller action can respond to at a controller level configuration for example:

respond_to :json, :except => [ :edit ]

as well as inspecting the block passed to the respond_to method. The collector for the particular request is attempted to be found.

Assuming that the the request is JSON for our case, the collector for our particular request will initially be assigned the format.json block in the if condition(It is important to note that the code inside the actual format blocks will not be executed yet) and since another format.json block exists, our collector will later on be assigned that block. This is carried out by the block.call(collector) statement.

def respond_to(*mimes, &block)  
  raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given?

  if collector = retrieve_collector_from_mimes(mimes, &block)
    response = collector.response
    response ? response.call : default_render({})
  end
end

# File actionpack/lib/action_controller/metal/mime_responds.rb, line 267
def retrieve_collector_from_mimes(mimes=nil, &block) #:nodoc:  
  mimes ||= collect_mimes_from_class_level
  collector = Collector.new(mimes)
  block.call(collector) if block_given?
  format = collector.negotiate_format(request)

  if format
    self.content_type ||= format.to_s
    lookup_context.formats = [format.to_sym]
    lookup_context.rendered_format = lookup_context.formats.first
    collector
  else
    head :not_acceptable
    nil
  end
end  

As the code in the format.json block isn't executed yet, @was_in_if_block will still have a value false, and the @was_outside_if_block will be assigned a value of true.

The format.json block will then execute, which is the last line of the respond_to method.

comments powered by Disqus