-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Scheduler][Messenger] Message can't be redispatched with Symfony serializer in between #53562
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
As a temporary workaround I've included a middleware in the command bus to remove the stamp that's causing the issue:
|
Should the trigger property be serialized as a string and not as an object?
final class MessageContext
{
public function __construct(
public readonly string $name,
public readonly string $id,
public readonly string $trigger,
public readonly \DateTimeImmutable $triggeredAt,
public readonly ?\DateTimeImmutable $nextTriggerAt = null,
) {
}
} Any idea @tucksaun ? |
@Gwemox The As far as I'm aware the "Symfony way" of doing serialization/deserialization is through normalizers, isn't it? 🙂 |
@christian-kolb It is not necessary to serialize the entire trigger object because it is not intended to be used once deserialized. In its current form, if you instantiate it again, you will lose its state. 🙂 |
Is there to way to serialize the state? When not using JSON serializer and using the default PHP serializer, that state would still exist, wouldn't it? As a user of the component I would never guess that data is being lost when switching the serializer. I don't know how to do it (not deep enough in it), but at the moment it just looks like the implementation isn't "completed". |
Schedule triggers may contain closures nowadays so it is pretty much unserializable, even with native serializer. To me that string conversion sounded perfectly ok, I bet more people are struggling right now with Scheduler not working at all vs. not being able to get the trigger from the stamp. If you need the trigger object, I think we should rather add I'm considering submitting a PR on that since for many people this is really blocking the upgrade to 6.4. |
IIRC having the trigger available was an idea from @kbond and this looked sensible to me back then. Without a change on the context, indeed this means that when using While we look for a good Symfony solution (or wait to be allowed for a BC), the safest way here could be to have a small handler for the recurring message that is handled synchronously and does the redispatch ? |
Since the trigger isn't required anymore at this point, could we create a normalizer that normalizes to the string, then denormalizes to a stub object - maybe just an anon. class that implements the interface, throws an exception in To account for the possibility of Closures in the trigger causing serialization problems, perhaps something similar to above in |
…valtzu) This PR was merged into the 7.3 branch. Discussion ---------- [Scheduler] Normalize `TriggerInterface` as `string` | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | yes | Deprecations? | no | Issues | Fix #53562 | License | MIT Fix the issue with Scheduler & Serializer like `@kbond` described in symfony/symfony#53562 (comment). Fixes the following error: ``` Could not decode stamp: The type of the "trigger" attribute for class "Symfony\Component\Scheduler\Generator\MessageContext" must be one of "Symfony\Component\Scheduler\Trigger\TriggerInterface" ("array" given). ``` Commits ------- 3aa47e0ada2 Normalize `TriggerInterface` as `string`
Symfony version(s) affected
6.4.0
Description
When using the
RedispatchMessage
in combination with the scheduler and the symfony serializer (with JSON), then the messenger fails while trying to deserialize the message. This is due to theMessageContext
class which expects theTriggerInterface
for the$trigger
property. When the serializer serialized the message to JSON, the information which implementation of the trigger is used, is lost.I think we're missing a custom normalizer for the
MessageContext
that is able to infer the correct implementation depending on the content.This is not a problem when the default PHP serialization is used as the relevant class information is backed into the serialized content. I guess that's also the reason that this issue didn't come up yet.
How to reproduce
Create a schedule with any message that transfers to a different transport (so that the message is written to a message queue).
Configure the messenger to use the Symfony serializer like shown in the documentation:
Make sure to run the async messenger queue and at the then start the scheduler:
Then the job queue will fail with the following error stack:
Possible Solution
Having a custom normalizer that can denormalize the
TriggerInterface
depending on the context. That of course would not solve the issue when someone implements their own trigger. But then it's "simple" to construct a custom normalizer for the project.This is a simplified version of such a normalizer that specifically only handles
PeriodicalTrigger
(which fixes my local problems):Additional Context
The primary question I'm having is about the approach. Does this makes sense to have an additional normalizer? If so, does it make sense to extend the triggers with logic for normalization and denormalization? They are very flexible to use from the outside, but as a side effect not very "normalizable". The example I've added is a poor man version that doesn't cover a lot of the internals and I'm not sure whether with the current architecture it even could be handled from the outside or whether we need some kind of logic exposed to the outside specific to normalization.
Would ask for direction here.
The text was updated successfully, but these errors were encountered: