If a site has been configured to display PHP notice/warning/error messages in the logs only (not on the screen), Drupal should respect that setting. But currently it ignores that on update.php and always displays them on the screen.

This can cause confusion for people who are not used to seeing such messages in the admin UI but then suddenly see them when they update to a new Drupal version and run update.php. I suspect (but am not sure) that this kind of confusion is evident in some of the comments at https://www.drupal.org/node/2487215.

Before I looked through Git history to figure out when/why this behavior was originally added, my guess was that it was probably added somewhere around 2004 and the likely reason was that update.php wasn't capable of sending messages to the logs at that time. I wasn't 100% correct, but pretty close :) #54003: Watchdog not logging update errors

This behavior is no longer needed, so it should just be removed.

Membership dollars fund testing for the Drupal project. Drupal Association Learn more

Comments

David_Rothstein created an issue. See original summary.

David_Rothstein’s picture

Status: Active » Needs review
Issue tags: +needs backport to D7
FileSize
1.18 KB

Here is a patch - untested, but pretty simple.

The Drupal 7 version of this can just remove the MAINTENANCE_MODE check entirely, but since Drupal 8 is also ensuring that messages are always displayed to the screen during install.php, I guess we should not alter that part of the behavior here.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

aerozeppelin’s picture

Fix for D7 as per the comments in #2.

Status: Needs review » Needs work

The last submitted patch, 4: 2767663-4.patch, failed testing.

David_Rothstein’s picture

Status: Needs work » Needs review
daffie’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

I think that this patch needs some tests.

aerozeppelin’s picture

Added a failing test. Looking for someone to take it forward.

Status: Needs review » Needs work

The last submitted patch, 8: 2767663-8.patch, failed testing.

The last submitted patch, 8: test-only-fail-2767663-8.patch, failed testing.

aerozeppelin’s picture

Status: Needs work » Needs review
FileSize
1001 bytes
2.04 KB

Fix for failing tests on #8.

Status: Needs review » Needs work

The last submitted patch, 11: 2767663-11.patch, failed testing.

daffie’s picture

Quick review:

+++ b/modules/simpletest/tests/upgrade/upgrade.test
@@ -491,6 +491,24 @@ class BasicStandardUpdatePath extends UpdatePathTestCase {
+    module_enable(array('poll'));

I do not think we have a module called poll in D8 core any more.

aerozeppelin’s picture

The above patch was meant for D7 :D

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.