Post A Reply

plg: 2011-02-04 11:30:29

rvelices wrote:
Also note that fatal errors will not be displayed anymore either. So we'll end up with many 'blank pages'

I 100% agree that a blank page is worse displaying an error.

At visitor level, it would be better to have a nice "An error has occured" page (like the "Page not found" we have).

plg: 2011-02-04 11:11:34

rvelices, in your opinion, would it make sense to have:

Code:

if (is_admin())
{
  @ini_set('display_errors', true);
}
else
{
  @ini_set('display_errors', false);
}

(or written more compact)

Code:

@ini_set('display_errors', is_admin());

rvelices: 2011-02-04 10:34:39

Also note that fatal errors will not be displayed anymore either. So we'll end up with many 'blank pages'

rvelices: 2011-02-04 10:33:06

I'm OK with the code, but I have a doubt about releasing with display_errors=false.
User lambda will not have the reflex nor the knowledge to look into the error log (supposing he knows how to find it). So he will not come here and let us know about the problem.
Also in case something is wrong, he will come here for help and in addition to usual questions (what plugins do you have?, what versions, etc...) we will have to ask for the error log.

plg: 2011-02-03 15:07:25

You're right griou.

Currently, Piwigo does:

Code:

if(isset($conf['show_php_errors']) && !empty($conf['show_php_errors']))
{
  @ini_set('error_reporting', $conf['show_php_errors']);
  @ini_set('display_errors', true);
}

For a production website, I think we should do (without the "if" condition):

Code:

@ini_set('error_reporting', $conf['php_error_reporting']);
@ini_set('display_errors', false);
@ini_set('log_errors', true);

that means:

1) replace $conf['show_php_errors'] by $conf['php_error_reporting'] (because "show_php_errors" is confusing)

2) add $conf['php_display_errors']

3) add $conf['php_log_errors'] (errors are logged into Apache error.log)

and have the following code in include/common.inc.php:

Code:

@ini_set('error_reporting', $conf['php_error_reporting']);
@ini_set('display_errors', $conf['php_display_errors']);
@ini_set('log_errors', $conf['php_log_errors']);

and in include/config_default.inc.php:

Code:

$conf['php_error_reporting'] = E_ALL & ~E_DEPRECATED;
$conf['php_display_errors'] = false;
$conf['php_log_errors'] = true;

griou: 2011-01-31 11:47:04

Hi guys,

I think the level is not really the problem here , it's mostly the fact that errors are displayed. I understand it makes it easy for you guys to fix some user's problem .

For me errors shouldn't be displayed but they still have to be reported and logged. I don't know exactly how piwigo works, but if one uses the following config :

Code:

$conf['show_php_errors'] = "";

Does it means that error are not reported at all ?

Cheers,

Gaetan

plg: 2011-01-27 10:53:49

What does my php.ini file says:

; error_reporting
; Default Value: E_ALL & ~E_NOTICE
; Development Value: E_ALL | E_STRICT
; Production Value: E_ALL & ~E_DEPRECATED

rvelices: 2011-01-27 10:28:38

I would say we put E_ALL for RC versions, but not for final releases ?
Anyway I'm fine with any decision.

plg: 2011-01-27 09:19:21

Hi Griou,

Yes, you may be right. I admit it helps a lot when a user encounter an error, he can show us the error.

By default:

Code:

$conf['show_php_errors'] = E_ALL;

and maybe we should decrease this level.

Griou: 2011-01-26 12:18:01

Hi,

I was quite surprised to see that piwigo displays error by default. I think should be disable by default, it'll avoid unexperimented/careless user to have a live piwigo installation that could potentially show error and dangerous informations.

Cheers,

Gaetan

Piwigo.org

Announcement

Topic review (newest first)

rvelices wrote:

Code:

Code:

Code:

Code:

Code:

Code:

Code:

Code:

Board footer