This bug is related to the #1233232: Add unified context system to core issue.
The only core test case that doesn't pass following the WSCCI/Context API core inclusion is the Statistics module failing to pass test when a cached page is served.
Here is the real cause of the bug:
- Statistic module's
hook_exit()callsarg() arg()callsdrupal_get_context()->get('path:system')- This calls
HandlerPathSystem::getValue() - ... which calls
drupal_get_normal_path() drupal_get_normal_path()is defined in thepath.incthat isn't included yet.- WSOD happens
The worst part is this error remains silent, because _drupal_bootstrap_page_cache() already sent the headers and the full cached content, browsers received everything and this is run after. The crash is not a WSOD, because HTML is fully sent, but the error is fatal so PHP don't send the notice.
The only way to see it is in your HTTPd/PHP error log.
Now there is three methods to fix it, in order from the ugliest to the nicest:
- Add a pragmatic
require_onceinside the statistic module'shook_exit()implementation: this is ugly and it doesn't solve the potential bug for all other modules. - A compromise temporary quick solution is to do the same
require_onceinside theHandlerPathSystem::getValue()method. Yet again still not really nice. - Best solution is to refactor the path handling using OO code and let the autoloader do its magic: this need more than a one line path and need its own issue if that is the way to go.
What do you think?
Comments
Comment #1
aspilicious commentedI would go with 2 for the moment and add a big @todo and open a major issue for 3 when context is in.
Comment #2
aspilicious commentedhttp://drupalcode.org/sandbox/Crell/1260830.git/shortlog/refs/heads/1355...
New branch with stopgap fix
Comment #3
Crell commentedOption 3 is definitely the correct long-term solution, no question. For expediency we could do #2 for now to make everything work. I defer to Dratch on what they'd prefer to see.
Comment #4
catch#3 beejeebus is working on in #464164: Move URL alias functionality into a Path API module (still separate from the Path UI).
Comment #5
Anonymous (not verified) commentedi'm ok with a workaround, then circling back to the path code.
i just talked with Crell about this in IRC, and he indicated that there could be issues with a) making the path code pluggable and b) making it a module. i'll let him clarify that, but i don't think we should block this on resolving all of that.
so i'm ok with getting this in with a work around and continuing the discussion in #464164: Move URL alias functionality into a Path API module (still separate from the Path UI) and #1269742: Make path lookup code into a pluggable class.
Comment #6
Crell commentedGreat. Hack for now it is!
My comments on the path system itself and its swappability are here: #1269742-14: Make path lookup code into a pluggable class
Comment #7
Crell commentedMerged the hack. We can fix it in later follow-up issues.