mercure icon indicating copy to clipboard operation
mercure copied to clipboard

Add event on Update publish to Hub

Open tostiheld opened this issue 3 years ago • 4 comments

For debugging purposes, we maintain an API log. We have a listener on KernelEvents::TERMINATE that writes any relevant request/response information to a log file. However we do not have this for SSE's. That's why we now decorate the Publisher/Hub to add this functionality. I thought it might be nice to submit this change as a pull request, as I can imagine it might be useful for others as well.

tostiheld avatar Jun 27 '22 15:06 tostiheld

The code looks good to me, however I'm unsure if this use case is frequent enough to be covered directly in the core component, as it's already possible to use the current extension points. WDYT @symfony/mergers?

dunglas avatar Jun 27 '22 18:06 dunglas

By other extension points you mean decorating the Hub? Or is there some other extension point I am not aware of?

tostiheld avatar Jun 27 '22 20:06 tostiheld

Yes, decorating the hub.

dunglas avatar Jun 27 '22 22:06 dunglas

Adding such event is consistent with e.g. the event dispatched by the Workflow or Console components. That said, we're not very consistent with events in components (e.g. HttpClient and Mailer don't dispatch lifecycle events).

Decoration does add some cognitive overhead, as you need to look at the DI configuration file in order to discover that the default hub service is decorated with one of your own classes.

So I'm leaning towards +1 for this PR. Can we think of other use-cases of this event? Otherwise, we maybe can add optional support for psr/log directly in hub instead of dispatching an event?

wouterj avatar Jun 28 '22 13:06 wouterj

I would prefer adding optional support psr/log than maintaining another extension point.

dunglas avatar Nov 14 '22 18:11 dunglas

Closed in favour of #110

tostiheld avatar Dec 09 '23 10:12 tostiheld