When an exceptions tries to bubble out of a shutdown function or the errorhandler (or autoload functions, but they are not handled yet), the following happens:
Fatal error: Exception thrown without a stack frame in Unknown on line 0
This is hell to debug, because you don't have a clue where the exception happened. It could be a hook_cron(), something with the session, a borked database table or something else.
This patch catches these and prints the actual exception message. Not that the message is currently simply printed, it's pretty much only a proof of concept and I'm looking for better ways to do it.
To do this for shutdown functions, the patch adds a new function called drupal_register_shutdown_function(), which registers a single function called _drupal_shutdown_function() if required so. All registered functions are then executed in that function inside a try/catch block.
For more information, see also #566494-167: Running cron in a shutdown function breaks the site
Comment | File | Size | Author |
---|---|---|---|
#24 | catch_exceptions7.patch | 10.09 KB | Berdir |
#22 | catch_exceptions.patch | 10.02 KB | chx |
#16 | catch_exceptions6.patch | 10.09 KB | Berdir |
#15 | catch_exceptions5.patch | 10.04 KB | Berdir |
#13 | catch_exceptions4.patch | 7.19 KB | Berdir |
Comments
Comment #1
BerdirOk, let's only handle shutdown functions here, re-rolled the patch.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedparameters should be plural
I would personally store these in one static but I'm not too fussed about that.
t() may not be available. consider cached pages. try something like:
<?php
$t = function_exists('t') ? 't' : 'strtr'
$t(some string)
Powered by Dreditor.
Comment #4
moshe weitzman CreditAttribution: moshe weitzman commentedThis is a major DX regression.
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commentedThis patch helped me find an mysterious exception in devel's shutdown. This feature is mandatory now that we have exceptions. It should go in no matter what happens with poormanscron.
Fixed those issues and posted new patch.
Comment #6
moshe weitzman CreditAttribution: moshe weitzman commentedActually, I made minor plural change and minor t() change. Berdir's patch is RTBC. Credit him.
Comment #8
moshe weitzman CreditAttribution: moshe weitzman commentedtest failure in simpletest module. i have no idea what it means tho. help.
Comment #9
BerdirI'm working on that. Can someone also point boombatower to this issue as he knows that simpletest/batch api stuff hopefully well? :)
Comment #10
BerdirOk, this was ugly to find but pretty much logical.
We can't rely on drupal_static() in these functions because that is deleted during batch process somewhere.
The new patch uses a helper function which stores the callbacks in a normal static array.
Comment #11
moshe weitzman CreditAttribution: moshe weitzman commentedNice detective work ... Please wait for green before commit (we are not reporting status back to project-issue module these days?).
Comment #12
chx CreditAttribution: chx commentedIt truly is awesome but drupal_register_shutdown_function could keep the static instead of the shared getter as its return value is not used now. Make it so that if you dont pass in anything it does nothing but always returns its static. You don't even need references, woohoo. Nice job, dude.
Comment #13
BerdirOk, let's try this.
Changed as suggested above by chx. It still needs references though, for two reasons.
- This basically allows contrib to replace core shutdown functions with their own, by getting them and replacing bya specific function ;)
- Some modules (even core, now that cron is called in a shutdown function) like devel.module register new shutdown functions in a shutdown function.
Comment #14
chx CreditAttribution: chx commentedIf that's the case then foreach wont work, you need a while(list=each) loop because foreach copies.
Comment #15
BerdirYes, of course :)
I thought the same but wasn't sure as I've seen one case where clon_cleanup has been called. but that was probably because simpletest calls drupal_run_cron() directly when installing a test env.
I also added a simple test case to confirm this and it passes with the while while it failed with a foreach. Not sure if the test can be improved, though... I could add a drupal_static_reset() call but I'm not sure how much that actually helps...
Comment #16
BerdirChanged to t() instead of check_plain() in the dummy shutdown functions...
Comment #17
chx CreditAttribution: chx commentedYay!
Comment #19
Berdir#16: catch_exceptions6.patch queued for re-testing.
Comment #20
Berdirchx and moshe had set this to rtbc, status was reset because of a test failure (which seemed to be a test client hickup as it works now, after a re-test). Who could ask for more ? ;)
Comment #21
webchickI think now that I committed #325169-94: Move error/exception handler higher up in the bootstrap process, we no longer need that weird t() / strtr check. Could someone double-check?
Comment #22
chx CreditAttribution: chx commentedRemoved $t and grabbed this excellent opportunity to change the !is_null to isset.
Comment #23
moshe weitzman CreditAttribution: moshe weitzman commented_drupal_decode_exception() may not be available. That only gets included with full bootstrap AFAICT. Perhaps include errors.inc before using _drupal_decode_exception()?
@param $parameter - should be plural (my fault).
Comment #24
BerdirIncluded errors.inc when necessary and changed to $parameters.
Comment #25
moshe weitzman CreditAttribution: moshe weitzman commentedgood for me. WFGBC
Comment #26
webchickWFGBC. Do I want to know what that stands for? ;)
Committed to HEAD. Thanks a lot, folks!
Comment #27
BerdirWait for green before commit :)
Comment #28
webchickAhhh. :) Thanks!
Comment #29
andypostIntroduced drupal_register_shutdown_function() statically caches handlers so it breaks simpletest:
- When running a interactive tests - handlers from testing setup in drupal_register_shutdown_function() statically
- When Simpletests clears data there's no ability to clear handlers
To reproduce simply use minimal install or just uninstall search module and get
Comment #30
andypostFiled another issue #721210: Interactive simpletest broken because shutdown handlers mix