Most Rails developers will tell you that ActiveRecord’s callbacks are bad. And it’s true as far as it goes.
The syntactic sugar of callbacks can certainly enable bad design and lead to untestable and highly coupled code. It’s probably a good idea to refactor your models if your callbacks are doing more than modifying the internal state of the object itself, as per Jonathan Wallace.
But there are plenty of situations where callbacks are the right tool for the job. Even so, I’m always very leery of using them. Even with best of intentions and a careful implementation they can lead to unexpected behavior and an impromptu debugging session.
I recently enjoyed just such a session while working on Doable’s new (not yet beta) version 2 Rails API. I had a good reason to use an
after_commit callback in the project and the behavior caught me offguard.
The code goes something like this:
after_commit :api_call_and_update, on: :create
... def api_call_and_update
# API call to move resource from temp to proper S3 bucket...
self.update(bucket: 'New bucket')
And it raises a
Stack level too deep error. This didn’t make sense to me right away, it seemed like the call to
update shouldn’t have triggered my callback with its
on: :create condition. But it was and it was creating an infinite loop.
So what gives?
The answer is that, to ActiveRecord, the
create lifecycle doesn’t end until the last callback has finished execution. The
self.update() is therefore not interpreted as a
update at all, but still part of the original
create lifecycle, and so the callbacks for
on: :create will be executed again. And again. And again.
The solution in this case was to use
self.update_columns(...) which bypasses validations and callbacks and sidesteps the infinite loop. In another situation, it may have been better to refactor the logic into a service object that handles the API call and update operation outside of the
MyModel create lifecycle all together.