#4367 [PATCH] Using Sinatra as middleware fails - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4367 ✓stale

[PATCH] Using Sinatra as middleware fails

Reported by jaswope | April 12th, 2010 @ 02:58 AM | in 2.3.10

When trying to use Sinatra (or any rack middleware that is capable of running as a singleton) the app stack fails to initialize. This is due to the middleware stack using #respond_to?(:call) to test if a piece of middleware is a Proc. Since Sinatra can respond to call without being initialized, and because it requires the Rack environment to be passed to it, the initialization.

Attached is a proposed patch to replace #respond_to?(:call) with #is_a?(Proc). Tests still pass after this change, and I have made the change using 2-3-stable as my base.

middleware_fix.patch 858 Bytes

Comments and changes to this ticket

You flagged this item as spam.
jaswope April 12th, 2010 @ 02:59 AM
- Tag changed from “patch” to “2-3-stable, patch”
You flagged this item as spam.
jaswope April 13th, 2010 @ 02:46 AM
- Title changed from “Using Sinatra as middleware fails” to “[PATCH] Using Sinatra as middleware fails”
Yehuda Katz (wycats) April 13th, 2010 @ 04:06 AM
- Milestone set to “2.3.6”
- Assigned user set to “Yehuda Katz (wycats)”
This is handled in master by first checking for respond_to?(:new), the middleware convention:
```
def klass
  if @klass.respond_to?(:new)
    @klass
  elsif @klass.respond_to?(:call)
    @klass.call
  else
    ActiveSupport::Inflector.constantize(@klass.to_s)
  end
end
```
Perhaps do the same for 2.3? Maybe just backport the entire middleware stack?
Rizwan Reza May 16th, 2010 @ 02:41 AM
- Tag changed from “2-3-stable, patch” to “2-3-stable, bugmash, patch”
[bulk edit]
You flagged this item as spam.
Anil Wadghule May 16th, 2010 @ 02:55 PM
I am able to do this

λ cat config.ru
```
require "config/environment"
require 'sinatra'

use Rack::Reloader
use Sinatra::Application
use Rails::Rack::LogTailer
use Rails::Rack::Static
use Rack::CommonLogger
run ActionController::Dispatcher.new
```
Attached is a app showing it's running properly.

Am I missing something here?
- sinatra_middleware_4367.zip 95.1 KB
José Valim May 16th, 2010 @ 03:55 PM
- Assigned user changed from “Yehuda Katz (wycats)” to “José Valim”
Can someone provide a patch with the fix mentioned by Yehuda on 2-3-stable?
Rizwan Reza May 16th, 2010 @ 04:18 PM
- State changed from “new” to “open”
You flagged this item as spam.
Anil Wadghule May 16th, 2010 @ 04:54 PM
Attached is a patch to backport middleware stack from Rails 3 to Rails 2-3. All the tests pass fine.
- backport_middleware_stack.diff 2.9 KB
Rizwan Reza May 16th, 2010 @ 04:57 PM
Hey Anil,

Nice work. There are some tests in middleware_stack_test you might want backport as well. Just the ones we need.
José Valim May 16th, 2010 @ 05:01 PM
There is no need to backport the whole middleware stack. And as Rizwan said, it would be good to backport the relative tests as well.
You flagged this item as spam.
Anil Wadghule May 16th, 2010 @ 05:33 PM
Attached is a patch which backports a test from Rails 3 middleware stack to Rails 2-3

Also I don't see any code which I should remove from the 'backport_middleware_stack' patch.
- backport_middleware_stack_test.diff 1008 Bytes
José Valim May 16th, 2010 @ 05:41 PM
Anil, your patch, for instance, change the build method to allow a block to be given. But there are no tests for it in your patch. Also there are not tests for the case described in this ticket. So if we backport all these, we need to add the respective tests. Thanks!
You flagged this item as spam.
Anil Wadghule May 16th, 2010 @ 05:50 PM
Okay I will add patch with necessary tests.
Jeremy Kemper May 23rd, 2010 @ 05:54 PM
- Milestone changed from “2.3.6” to “2.3.7”
[bulk edit]
Jeremy Kemper May 24th, 2010 @ 09:40 AM
- Milestone changed from “2.3.7” to “2.3.8”
[bulk edit]
Jeremy Kemper May 25th, 2010 @ 11:45 PM
- Milestone changed from “2.3.8” to “2.3.9”
[bulk edit]
José Valim June 7th, 2010 @ 10:03 AM
- State changed from “open” to “incomplete”
Anil, you just need to backport the part relative to the naming (that Yehuda posted) and not the whole middleware stack. Still willing to try out a patch?
You flagged this item as spam.
Anil Wadghule June 7th, 2010 @ 10:52 AM
okay Jose, I will try to provide the patch soon.
You flagged this item as spam.
Anil Wadghule June 8th, 2010 @ 05:44 PM
José, I have attached a patch which fixes this issue.

I could not find way to write tests for it. Even in Rails 3 master seems that there are no tests which tests @klass.respond_to?(:call) which I could just refer to. If there are any, can you tell me where are those.
- 0001-fix-failing-of-sinatra-as-middleware-4367.patch 1.1 KB
Neeraj Singh June 11th, 2010 @ 08:31 PM
@Anil Can you post an app which uses Sinatra as a middleware and fails without the patch. I don't fully understand rack stuff and an app demonstrating the problem would be helpful.

I am close to finishing writing test case for the patch you added. However a deeper understanding of the problem would help me write better test. Thanks.
You flagged this item as spam.
Anil Wadghule June 11th, 2010 @ 09:31 PM
Neeraj, Attached is a rails app which fails on Rails 2.3.8 without my fix (try to access http://localhost:3000/).
I am eager to see the test patch you have writteen for the fix :)
- sinatra_middleware.zip 93.1 KB
You flagged this item as spam.
Anil Wadghule June 23rd, 2010 @ 04:53 PM
Neeraj, any update on the test patch?
Neeraj Singh June 23rd, 2010 @ 05:22 PM
Hey Anil,

Thanks for providing the app but I couldn't create the tests.
Jeremy Kemper August 30th, 2010 @ 02:28 AM
- Milestone changed from “2.3.9” to “2.3.10”
- Importance changed from “” to “Low”
[bulk edit]
José Valim September 2nd, 2010 @ 10:56 AM
- State changed from “incomplete” to “stale”