-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Mailer] Sending Messages Async with json serializer does not work #33394
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
Comments
Iam having the same issue. Sending message async doesn't work. On serialization I recive " A message must have a text or an HTML part or attachments." |
Check if your symfony/twig-bundle is in 4.3 version - helped for me |
Having exact same problem, and have |
Can any of you create a small example application that allows to reproduce the issue? |
Sample app showing error: https://github.com/diabl0/async-mailer |
I can confirm this issue. It happens because we cannot normalize We need to use a serializer with the |
Actually, i've already written about this bug to fabpot, about 6 months ago i hope, but I can't find it now, so i can't say what he said about this problem. For quick fix, my decision was to create custom
|
I did an ugly trick with serializer, because SendEmailMessage structure is too complicated for json. The special serializer was added to a project
And one more, I did not find how to inject context to serializer for specific class, so there is a file for exclude attributes:
Firstly I added a specific route and serializer, but there is an error when SendEmailMessage goes to failed queue it decoded as json and this task never be processed successfully.
|
@TheRatG creating a seperate transport with the native php serializer worked for me. |
Wouldn't adding a |
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [Mime] Fix serialization of RawMessage | Q | A | ------------- | --- | Branch? | 4.4 <!-- see below --> | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #38430, Related #33394 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT | Doc PR | - <!-- required for new features --> The serialization of RawMessage is currently broken if using a generator for message like done by `Symfony\Component\Mailer\SentMessage` see https://github.com/symfony/symfony/blob/5f1c3a797247a6d54992384df00bb22741fc1c34/src/Symfony/Component/Mailer/SentMessage.php#L45 This patch converts the message to a string so further serialization can be done. This patch probably also solves #33394. <!-- Replace this notice by a short README for your feature/bugfix. This will help people understand your PR and can be used as a start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - Never break backward compatibility (see https://symfony.com/bc). - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch 5.x. --> Commits ------- fd99eb2 [Mime] Fix serialization of RawMessage
not actually solved by the related PR, as it was fixing only the case of |
Having the same issue with fresh Mailer installation (v5.2.3) and Hope it will be fixed, but looking at issue's creation date... 🙄 |
Thank you for reporting the issue still exists. I would be happy to review a PR that fixes this issue. Im sure that everybody knows that "open source software" means that everybody can help fixing bugs or support the development. Could someone debug the problem or create a reproducible script (repository) to show this issue? |
@Nyholm I believe @diabl0's repo is a good starting point, I've just reproduced the issue locally with it. Also I've changed
and after
Unfortunately this repo requires local PHP installation (with |
Thank you. Could you help me understand why 87f3284 wasn't a fix for this problem? Also, do you have any suggestions for what actually will fix the issue? |
@Nyholm AFAIS For reference, this is payload sent to RabbitMQ with
I've tested @weaverryan's suggestion and it somehow fixes deserialization issue, but it ends with:
I've just debug it a little and When I change |
Actually looking at the
where FYI: If it's acceptable solution I would like to make PR with this change, let my debug time be awarded 😉 |
@Nyholm Personally I am not so sure this is Serializer's issue. It looks to me like architectural mistake in Mailer/Mime component - there is It's from IMO Mailer/Notifier (and every other component that works with emails) should work with actual Without bus (using Changing But it's Symfony's team decision of course 🙂 Maybe it was done this way for a reason I just don't get. |
Hey, thanks for your report! |
Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3 |
Hey, I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen! |
The issue still exists. I had to use the NativePhpSerializer to make it work.
|
just had the same problem, found this issue, it very much is still a problem in 5.3 |
I have the same issue on 5.3 |
@xabbuh since you reopened this issue, what are the next steps for this ? We have been using messenger in production for a couple of weeks and being stuck with php-serialization instead of json-based message body is unfortunate. Regarding @Wirone 's comment #33394 (comment) there seem to be a solution but we need a maintainer's input. I would be willing to make the changes if needed. |
@florentdestremau I support your change, we need to use JSON instead of php-serialization |
@florentdestremau please make a PR, I will review it and hopefully, we can get it merged. This is important feature |
I have no idea what is needed so unless a contributor is willing to point out the direction, this isn't going anywhere for now |
@Nyholm is there any chance of getting some guidance on this issue? It's blocking a pretty major PR we need to integrate in our next major release of Mautic to switch from Swiftmailer to Symfony Mailer but folks don't seem to be clear on how to proceed. |
IIUC, there is no issue to fix in Symfony Mailer, but it's more about the inability to serialize an email to JSON via the Symfony serializer. |
@fabpot I don't know if you're familiar with my comment but IMHO there was architectural issue in Mailer/Mime component. I said "was" because I don't use Messenger/Mailer currently and I don't know state of the codebase. But looking at |
what about widening |
@fabpot please, could you explain why is it designed this way ( #33394 (comment) ) ? 🙏
seems to me that |
…symfony/symfony#33394). Using the combination of JSON and native PHP serialization is not a way as it causes issues with the failed transport.
…symfony/symfony#33394). Using the combination of JSON and native PHP serialization is not a way as it causes issues with the failed transport.
Any update on this one? |
As rapid fix, I reused the Normalizer provided by @TheRatG (many thanks to them!) to work around this issue. I added a AutoconfigureTag to enable it faster. Tested on Sf 7.4.
What would be the best long-term solution here? Should we consider revisiting the RawMessage class so it can be natively supported by the Symfony serializer? That could imply deprecating it in its current form and introducing a new class that would be fully supported wherever RawMessage is currently used. Alternatively, could introducing a few interfaces help make things more flexible and extensible? Or would it make more sense to simply include this Normalizer directly in the Mail component? I'd love to hear your thoughts on the most sustainable path forward. |
Symfony version(s) affected: symfony/mailer: 4.3.4
Description
Sending Messages Async with json serializer does not work with SendEmailMessage.
How to reproduce
The text was updated successfully, but these errors were encountered: