[DoctrineBridge] Throw an exception in case of circular dependencies in event listeners #48955
Conversation
carsonbot
changed the title
GromNaN
force-pushed
the
doctrine-evm-circular
branch
from
76dd0c4 to
5394f83
Compare
| } | ||
|
|
||
| private function initializeSubscribers() | ||
| { | ||
| $this->initializedSubscribers = true; | ||
| if ($this->initializingSubscribers) { | ||
| throw new EventSubscriberCircularException('Circular dependency detected during initialization of event subscribers.'); |
There was a problem hiding this comment.
Could there be a way to know which one triggers the loop?
There was a problem hiding this comment.
I can catch the EventSubscriberCircularException in the loop to rethrow an other exception with more context, but I didn't want to make code more complex for an info already provided by the stack trace.
There was a problem hiding this comment.
@symfony/mergers Do you think that context details should be added to the exception message, when this information can be found in the stacktrace?
That is, add a try-catch to throw a new exception with more information, pushing the original exception as $previousException.
There was a problem hiding this comment.
I added more context to the message.
For a subscriber:
For a listener:
GromNaN
force-pushed
the
doctrine-evm-circular
branch
from
5394f83 to
6421d03
Compare
GromNaN
force-pushed
the
doctrine-evm-circular
branch
from
12e5402 to
963f7bb
Compare
GromNaN
force-pushed
the
doctrine-evm-circular
branch
2 times, most recently
from
b160737 to
335631b
Compare
GromNaN
force-pushed
the
doctrine-evm-circular
branch
from
335631b to
f1277c1
Compare
GromNaN
force-pushed
the
doctrine-evm-circular
branch
from
f1277c1 to
60d8628
Compare
|
I rebased this PR on 6.4. Adding a new exception is a feature. The detection of circular dependency on even subscriber have been removed because this feature is deprecated. |
…rine event listeners
GromNaN
force-pushed
the
doctrine-evm-circular
branch
from
60d8628 to
082d997
Compare
| */ | ||
| class EventListenerCircularException extends \RuntimeException | ||
| { | ||
| public function setMessage(string $message): self |
There was a problem hiding this comment.
I suggest marking that method as @internal
|
As we are in feature freeze this must target 7.1 |
|
@GromNaN do you still have interest to work on this PR? |
|
I missed last comment from @stof. I can update the PR if we think this feature is useful. In fact, I would prefer getting rid of the doctrine event listeners and leverage symfony system in Doctrine. But that's an other story. |
|
Closing as the problem doesn't seem to occur all that often to justify dedicated exception to help debugging. |
When a doctrine event listener or subscriber calls the event manager in its constructor, the event manager has an incomplete initialization status. This can cause errors that are difficult to figure (for event listener) or invisible (for event subscribers).
This PR introduces
EventSubscriberCircularExceptionas a new feature to help debugging circular dependencies. This is not a bugfix since there is no broken behavior.