While documenting stuff related to transactions and callbacks I've
realized that if a hook raises precisely AR::Rollback save! silently
returns nil. I've documented that behaviour, but it surprised to me
that save! may not raise an exception on failure. Looks like a side
effect of the implementation in fact more than something intended.
Yes imho it is. Since AR::Rollback represents in essence a reason
because record is not saved I can't see what would be the reason to be
treated a different way.
I am not enterily sold on this option but we could manually raise
AR::RecordNotFound on AR::Rollback inside save! and of course document
it. Not sure about the implementation because silencing AR::Rollback
is done deep in database_statements.rb but I am just thinking about
the usage by now. That would give an idiomatic way to cancel +
rollback that in turn results in expected client code.
Another option would be to document the current behaviour (this is
already done) and warn people about the resulting if/else. You can
still deal with the rollback in a rescue clause raising something
different from AR::Rollback
begin
model.save!
# success
rescue AR::RecordNotSaved, MyApp::SomeError => e
# failure
end
Yeah, it is, but I'm not sure that people should be raising
AR::Rollback themselves anyway, it's intended as a way to abort the
transaction and continue processing, and if you're not using it for
that, then you should be raising some other exception
In fact, I wonder why halting the chain in before_* callbacks commits
the transaction. Wouldn't you expect a rollback? If AR executed a
rollback automatically, in addition, save and save! would preserve
their usage idioms.
That would still leave room for best practices for rollbacks in
after_* hooks, but I think that would be fine.
In fact, I wonder why halting the chain in before_* callbacks commits
the transaction. Wouldn't you expect a rollback? If AR executed a
rollback automatically, in addition, save and save! would preserve
their usage idioms.
That does sound more ... sensible. Why don't you give it a try in a
branch and see what breaks?