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

Meaningless exception for corrupted cache files #230

Closed
ndobromirov opened this issue Mar 10, 2021 · 1 comment · Fixed by #200
Closed

Meaningless exception for corrupted cache files #230

ndobromirov opened this issue Mar 10, 2021 · 1 comment · Fixed by #200
Assignees

Comments

@ndobromirov
Copy link

throw new RuntimeException('Invalid cache file "' . $options['cacheFile'] . '"');

I am getting the following exceptions in a slim APP in some random cases.

The exception thrown on the line above is redundant and causes more harm than it helps.
The correct code should be something like so (with no exceptions thrown):

        if (!$options['cacheDisabled'] && file_exists($options['cacheFile'])) {
            $dispatchData = require $options['cacheFile'];
            if (is_array($dispatchData)) {
                return new $options['dispatcher']($dispatchData);
            }

            // Cache file is corrupted, so delete it and regenerate it below.
            @unlink($options['cacheFile']);
        }

The only issues I can think of are race conditions during the cache file regeneration that are not handled in the code in any way at the moment. The new solution will cause a stampede of the cache, where the old one will throw an exception that can cause a fatal because of a cache layer (should not happen).

In short if there is a cache and it is valid (an array) - use that.
If there is an invalid file - delete it and proceed with regenerating it during the request.

@lcobucci lcobucci linked a pull request Jun 14, 2021 that will close this issue
@lcobucci
Copy link
Collaborator

@ndobromirov that's a valid point. I'm redesigning the file-based cache in #200, which should address this too.

@lcobucci lcobucci self-assigned this Jun 14, 2021
@lcobucci lcobucci changed the title Meaningless exception. Meaningless exception for corrupted cache files Jun 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants