make sure we only use the cache if write was successful #82
Conversation
| // we did not write all characters, or completely failed to write. | ||
| @unlink($tmpFile); | ||
|
|
||
| return; |
There was a problem hiding this comment.
on other failures, this class throws exceptions. being a cache, i would expect it to continue silently, as cache problems should not break the application. if you prefer, i can change this to an exception to be more consistent with the behaviour of the rest of the cache implementation.
There was a problem hiding this comment.
this should be a very bad situation but I guess it makes sense to cover this case...
When the disk is out of space... a lot of other bad things could happen...
There was a problem hiding this comment.
indeed so. we don't maintain the servers themselves and when we checked, there was ample disk space available. but with enough writes, one might fail for some reason somewhere, and in our case it left us with this half-written cache that broke everything (on one of several nodes only, so it looked like the error only happens randomly...)
There was a problem hiding this comment.
Alright. I guess this is pretty hard to test, right?
There was a problem hiding this comment.
the happy case is tested, but failure is difficult to test. maybe with stream wrappers and whatnot, but stream wrapper is so fragile that it randomly breaks other things too.
|
how do you feel about merging this as it is now? would that be ok or do we need more tests? and do you plan a patch release after these improvements and fixes? there are a few other tweaks since 2.0.0 2.0.0...master |
|
This is ok for me! Will do a patch release soon! will have to put it in my agenda ! |
|
thanks a lot! |
I don't know how to test this, as those things normally don't fail. (Most likely offender would be out of disk space...). But i did validate with a 😃 in the TestObject that utf8 in code does not break the strlen vs bytes written.
See also #81 for when the data already is written or a disk is corrupted later on.