New filter implementation causing problems in production ?

Hi all,

I'm using a Pistonized Rails export from the 1.2 branch. I just
updated to r6651 and got the new implementation. Running functional
and integration tests reveals nothing, as well as running under
Mongrel in development mode.

After deploying to a staging server (in production mode), I get the
following failure:

ActionController::ActionControllerError in PartiesController#index

filter #<ActionController::Filters::ClassMethods::BeforeFilterProxy:0xb60ee668
@filter=#<ActionController::Filters::ClassMethods::SymbolFilter:0xb60ee6cc
@filter=:login_from_cookie>> was in the wrong place!

RAILS_ROOT: /usr/local/www/xlsuite.com/config/..
Application Trace | Framework Trace | Full Trace

/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/filters.rb:729:in
`call_filters'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/filters.rb:675:in
`perform_action_without_benchmark'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/benchmarking.rb:66:in
`perform_action_without_rescue'
/usr/lib/ruby/1.8/benchmark.rb:293:in `measure'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/benchmarking.rb:66:in
`perform_action_without_rescue'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/rescue.rb:83:in
`perform_action'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/base.rb:430:in
`send'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/base.rb:430:in
`process_without_filters'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/filters.rb:680:in
`process_without_session_management_support'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/session_management.rb:114:in
`process'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/base.rb:330:in
`process'
/usr/local/www/xlsuite.com/vendor/rails/railties/lib/dispatcher.rb:41:in
`dispatch'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/rails.rb:78:in `process'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/rails.rb:76:in
`synchronize'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/rails.rb:76:in `process'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:618:in `process_client'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:617:in `each'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:617:in `process_client'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `run'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `initialize'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `new'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:736:in `run'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:720:in `initialize'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:720:in `new'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel.rb:720:in `run'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/configurator.rb:271:in
`run'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/configurator.rb:270:in
`each'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/configurator.rb:270:in
`run'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/bin/mongrel_rails:127:in `run'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/lib/mongrel/command.rb:211:in `run'
/usr/lib/ruby/gems/1.8/gems/mongrel-1.0.1/bin/mongrel_rails:243

/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/filters.rb:729:in
`call_filters'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/filters.rb:675:in
`perform_action_without_benchmark'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/benchmarking.rb:66:in
`perform_action_without_rescue'
/usr/lib/ruby/1.8/benchmark.rb:293:in `measure'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/benchmarking.rb:66:in
`perform_action_without_rescue'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/rescue.rb:83:in
`perform_action'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/base.rb:430:in
`send'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/base.rb:430:in
`process_without_filters'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/filters.rb:680:in
`process_without_session_management_support'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/session_management.rb:114:in
`process'
/usr/local/www/xlsuite.com/vendor/rails/actionpack/lib/action_controller/base.rb:330:in
`process'
/usr/local/www/xlsuite.com/vendor/rails/railties/lib/dispatcher.rb:41:in
`dispatch'

I looked at the code, and it seems the filter isn't put in the right
spot ? The chain of events is:

class ApplicationController
  include XlSuite::AuthenticatedSystem
end

class XlSuite::AuthenticatedSystem
  def self.included(base)
    base.before_filter :login_from_cookie
  end
end

I received a similar failure for another filter that is added using
prepend_before_filter.

Any change I did something wrong ?

Thanks !

I'm using a Pistonized Rails export from the 1.2 branch. I just
updated to r6651 and got the new implementation. Running functional
and integration tests reveals nothing, as well as running under
Mongrel in development mode.

After deploying to a staging server (in production mode), I get the
following failure:

Hmm, this could be a pain to track down. Can you reproduce it with a
simple sample application?

I received a similar failure for another filter that is added using
prepend_before_filter.

Any change I did something wrong ?

Seems ok to me from what you've described....

Hi,

There is a ticket with a failing test case attached at:
http://dev.rubyonrails.org/ticket/8803

Cheers,

Nik

(repost from the ticket comment for the benefit of the mailing list)

I hit Filter in the wrong place error for a different scenario.

My scenario is that I have a before_filter that was raising an
exception, and the around_filter rescuing it with a render / redirect.
I have included test cases for this scenario as well as the scenario
when the action itself raising an exception.

I have included a one-liner patch that basically just returns before
it gets the chance to raise the Filter in wrong place exception if
performed? is true. Seems to work, and all tests pass but I don't know
if this is right.

I have also included the original test case above by nik.wakelin to
test that this patch works.

http://dev.rubyonrails.org/ticket/8803

Regards,
kamal

Stefan has a patch for these problems and more. Hopefully it'll show
up on trac in the next few days.

The patch is here: http://dev.rubyonrails.org/ticket/8891

-- stefan

The patch is here: http://dev.rubyonrails.org/ticket/8891

I've forgotten why the return false was required for around filters.
Couldn't we do:

yielded = false
filter.call(self) do
  yielded = true
  # all remaining before and around filters will be run in this call
  index = call_filters(chain, index.next, nesting.next)
end
halt_filter_chain(filter) unless yielded

Yes we could do that. The question in this case is whether we want to
distinguish between around filters returning false and filters that
simply don’t yield w.r.t. after filter processing.

we could say
that an around filter returning false will stop after filter processing
whereas filters that simply don’t yield will not stop the after filter
chain.

and while we’re at it: I suggest that it should be possible
for after filters to stop after filter chain processing by returning
false.

Yes we could do that. The question in this case is whether we want to
distinguish between around filters returning false and filters that simply
don't yield w.r.t. after filter processing.

we could say that an around filter returning false will stop after filter
processing whereas filters that simply don't yield will not stop the after
filter chain.

and while we're at it: I suggest that it should be possible for after
filters to stop after filter chain processing by returning false.

So assuming I've got this right, failing to yield halts the
before/around filter chain, prevents the action from rendering but
still runs the after filters. Failing to yield and returning false
will behave as above, but *not* run the after filters.

Does anyone have any objections to this?