RedisWatcher does not serialize an object and throws a conversion error

[CSalih]

Telescope Version

4.15.2

Laravel Version

9.x

PHP Version

8.2

Database Driver & Version

---

Description

Predis requires for some commands an object as argument (e.g for search) but RedisWatcher::formatCommand does not serialize objects, due to the missing serializing an object will be returned instead of a string and implode fails to concatenate it.
Because the list of $parameters may contain an object a serialization is required to avoid an exception.

I made for myself a dirty quick patch but it needs a better solution to make sure $parameter is always a string.

    private function formatCommand($command, $parameters)
    {
        $parameters = collect($parameters)->map(function ($parameter) {
+            // dirty workaround
+           if (method_exists($parameter, "toArray")) {
+              $parameter = $parameter->toArray();
+           }
            if (is_array($parameter)) {
            ...
        })->implode(' ');

        return "{$command} {$parameters}";
    }

Steps To Reproduce

1. Enable RedisWatcher (e.g. env TELESCOPE_REDIS_WATCHER=true)
2. Use predis as client (e.g. env REDIS_CLIENT=predis)
3. Execute following code

$args = new SearchArguments();
$args->dialect("3");
$args->withScores();
$result = Redis::ftSearch("your-index", "*", $args);

The command itself will be executed successfully but dispatch of CommandExecuted throws an exception with the message: "Object of class Predis\Command\Argument\Search\SearchArguments could not be converted to string".

[crynobone]

@CSalih is there any simplier example where we can use to create a test to cover the bugfix?

[CSalih]

Hi @crynobone, the simples way is probably to create a PredisCommandArgumentFake class with a simple toArray implementation (it may return ["FAKE"]), with this class we can "mock" ArrayableArgument without depending on predis.

A simple test could be like:

$properties = ["idx", "*", new PredisCommandArgumentFake()];
$commandExecuted = new CommandExecuted("command", $properties, ...);

// test should make sure `toArray` is called
// test formatted string should contain the same return value of the Fake class
// ...
// with this test(s) any predis command argument should be covered.

For code quality i would suggest to test any kind of type

$properties = ["str", 12, 12.5, true, [], new stdClass()];
$commandExecuted = new CommandExecuted("command", $properties, ...);

// properties may be an invalid redis client argument but it avoids the application to break here
// with this test we make sure implode does not fail anyway

I hope I could be helpful to you.

[crynobone]

A PR to fix this issue has been merged on Laravel Framework 9 and will be available soon.