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:
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 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:
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:
For such an important bit of knowledge, it seems buried halfway down the
page.
So, i guess i was being a bit dumb, but maybe a little forgiveably so?
There must be a lot of people making this mistake. There’s probably a
lot of people with callbacks in their application already who don’t even
realise that they’re returning false just because of the last statement
in there and breaking.
I still think it’s wrong that this situation can occur:
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…
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 J.
It’s the returning false by accident that was the problem, though. I’ve
now gone through my callbacks and added ‘true’ before the end, when i
don’t want them to fail. This seems kind of horrible.
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.
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
thanks!
max
This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.
