[wp-trac] [WordPress Trac] #52345: Transient with expiration never expires when storing expiration failed

[WordPress Trac]

#52345: Transient with expiration never expires when storing expiration failed
-------------------------------------------------+-------------------------
 Reporter:  mennoll                              |       Owner:  (none)
     Type:  defect (bug)                         |      Status:  new
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  Options, Meta APIs                   |     Version:  2.8
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-      |     Focuses:
  refresh                                        |
-------------------------------------------------+-------------------------

Comment (by mennoll):

 Hi Peter, thank you for the response. To first reply to the last question
 you asked: yes, i've encountered this issue a few times now, in multiple
 websites. From testing, it looks like getting transients is going well,
 except for when a transient is not requested for some time. It looks like
 some garbage collection feature is deleting the transient timeout value
 from the database. I was not able to see where that went wrong, so I was
 not able to detect if that's a problem in WordPress Core, or in a plugin.
 I searched in my plugins but there was no code to be found that could
 interfear with how transiens work. Therefore it could be in core, but
 looking in code, I could not find the immediate problem.

 However, like you can see, I was able to modify the `get_transient()`
 function to let it work correctly.

 I see you are worried about that my solution might break transients that
 don't have a timeout. But I do think that this is not true. The code
 change I made is inside the if statement, that checks if a transient
 should have a timeout: (see this code below). Therefore I think this
 change only applies to transients that do have a timeout.

 {{{#!php
 <?php
 // If option is not in alloptions, it is not autoloaded and thus has a
 timeout.
 $alloptions = wp_load_alloptions();
 if ( ! isset( $alloptions[ $transient_option ] ) ) {
     // Code is here
 }
 }}}

 I am indeed high aware of the issue that you pointed out in the Github PR,
 and how we could possibly fix my experienced bug via the `get_transient()`
 function. Unfortunately, I do also think that's not the full solution. I
 do think, we actually need both your suggestion, and mine.
 To be even more clear, I do think that if I read trac ticket
 [https://core.trac.wordpress.org/ticket/30380#ticket WP#30380], that the
 ticket is actually outdated. I fully understand that they wanted to save a
 few queries here because they seemed useless. But I think their unit test
 is not valid anymore.
 We can however still pass their unit test, and also have a solution. If I
 change my code so that the transient will not be deleted, but the
 `get_transient()` still returns false, all unit tests should succeed.

 To summarize, I think we need to discuss a little about the correct
 solution, and am happy to do so. Could you please give your opinion about
 the following list of questions:? Thanks :)

 - I think that my change does not affect permanent transients. Could you
 please explain how you think it will?
 - What do you think about both having a check in `get_transient()`, and
 your suggestion in `set_transient()`?
 - I have doubt if i think the trac ticket 30380 is still valid, and
 therefore would like to know if you think that in my code deleting the
 transient there is the way to go (which requires that unit test to be
 deleted)

 Thank you, hope to hear from you soon.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/52345#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform