sroze / messenger-enqueue-transport

Uses Enqueue with Symfony's Messenger component.
MIT License
191 stars 54 forks source link

Delay stamps not supported #77

Closed spantaleev closed 4 years ago

spantaleev commented 5 years ago

I'm using Symfony's Messenger component, going through this interop layer and finally going through Enqueue.

It seems like envelopes are serialized as-is, without inspecting their stamps.. The DelayStamp is not taken into account at all when creating the interop message.

Now, it may probably be easy to loop through the original envelope and try to call $interopMessage->setDelay() when a DelayStamp is encountered. However it doesn't appear like this message delay thing is part of the interop Message interface.

So there's a need to either extend the interface or to do some method_exists() hacks on it.

Not supporting delay stamps is a complete dealbreaker for using this interop layer. If messages cannot be retried in a sensible manner (that is, with a delay), there's usually no point in retrying (10 retries within the same second usually leads to 10 failures).

weaverryan commented 5 years ago

Indeed, that method_exists() type of thing already happens if you use TransportConfiguration, which is a stamp that's specific to this library - here's that logic: https://github.com/sroze/messenger-enqueue-transport/blob/master/QueueInteropTransport.php

So, I'm pretty sure that the setMessageMetadata() method could be enhanced to look for the DelayStamp and use something like $class->hasMethod('setDeliveryDelay') and call it if it's present.

A PR is welcome! I think this could be an "easy pick" and I'm sure others also want this. It's not implemented simply because delaying, in general, is new to Symfony 4.3, and it hasn't been added here yet.

Cheers!

spantaleev commented 4 years ago

Thanks for the pointers!

I won't be able to spend the time to properly do it and test it out, so if someone else is willing to work on it, please go ahead!

kor3k commented 4 years ago

perhaps related to https://github.com/sroze/messenger-enqueue-transport/issues/66

Nyholm commented 4 years ago

This is fixed by #84