Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Coming from #1879726: Should statistics be collected on every page?
Origin report by mondrake
Notice: Trying to get property of non-object in drupal_lookup_path() (line 77 of /..../includes/path.inc).This is happening when serving a cached page, and you have specified page-based restrictions.
There is a drupal_get_path_alias() on line 147 of better_statistics.statistics.inc, which calls a drupal_lookup_path(), which in turns expects the global $language_url to be set - but with low bootstrap phase it looks like this is not defined, hence the notice.
...
Nothing broken AFAICS. I updated the module on a live site where I applied the following restrictions:
- cache: enabled
- roles: only anon users
- pages: all but an excluded pathEverything gets logged as expected, but any time a page is served from cache, the php notice gets logged in the watchdog.
And yes, the site is multilingual, albeit URL detection is not enabled.
I just gave a try at raising the bootstrap phase to DRUPAL_BOOTSTRAP_LANGUAGE in the hook_exit implementation - which takes away the notice, but unfortunately is very late in the page processing request and raises some 'headers already sent' warnings, which is definitely worse.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | better_statistics-fix_notice-1907684-4-test_only.patch | 1.9 KB | mondrake |
| #4 | better_statistics-fix_notice-1907684-4.patch | 2.65 KB | mondrake |
| #1 | better_statistics-fix_notice-1907684-1.patch | 770 bytes | mondrake |
Comments
Comment #1
mondrakeNot sure if this is the right thing to do...
this patch checks if the global variable $language_url is set just before calling drupal_get_path_alias(), and if not initializes the language via drupal_language_initialize(). We're already at hook_exit here, and this hopefully "shouldn't" be creating issues with drupal_bootstrap().
If we were to raise to DRUPAL_BOOTSTRAP_LANGUAGE via drupal_boostrap(), that would also invoke _drupal_bootstrap_page_header() hence trying to open headers again (but here the cached page has already been sent I believe).
This seems to be working for me, I'll report back when I have more history.
Comment #2
mondrakeForgot to change status... :(
Comment #3
iamEAP CreditAttribution: iamEAP commentedThis seems fine to me. Let me know if you come across any side effects, otherwise I'm fine committing this.
Comment #4
mondrakeFix in #1 works fine in my environment, no side effects. I am re-rolling here with a test as well.
Arguably, the test is much more complex than the fix itself :(, so please take a good look at it.
Cheers
Comment #5
iamEAP CreditAttribution: iamEAP commented#4: better_statistics-fix_notice-1907684-4.patch queued for re-testing.
Comment #6
iamEAP CreditAttribution: iamEAP commentedThis looks good to me; the test may catch unrelated issues, but that's probably a good thing. Committed 0e9d65b.
Thanks, mondrake!