Avoid returning `[nil]` to Rack

[stanhu]

Prior to Rack v2.1.0, Rack::Response would call to_s on each of the body parts: https://github.com/rack/rack/blob/85684323f8f58409e717af91e446d257d496f8b8/lib/rack/response.rb#L40-L43. That means if Grape returned [nil], Rack would handle this gracefully.

This is no longer the case now with https://github.com/rack/rack/pull/1434/files. It's easy to have an API call that returns nil to Grape, and the body will be set to [nil]: https://github.com/ruby-grape/grape/blob/0d5cf4035c04c1c44e7868a8b77a969b28766245/lib/grape/endpoint.rb#L267-L277

This causes fits with Rack:

NoMethodError (undefined method `empty?' for nil:NilClass):
rack (2.1.4) lib/rack/etag.rb:70:in `block in digest_body'
rack (2.1.4) lib/rack/body_proxy.rb:34:in `block in each'
rack (2.1.4) lib/rack/body_proxy.rb:34:in `each'
rack (2.1.4) lib/rack/body_proxy.rb:34:in `each'
rack (2.1.4) lib/rack/etag.rb:68:in `digest_body'
rack (2.1.4) lib/rack/etag.rb:31:in `call'
rack (2.1.4) lib/rack/conditional_get.rb:40:in `call'
rack (2.1.4) lib/rack/head.rb:14:in `call'

Maybe Grape should return nil instead of [nil] if there is no body? This might have to be done in the formatters since nil may mean null in JSON.

Rack issue: https://github.com/rack/rack/issues/1712

[stanhu]

I'm going to close this issue because this problem seems to have gone away in Rack v2.2.3, but v2.1.x has this issue. https://github.com/rack/rack/pull/1535 might have helped.

[stanhu]

Closed too soon. I got this with v2.2.3 as well.

[stanhu]

Based on https://github.com/rack/rack/pull/1713#issuecomment-709379195, it sounds like Grape should flag responses that don't contain String values?

[dblock]

Do we want to raise errors in Grape for this? Feels like that would be a no, because a contract is a contract, but I'm open to it.

[stanhu]

I created this middleware:

class ResponseCoercerMiddleware < ::Grape::Middleware::Base
  def call(env)
    response = super(env)

    status = response[0]
    body = response[2]

    return response if Rack::Utils::STATUS_WITH_NO_ENTITY_BODY[status]
    return response unless body.is_a?(Array)

    body.map! do |part|
      if part.is_a?(String)
        part
      else
        part.to_s
      end
    end

     response
  end
end

In test and development, I modified this to raise an exception when part is not a String.

[stanhu]

I wonder if we want to add some variant of this to the base middleware.

[ebenoist]

Just got hit by this as well. Is there a blessed way to fix this issue?

[dblock]

@ebenoist Care to dig up whether this is the case un latest Rack and what the Rack API is supposed to be? Seems like there's no conclusion above.