Download & Extend

Performance regression: Drupal overriding error_reporting setting in php.ini

Project:Drupal core
Version:8.x-dev
Component:base system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work
Issue tags:needs backport to D7

Issue Summary

In #291026: change E_NOTICE to warning and allow selection of error level, webchick requested that this be made a new issue.

As it happens, Drupal 7 includes a rather unfortunate change, that forces PHPs error_reporting setting to E_ALL or higher, overriding the system configuration.

This incurs a significant performance overhead for error handling whenever a notice occurs, as well as a lot of useless noise in Drupal's watchdog log (and even on screen if error logging to screen has not been disabled), and given that these are mostly coding style issues and not indicative of any real problem.

Drupal should not change system configuration unnecessarily in ways that requires hacking core to undo. This is probably a well-intentioned attempt at getting developers to fix notices in their own modules. However, this is the wrong approach. System settings are not for user-space applications to thumb their nose at, and as a system administrator, I was appalled to learn that Drupal thinks it knows better how to configure my server.

If anything, we could introduce error_reporting as one of the settings in default.settings.php, but for now, the attached patch removes the line in question from bootstrap.inc.

AttachmentSizeStatusTest resultOperations
2011-09-02-leave-error_reporting-alone.patch660 bytesIdlePASSED: [[SimpleTest]]: [MySQL] 35,928 pass(es).View details | Re-test

Comments

#1

Status:active» needs review

This should be mergeable as is…

#2

Sub (I need to know more about this stuff in any case)

#3

Subscribe.

#4

I was started a similar issue #1138464: Error reporting slow down drupal

#5

Would someone please commit this !

this is the wrong approach. System settings are not for user-space applications to thumb their nose at, and as a system administrator, I was appalled to learn that Drupal thinks it knows better how to configure my server.

Goes double for me.

f-ing annoying as hell to not be able to change this in the proper place,
Sam

#6

Has this not yet been committed? It's a pain having to go in and fix this (let alone remember to) manually every time.

#7

Status:needs review» reviewed & tested by the community

edeloa,

Maybe we're not using the issue queue correctly?

Changed status to tested and reviewed.

#8

Hmmm, I think it would be preferable to add it to default.settings.php. Removing the forced setting might be appropriate, though; let's see if I can find the original issue where this was added...
Edit: See #348448: Always report E_STRICT errors. We'd be reverting that issue, so would need to justify that, or at a minimum bump the error reporting during simpletest execution.

#9

Version:7.x-dev» 8.x-dev
Status:reviewed & tested by the community» needs work

Also, this should go into D8 first. Thanks!

#10

For those looking for a quick fix to this, just add ini_set('error_reporting', E_ERROR); to the top of one of your module files. No hacking core required.

#11

Hi Jess,

I can't believe that the original intent is to force, against a user's will and server settings, anything. Much less something that near cripples a large module site.

Should we re-open #348448: Always report E_STRICT errors as a bugged resolution to that problem?

Best,
Sam

PS: Thanks Nick, that's a bit easier, safer, and more permanent.