Observers no longer fire outside of ActiveRecord transactions

I’ve just encountered a strange bug in an app where a newly created model was not visible in the database to a process was kicked off by an observer.

Looking through the logs and then the rails source, it looks like Observer methods are now getting fired as standard AR callbacks, meaning they fire in the AR transaction.

This wasn’t the case in 2.3 as far as I can tell.

In my mind, observer callbacks should not be fired inside the AR transaction to avoid race conditions when observers kick of processes that try and access the new model before all the callbacks have completed and the transaction is committed.

Any thoughts?

Tekin Suleyman

http://tekin.co.uk

In my mind, observer callbacks should not be fired inside the AR transaction
to avoid race conditions when observers kick of processes that try and
access the new model before all the callbacks have completed and the
transaction is committed.

You could make the same argument about regular callbacks though,
after_create could be used to kick off jobs etc. I can see arguments
on both sides, observers are a different object so having a slightly
detached lifecycle makes sense. But at the same time it's really just
an organisation method, so why is there a difference.

Any thoughts?

Is it fair to say that the best fix would be another specific hook
which fires after the transaction is successfully committed?

That's why we have after_commit in Rails 3. :wink:

That's why we have after_commit in Rails 3. :wink:

Dude, you stole my punch line :slight_smile:

That’s why we have after_commit in Rails 3. :wink:

OK, I’ll use after_commit to get around this particular issue.

This does raise a few questions though.

For example, now that Observer methods are effectively being rolled into the normal callback chain of the models they observe, what exactly is their point? Wouldn’t it now be just as effective, and arguably easier to grok, to simply put peripheral callbacks in a module and include them directly in the model?

Also, doesn’t this new refactoring mean that observer callbacks can now halt the execution chain where they couldn’t before?

Finally, using after_commit in the observer is not quite the same thing as the previous behaviour as you can’t distinguish between after_commit on create and update without doing it in the model directly, right?

I can’t say I feel particularly strongly either way, just interested in stimulating some debate on what are some subtle but still potentially significant behavioural changes. At the very least, the docs/guides should probably be updated to document the new behaviour.

Tekin

For example, now that Observer methods are effectively being rolled into the
normal callback chain of the models they observe, what exactly is their
point? Wouldn't it now be just as effective, and arguably easier to grok, to
simply put peripheral callbacks in a module and include them directly in the
model?

I'm not so sure that it's actually easier to understand. There's
definitely something nice about having a single class which has a
particular aspect of object lifecycle. That class can be tested and
refined in isolation and can be turned on and off independently of the
model in question. In my own code I typically use model callbacks for
the bulk of the callback behaviour, observers are typically only for
truly tangential things like cache expiry.

Also, doesn't this new refactoring mean that observer callbacks can now halt
the execution chain where they couldn't before?

Good question, if they can I'm not sure that it would be the end of the world.

Finally, using after_commit in the observer is not quite the same thing as
the previous behaviour as you can't distinguish between after_commit on
create and update without doing it in the model directly, right?

after_commit fires on destroy too, so it's even slightly more
confusing. There's after_commit :your_thing, :on=>:create for model
level callbacks, but I don't believe that's available if you're using
observers. There probably should be.

I can't say I feel particularly strongly either way, just interested in
stimulating some debate on what are some subtle but still potentially
significant behavioural changes. At the very least, the docs/guides should
probably be updated to document the new behaviour.

I definitely agree here, the change isn't particularly earth
shattering or confusing, so long as it's well documented with a few
examples. The current callbacks documentation is actually quite good,
but perhaps could do with some refinement.