Skip to content

[WIP] Handle unserialize() failures gracefully#198

Closed
prisis wants to merge 1 commit into
php-cache:masterfrom
PrisisForks:issue-65
Closed

[WIP] Handle unserialize() failures gracefully#198
prisis wants to merge 1 commit into
php-cache:masterfrom
PrisisForks:issue-65

Conversation

@prisis
Copy link
Copy Markdown
Contributor

@prisis prisis commented Nov 10, 2017 •
edited
Loading

Question Answer
Bug fix? yes
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets php-cache/issues/issues/65
License MIT

Description

errors triggered by unserialize should be turned to cache misses.

TODO

  • Add tests
  • Add documentation
  • Updated Changelog.md

@prisis prisis requested a review from Nyholm November 10, 2017 16:29
@prisis prisis changed the title Issue 65 [WIP] Handle unserialize() failures gracefully Nov 10, 2017
@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 10, 2017 •
edited
Loading

Codecov Report

Merging #198 into master will decrease coverage by 0.89%.
The diff coverage is 55.93%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master     #198     +/-   ##
===========================================
- Coverage      82.2%   81.31%   -0.9%     
+ Complexity      601      567     -34     
===========================================
  Files            33       28      -5     
  Lines          1540     1450     -90     
===========================================
- Hits           1266     1179     -87     
+ Misses          274      271      -3
Impacted Files Coverage Δ Complexity Δ
src/Adapter/Apc/ApcCachePool.php 0% <0%> (ø) 14 <0> (+1) ⬆️
src/Adapter/Memcache/MemcacheCachePool.php 0% <0%> (ø) 9 <0> (ø) ⬇️
src/Adapter/MongoDB/MongoDBCachePool.php 0% <0%> (ø) 15 <0> (-2) ⬇️
src/Adapter/Redis/RedisCachePool.php 96.96% <100%> (-1.18%) 14 <0> (-10)
src/Adapter/Memcached/MemcachedCachePool.php 94.11% <100%> (-2.95%) 13 <0> (ø)
src/Adapter/Filesystem/FilesystemCachePool.php 94.02% <100%> (-1.5%) 25 <0> (ø)
src/Adapter/Predis/PredisCachePool.php 94.28% <100%> (-2.86%) 15 <0> (ø)
src/Adapter/Common/CacheItem.php 97.72% <100%> (+0.05%) 37 <0> (+1) ⬆️
src/Adapter/Illuminate/IlluminateCachePool.php 88.37% <54.54%> (-11.63%) 19 <0> (+2)
src/Adapter/Doctrine/DoctrineCachePool.php 89.13% <54.54%> (-10.87%) 21 <0> (+5)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fa022f...775df63. Read the comment docs.

Nyholm
Nyholm reviewed Nov 22, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Good work!
Let me know when you are done with this PR.

@prisis
Copy link
Copy Markdown
Contributor Author

prisis commented Nov 22, 2017

I do, as always :D

@oytuntez
Copy link
Copy Markdown

oytuntez commented May 16, 2021

I think this may be an important PR.

@garak garak mentioned this pull request Oct 17, 2022
Open
@prisis
Copy link
Copy Markdown
Contributor Author

prisis commented Jun 23, 2023

Due to changes in interests, I will be closing this PR after several years.

@prisis prisis closed this Jun 23, 2023
@DavidGoodwin
Copy link
Copy Markdown

DavidGoodwin commented Mar 12, 2024

Shame this PR was dropped - I see e.g.

unserialize(): Passing null to parameter #1 ($data) of type string is deprecated in ...../vendor/cache/predis-adapter/PredisCachePool.php on line 48

when there's a cache miss

@DavidGoodwin
Copy link
Copy Markdown

DavidGoodwin commented Mar 12, 2024

see also #277

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nyholm Nyholm Nyholm left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@prisis @codecov-io @oytuntez @DavidGoodwin @Nyholm