Halting filters with render/redirect instead of returning false

We've been wanting to deal with the fact that filters are too easy to inadvertently halt when the last statement returns false for some reason. A few suggestions were discussed, like throwing symbols, but after a thorough inspection of a ton of real filters in real applications, we realized that we already have a pattern: When a filter renders or redirects, it's always followed by returning false.

After spotting this, it seems like the return false part is redundant and that we instead could just assume that a filter wants to halt if performed? is true after it has run.

But before we make this happen for Rails 2.0, I thought it would be a good idea to hear if there's any reasonable claims as to when you'd want to either render/redirect, but still allow the chain to continue, or do neither and still want halting. Emphasis is on reasonable, I don't mind obscure edge cases breaking. This is 2.0, after all.

This makes sense to me also. I can’t imagine a scenario when I render or redirect in a filter where it makes sense to continue with the action.

Does positive feedback for this change mean you’ll consider dropping the old way of halting filters? Personally I’ve never felt that “false” was a bad value and I liked that it was chosen for halting. I don’t think it has ever inadvertently halted the chain for me – really, what are the chances of putting a method that returns boolean as the last statement in a filter?

This is a good move, and I feel ActiveRecord before_* filters' support for "false" has bit people just as often.

Does positive feedback for this change mean you'll consider dropping the old way of halting filters? Personally I've never felt that "false" was a bad value and I liked that it was chosen for halting. I don't think it has ever inadvertently halted the chain for me -- really, what are the chances of putting a method that returns boolean as the last statement in a filter?

Returning false will no longer halt the chain. It never really made any sense to halt the chain without rendering or redirecting, it only lead to White Screens of Death. Plenty of people were bitten by it.

This is now fact from http://dev.rubyonrails.org/changeset/7984 and forth.

What happens to old code that goes:

    redirect_to foo_url and return if foo.nil?

Does it break? Or does it now work just like:

    redirect_to foo_url if foo.nil?

?

What happens to old code that goes:

    redirect_to foo_url and return if foo.nil?

Does it break? Or does it now work just like:

    redirect_to foo_url if foo.nil?

Because the original code redirects, the filter chain is halted. The return won't do any harm, it will just have no effect.

Will this change prevent this error from occurring anymore?

ActionController::DoubleRenderError

"Render and/or redirect were called multiple times in this action. Please note that you may only call render OR redirect, and only once per action. Also note that neither redirect nor render terminate execution of the action, so if you want to exit an action after redirecting, you need to do something like \"redirect_to(...) and return\". Finally, note that to cause a before filter to halt execution of the rest of the filter chain, the filter must return false, explicitly, so \"render(...) and return false\"."

I always forget to add "and return false" because having a render or redirect seems like it should halt the chain to me.

- Trevor

Trevor, that’s exactly what this change did. You won’t be getting that error anymore and you can safely remove “and return false” from your filters.

Terrific. This seems like a smart change to me. I dig all of the simplification and streamlining going on in this release.

If that error can't occur anymore, shouldn't it be removed from Rails? I didn't see that it was taken out in the changeset linked above, which is why I'm wondering. I see a few mentions of "DoubleRenderError" in this file:

http://dev.rubyonrails.org/browser/trunk/actionpack/lib/action_controller/base.rb

I can open a ticket (or try to create a patch) if that's more appropriate.

- Trevor

Oh, it can occur all right, but not in the context that we were discussing in this thread.

It still needs to catch errors caused by clumsy conditionals in a single filter or action.