[PATCH] Filters and callbacks need a more obvious way to stop processing

Test, documentation and implementation to throw :abort in before filters

I don't think its going to be easier to remember to introduce another way as well. Returning false breaks the chain, so does raising an exception.

I left the other methods in for backwards compatibility. I was thinking that post 1.0, we would remove all methods, save for the throw(:abort) one. Isn't it much more explicit ?

We're going to be in Rails 1.x mode for quite a while (where backwards compatibility is to be ensured). This may indeed a be interesting alternative for Rails 2.0. But write it down as an idea for then.

Moving to 2.x milestone.

Doing throw(:continue) if you don't want to stop processing is also explicit, but that doesn't make it any good. I think throwing a symbol is cumbersome and error-prone.

I agree that it can be hard to remember whether a true return value means continue or stop, but would prefer to see a method call: e.g. stop! if stop_condition

Then, stop! would simply throw a Symbol, not unlike Find.prune ?

Sure :-) I think throwing a symbol is good implementation but poor API choice. Wrapping it in a nice, documented method is all that's needed.

After spending 3 hours tracing through active record code (callbacks.rb is pretty much unreadable at first glance by the way... reflection overkill ftw!) I realized that I was accidentally returning false in one of my callbacks. I think it's way too easy to slip up on this, and it's way too easy to not realize where you slipped up.

throw :abort, or throw :abort via stop! is an excellent idea, and I would strongly suggest pushing this up to 1.2 and generating a deprecation warning when a callback returns false (at the very least this would save some of us a few hours of banging our heads against hard surfaces -- judging from the many tickets on this, this is not a rare problem).

Gunark, your development.log indicates when a filter chain halts due to returning false.