Skip to content

feat: Drop exception catching and logging, fix retry and failure hand… #206

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

mleczakm
Copy link
Contributor

@mleczakm mleczakm commented Aug 5, 2025

…ling by default messenger way

Fixes #205

@mleczakm mleczakm requested a review from a team as a code owner August 5, 2025 19:33
@Rastusik
Copy link
Member

Rastusik commented Aug 6, 2025

heye @mleczakm ! thank you for the contribution!

I'm afraid I'm not able to merge this without a reproducer test.

But there is an important design question here - is it really needed to support retries in the task workers? The thing is that we're in non-persistent context here (php process memory). Retries usually use some kind of a durable queue. With too many retries in Swoole, I'm afraid there would be a risk for out of memory errors.

Maybe you should reconsider your design decisions?

@mleczakm
Copy link
Contributor Author

mleczakm commented Aug 6, 2025

@Rastusik That exception logging is also silencing errors. It breaks error handling in Symfony and interferes with message processing in Messenger. These are not my design decisions, but part of the Symfony Messenger design. Since this is a bridge for it, it should fully support that behavior.

What’s also important – it breaks the failed queue mechanism. While it can be fixed by reconfiguring services (so it's possible to fix it in userland), there’s no point in breaking the default framework behavior with no real profit - exception would be logged anyway.

@Rastusik
Copy link
Member

Rastusik commented Aug 6, 2025

I believe the exception catching is for a reason there (need to find time to look into it closer). if the exception won't be caught, the swoole worker process will terminate.
the point of this messenger integration is just to use the swoole task mechanism with a common symfony messenger interface.

I can look later where the deleted class is exactly hooked in, but I'm still not sure if I am the one who's missing context, or you, or us both :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Retry and failed transport not working for worker tasks
2 participants