Throw exceptions for disallowed operations during flush

[araines]

I've been playing around with the events system a bit more recently and found that the combination of the documentation being a little light and the UnitOfWork silently failing makes for some fairly challenging debug sessions.

I'd like to propose that the UnitOfWork throws exceptions when disallowed operations take place. For example, you cannot persist a new document during a postUpdate as it will be simply lost - so rather than just losing it, it would be nice to get an exception telling the developer what they are doing wrong.

[alcaeus]

@araines thanks for bringing this up here after our conversion in IRC earlier today. I agree with you, the events are a bit complicated in this regard, which is why I'd go as far as saying that you shouldn't use them unless you know what you are doing. That being said, you can't know what you're doing unless we provide some good documentation.

I've tagged this issue as a documentation issue and will try to come up with some documentation about the events, especially:

• What does the flush process look like: changeset computation, order of operation
• At what stage are events triggered and what actions are permissible
• Ideally, add some common use cases of what events are used for and indicate what should and should not be done.

Now, these are just some ideas, obviously we can use all the input we can get here. All of those caveats of the event system make a lot of sense when you've dealt with them for years, but for the average developer it may be quite confusing and overwhelming.

I'd like to propose that the UnitOfWork throws exceptions when disallowed operations take place.

Having previously experimented with functionality like that to prevent nested flushes (which caused issues due to excessive event handling involved) I'd caution against that, at least for now. There are just too many variables involved, and generally speaking, a single exception within the flush process or event handling process will cause all kinds of issues.

[araines]

No problem - more than happy with documentation as the solution to this problem. Whilst I don't mind diving through code of libraries to learn how they work, it's much nicer to be able to get going quickly based on the docs!

Also happy to contribute if I can be of service.

[alcaeus]

#1647 starts this process by throwing deprecation notices when calling flush from an event subscriber/event listener/lifecycle callback. These calls will definitely be removed in 2.0. I'll have to think whether it's feasible to apply the very same logic to persist and other calls 🤔

[malarzm]

I'll have to think whether it's feasible to apply the very same logic to persist and other calls 🤔

I'm not sure whether it's needed, other events are quite safe to be called from one another and removing other documents while removing one is quite handy (or am I misunderstanding the direction?)

[malarzm]

Nvm, just saw #1647 (comment)