AccountProxy is not calling date_set_default_timezone() for anonymous users

Likely also is the proper fix of #2471405: When not logged Drupal\Core\EventSubscriber\FinishResponseSubscriber->setExpiresNoCache() causes a PHP warning because of an invalid timezone

Patch works for me, encountered the issue when trying to access the site as anon after turning on page cache.
Don't know enough about d8 to review the code though.

I failed to reproduce the message mentioned in #2471405: When not logged Drupal\Core\EventSubscriber\FinishResponseSubscriber->setExpiresNoCache() causes a PHP warning because of an invalid timezone even when evaluating the offending line directly:

$ php -r "DateTime::createFromFormat('j-M-Y H:i:s T', '19-Nov-1978 05:00:00 GMT');"

My environment:

$ php -v
PHP 5.6.9-0+deb8u1 (cli) (built: Jun  5 2015 11:03:27) 
Copyright (c) 1997-2015 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2015 Zend Technologies
    with Zend OPcache v7.0.4-dev, Copyright (c) 1999-2015, by Zend Technologies
    with Xdebug v2.2.5, Copyright (c) 2002-2014, by Derick Rethans
$ php -i | grep timezone
Default timezone => Europe/Berlin
date.timezone => no value => no value
$ php -i | grep error_reporting
error_reporting => 32767 => 32767
$ php -r 'echo(E_ALL);'
32767

mhhh, its tricky, given that we should not authenticate too early,
so what about running it after authenciation?

Is this Needs Review or Needs Work? It looks like the retest got stuck.

Uploaded patch:

• added route and controller in the system test module
• made sure to initialise the timezone when no account has been set explicitly in AuthenticationSubscriber instead of AccountProxy (from patch in #19)
• added an assertion in the test to ensure that this test date uses the default timezone
• fixed the NodeAttributesTest failing test

+      }
+      catch (\Exception $e) {
+        // Ignore exceptions that happen when the current user can't be
+        // accessed.
       }
     }

This means an exception may be thrown in subsequent lines if $context['user'] or $context['uid'] is referenced since they may not be defined.

To fix these tests we need to explicitly call date_default_timezone_set('Australia/Sydney'); in setUp(), so that the timezone is consistent while the installer runs. The proper fix is already discussed in #2611082: Inconsistent time zones in web tests issue, so the change in the patch #2 is the solution for both issues. Both changes are needed at the same time.

I'll close the other issue as we will now solve the bug here.

Patch.

There were other bits in the patch in #2611082: Inconsistent time zones in web tests that also need to be added. I'll add them over my lunch break and upload.

OK, just talked with @berdir on IRC, @tduong is going to add a few bit from the other issue in, and I'll do final review.

Added these few bit from the other issue in.

+++ b/core/lib/Drupal/Core/Logger/LoggerChannel.php
@@ -93,9 +93,15 @@ public function log($level, $message, array $context = array()) {
+      try {
+        if ($this->currentUser) {
+          $context['user'] = $this->currentUser;
+          $context['uid'] = $this->currentUser->id();
+        }
+      }
+      catch (\Exception $e) {
+        // Ignore exceptions that happen when the current user can't be
+        // accessed.
       }
     }

I think the code here can warrant a better comment as to who the try/catch is needed and expand a bit why the exception can be ignored.

Otherwise, I think the change is reasonable and the test coverage is adequate.

Some of the code is cherry-picked from one of my patches on #2611082: Inconsistent time zones in web tests (mainly the new TimeZoneTest), if anyone objects to me doing the final review.

Going to do some manual testing to double check this before I RTBC it.

Just ran some manual tests w/ a bad TZ in php.ini, and this does appear to fix the original issue reported in #2638678: date.timezone not being set in php.ini results in exceptions (which is mostly an untestable situation).

I'm not hung up on the comment for the try/catch I mentioned above, so I am setting RTBC, and a committer can the have final call on that.

1. +++ b/core/lib/Drupal/Core/Logger/LoggerChannel.php
   @@ -93,9 +93,15 @@ public function log($level, $message, array $context = array()) {
   +      try {
   +        if ($this->currentUser) {
   +          $context['user'] = $this->currentUser;
   +          $context['uid'] = $this->currentUser->id();
   +        }
   +      }
   +      catch (\Exception $e) {
   +        // Ignore exceptions that happen when the current user can't be
   +        // accessed.
          }
   

   This needs a comment - why is the if($this->currentUser) not working? Or to ask the other way around how do we get into the situation where $this->currentUser == TRUE but ->id() throws an exception?

2. +++ b/core/modules/simpletest/src/WebTestBase.php
   @@ -674,6 +674,15 @@ protected function drupalLogout() {
   +
   

   Unnecessary blank line

Updated exception comment and deleted unnecessary blank line.

Back to RTBC then.

Thanks for addressing my feedback. Committed 9970cf7 and pushed to 8.0.x and 8.1.x. Thanks!