Skip to content
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

[Cache] Improve some error messages #29710

Closed
javiereguiluz opened this issue Dec 28, 2018 · 6 comments
Closed

[Cache] Improve some error messages #29710

javiereguiluz opened this issue Dec 28, 2018 · 6 comments

Comments

@javiereguiluz
Copy link
Member

@javiereguiluz javiereguiluz commented Dec 28, 2018

Description
When I run some commands that make use of the cache, I get this:

$ ./bin/console app:some-command

10:45:08 WARNING [cache] Failed to save key "... some key ..." (array)
["key" => "... some key ...", "type" => "array", "exception" => null]
["log_uuid" => "cf01082a-0f5f-4b36-86e9-3b5a5ee6832a"]

No additional info is provided on dev.log either, so this is tricky to debug. Thanks to @nicolas-grekas this was restricted to either a serialization problem of the cached content or the use of APCu-based cache. In this case it wasn't a serialization issue because the content was a simple array.

Let's check if we can add more error messages (or improve the existing ones) to explain the error in more detail and provide, if possible, solutions to fix them. Thanks!

@Simperfit
Copy link
Contributor

@Simperfit Simperfit commented Apr 6, 2019

That could be a nice first issue IMHO.

@Deuchnord
Copy link
Contributor

@Deuchnord Deuchnord commented Apr 7, 2019

Taking this for the EUFOSSA hackaton

@Deuchnord
Copy link
Contributor

@Deuchnord Deuchnord commented Apr 7, 2019

@javiereguiluz could you please provide a reproducer?

@nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Apr 7, 2019

E.g. when trying to cache a Closure I think.

@Simperfit
Copy link
Contributor

@Simperfit Simperfit commented Apr 10, 2019

So I made a little SF command to reproduce:

 /**
     * This method is executed after initialize(). It usually contains the logic
     * to execute to complete this command task.
     */
    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $function = function($test) {
            var_dump($test);
        };
        $array = [$function];
        $this->cache->set('test', $function);
        $this->cache->set('test2', $array);

    }

And I don't have any error but a just a debug:

06:51:51 DEBUG     [php] User Warning: Failed to save key "test2" (array)
06:51:51 DEBUG     [php] User Warning: Failed to save key "test" (Closure)
[
  "exception" => Symfony\Component\Debug\Exception\SilencedErrorContext {
    +count: 1
    -severity: E_USER_WARNING
    trace: {
      ./vendor/symfony/cache/CacheItem.php:197 { …}
      ./vendor/symfony/cache/Traits/ArrayTrait.php:131 { …}
    }
  }
]

How could we improve this message ?

@Deuchnord
Copy link
Contributor

@Deuchnord Deuchnord commented Apr 10, 2019

Thank you for these details @Simperfit, I'll take a look to this and make a PR :)

nicolas-grekas added a commit that referenced this issue May 11, 2019
… an unsupported type (Deuchnord)

This PR was merged into the 4.3 branch.

Discussion
----------

[Cache] Log a more readable message when trying to cache an unsupported type

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #29710   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |

Improved the warning risen when trying to save something that has a non-supported type in the Simple cache.

For instance, let's say the following code:
```php
class TestCommand extends Command
{
    private $cache;

    public function __construct(CacheInterface $cache)
    {
        parent::__construct();
        $this->cache = $cache;
    }

    // ...

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $n = $this->cache->get('n', function () {
            return function () {
                return rand(0,10);
            };
        });

        dump($n());
    }
}
```

Running this code will give the following:
```
14:32:03 WARNING   [cache] Could not save key "n" in cache: the Closure type is not supported.
0
```

Commits
-------

21ba3c0 [Cache] Log a more readable error message when saving into cache fails
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants