before_create return value breaking object.save: rails bug?

I know that usually when people say 'i think i found a bug in rails/ruby' they've done something dumb. I'm wondering if this is the case here, but this does seem like a genuine rails bug to me.

In my user model i have a few different before_create callbacks. In one of them, if it happens to return false (just because the last statement in it evaluated to false), then the saving gets blocked - i can see the db transaction rolling back. However, this doesn't affect the results of calling .valid? on the object, so i get this situation:

new_user.valid? => true new_user.save => false new_user.errors.full_messages =>

This has caused me a considerable amount of beard-tugging to track this down.

Now, i was under the impression that before_create doesn't care what's returned by the method it calls, and for my other before_save callbacks it doesn't seem to make any difference - just this one. It's definitely the return value because if i put 'true' as the last line of the callback method then everything is fine. I change it to 'false' and everything's not-fine again.

I'm in rails 2.2.2.

Can anyone shed any light on this?

thanks max

I know that usually when people say 'i think i found a bug in rails/ruby' they've done something dumb. I'm wondering if this is the case here, but this does seem like a genuine rails bug to me.

In my user model i have a few different before_create callbacks. In one of them, if it happens to return false (just because the last statement in it evaluated to false), then the saving gets blocked - i can see the db transaction rolling back. However, this doesn't affect the results of calling .valid? on the object, so i get this situation:

It's a feature. See ActiveRecord::Callbacks (under Canceling callbacks)

Fred

If you're returning false on purpose, you'll typically add an error to the current object (errors.add or add_to_base) to explain what happened. Otherwise, it can be a little mysterious...

--Matt Jones

IMHO, what you are saying does not make much sense looking at it from the perspective of the intent of the feature. The documentation says that if the callback returns false the chain is halted and it is my understanding that it is provided for you/us as a feature in case it is needed. The fact that you wrote your callbacks not knowing about the feature and unfortunately (or fortunately, so you now know about it) some of them end in a statement that can return false does not make the way it works horrible, just not convenient because you already wrote your code not taking that into cosideration. I am sure that many RoR programmers enjoy what the feature does for them and use it on purpose.

What you are saying could be compared to complaining when taking the key out of a car stops the engine if you didn't know it would occur. Well, that's just the way it works and many of us are thankful for that 'feature'. It saves us a lot of $$ in gas and possibly stops countless accidents.

My 2 cents.

Pepe

I actually ran into this same issue sometime back. n the worst part was that it took me even longer to figure out this was happening cos i was saving multiple records and some just wouldnt go into the db and i would have no idea. but i really think its a useful, and more importantly, correct feature. before_create and before_save are meant for things u wanna do to/with the record before db commit. If one of those actions fail, u wanna know about it. though wat u say about @user.valid?, @user.save, @user.errors.full_messages does make sense. a built-in rails solution to this?.. hmmm.. mayb raise a standard error for all callbacks when false is returned?

Pepe, i know what you're saying - ultimately this is my fault for not reading the documentation. But, it seems like a lot of people have missed this too. It's a very important aspect of callbacks, and i think it should be made more obvious in the documentation, especially since one is given no feedback whatsoever as to why the process failed.

In this case i noticed it because a record wasn't saved, but if it wasn't a before_create callback that went wrong, and was instead an after_destroy for example, i wouldn't have noticed. Maybe the documentation could be changed so that this is immediately drawn to the attention of someone reading the section on callbacks, rather than being two pages down.

Anyway, i'd be interested to know how many other people didn't know about this. I think i'll do a couple of polls :slight_smile:

thanks! max