Sentry-php: Allow class_serializers option to allow generic serializers without specific type, handling any object

[JanMikes]

Hi!

I like concept of class serializes, but i have a bit specific use case:

I have 100+ classes like this:

final class OrderId
{
    use EntityId;
}

And would love to be able to serialize them. They have no common parent or either interface.

At first my idea was something like:

final class GenericSentrySerializer
{
    /**
     * @param object $object
     */
    public function __invoke($object): ?array
    {
        if (method_exists($object, 'id')) {
            return [
                'id' => (string) $object->id(),
            ];
        }

        if (in_array(EntityId::class, \Safe\class_uses($object), true)) {
            return [
                'id' => (string) $object,
            ];
        }

        return null;
    }
}

But this does not work, since i have to pass into Options object array of $type => $serializer (AbstractSerializer::resolveClassSerializers():

foreach ($this->options->getClassSerializers() as $type => $serializer) {
            if ($object instanceof $type) { // <<<<<---- here it will fail
                $serializers[] = $serializer;
            }
        }

Do you have any ideas how to overcome this limitation?

I could imagine having option generic_class_serializers or universal_class_serializers :-)

[stayallive]

This is a bit of an error prone option to add 😄 but not much more "unsafe" than what we have in place so I guess it's not a terrible idea to add.

This could also allow the user to implement a single serializer instead of making multiple serializers for all types, could simplify adding serializers to your app.

I think class_serializer (singular) would be a good option name to allow the user to set a single class serializer that should take precedence over the class_serializers (if it returns an array).

The signature of such serializer callable would be:

function serialize($object): ?array {}

---

Another option could be to allow * as a "type" in the class_serializers option so you can add a wildcard serializer.

---

I'm not so sure we should allow multiple of those generic serializers, but thats more of a feeling that it's a bad idea and would generally be unneeded since a single class serializer could always call out to others if it needs.

---

What do we think about that?

[Jean85]

I'm opposed to this :-1: if you want to apply a general solution to your serialization needs, you can replace the whole serializer with ClientBuilder::setSerializer.

[ste93cry]

I don't like the idea of adding yet another option when we already have something similar that just doesn't work for one use-case. Instead, if we really want to support this I'm more inclined to reuse somehow what we already have by supporting * or even better by finding the way to have some kind of interface for the class serializers so that each of them can tell if they support serializing something instead of us doing the check for them, e.g:

interface ClassSerializerInterface
{
    public function serialize($value): array;

    public function supports($value): bool;
}

[JanMikes]

I did not know about ClientBuilder::setSerializer() option and i believe it will be good enough solution for this use case 👍

Though i am worried about how difficult it was to get this working with Symfony, as this part is not very well documented (but probably it is more related to the https://github.com/getsentry/sentry-symfony repository), i created issue for that - https://github.com/getsentry/sentry-symfony/issues/251

interface ClassSerializerInterface
{
    public function serialize($value): array;

    public function supports($value): bool;
}

I really like this one but it would cause a BC break 😞

I think class_serializer (singular) would be a good option name to allow the user to set a single class serializer that should take precedence over the class_serializers (if it returns an array).

This is as well good, but again, it would be a BC break.

As i was able to solve it for my usecase by using ClientBuilderInterface::setSerializer() with decorating the original serializer, if you do not want to continue in discussion about implementing this, feel free to close this issue.