Observers no longer fire outside of ActiveRecord transactions

rubyonrails-core
1

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

2

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?

3

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

4

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

Dude, you stole my punch line :slight_smile:

5

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

6

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.