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:

  1. Statistic module's hook_exit() calls arg()
  2. arg() calls drupal_get_context()->get('path:system')
  3. This calls HandlerPathSystem::getValue()
  4. ... which calls drupal_get_normal_path()
  5. drupal_get_normal_path() is defined in the path.inc that isn't included yet.
  6. 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:

  1. Add a pragmatic require_once inside the statistic module's hook_exit() implementation: this is ugly and it doesn't solve the potential bug for all other modules.
  2. A compromise temporary quick solution is to do the same require_once inside the HandlerPathSystem::getValue() method. Yet again still not really nice.
  3. 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

aspilicious’s picture

I would go with 2 for the moment and add a big @todo and open a major issue for 3 when context is in.

aspilicious’s picture

Crell’s picture

Option 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.

catch’s picture

Anonymous’s picture

i'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.

Crell’s picture

Great. 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

Crell’s picture

Status: Active » Fixed

Merged the hack. We can fix it in later follow-up issues.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.