Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow Subscriber listening on the same event multiple times #655

Open
DanielBadura opened this issue Dec 12, 2024 · 2 comments · Fixed by #701
Open

Disallow Subscriber listening on the same event multiple times #655

DanielBadura opened this issue Dec 12, 2024 · 2 comments · Fixed by #701
Assignees
Labels
BC-Break bug Something isn't working

Comments

@DanielBadura
Copy link
Member

DanielBadura commented Dec 12, 2024

We enabled to have multiple #[Subscribe(Event::class)] in one Subscriber when we added the feature to listen to all events with * notation, see here. This leads to the situation that the subscription engine cannot work 100% perfectly. What does that mean? If a Subscriber listens to one Event with multiple methods it can lead to unwanted actions. Let's take this Subscriber as an example:

#[Projection('order')]
class OrderProjection 
{
   // ...
   
   #[Subscribe(OrderSubmitted::class)]
   public function sendConfirmationMail(OrderSubmitted $event): void 
   {
       // send mail
   }
   
   #[Subscribe(OrderSubmitted::class)]
   public function updateOrderInReadModel(OrderSubmitted $event): void 
   {
       // write to read model
   }
}

The described Subscribe may be defined as an antipattern here, as it is talking to 2 different systems here, but this is not the point of the Issue.

Now, if one of the 2 methods is failing, like e.g. the DB is not reachable, then the projection fails, which is normally no problem. But in this case, we dont know - or better the subcription engine does not know - which of these 2 methods failed. Since they are both listening on the same event the position is not helpful here. The result is, that both of these methods will be called again when retried. That could lead to either sending the same mail multiple times or corrupting the database with maybe false information if we are doing additive/mathy operations.

As I mentioned earlier: IMHO this situation would not occur if the software design would be better here in the application. So you could argue it is not a big issue then. But I can also not think of an valid usecase to allow this subscriber configuration. So I think we should not allow it at all.

Does it affect * notation? Yes, I think so, since we would have the same problem again if the have a method on the subscriber which listens on all events and some dedicated methods which are listening to some events. We would encounter the exact same problem again.

So my idea would be to disallow listening to the same event multiple times in a subscriber. As soon a subscriber listens to all events via * it cannot have other methods listening to any event.

@DanielBadura DanielBadura added bug Something isn't working BC-Break version: 4.0.x labels Dec 12, 2024
@DanielBadura DanielBadura self-assigned this Dec 12, 2024
@DavidBadura
Copy link
Member

I would do it as a bugfix in 3.x because otherwise it can cause big problems in production.

@DanielBadura
Copy link
Member Author

As I already mentioned that I cannot think of an valid usecase for this behaviour I think that we should fix it 3.x and not in our next major despite being a BC-Break. The risk is just too high that this might disrupt a production system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC-Break bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants