bgarrels / textpattern

Automatically exported from code.google.com/p/textpattern
0 stars 0 forks source link

Error handling causing errors #373

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
There are few slight issues with the error handler functions:

* All handlers will cause error on their own if they catch an error number that 
is not in the $error list.
* Due to differences in error types between PHP version, the admin error 
handler kills script responses without reason.

The biggest problem we have is E_STRICT. Textpattern does invoke those on 
certain PHP version. For instance extract will invoke E_STRICTS on certain PHP 
releases due to the function, at one point, only accepting variables due to it 
'defaulting' to referencing.

In which case the there will be E_STRICT, error code 2048. Which isn't in the 
table which in return invokes an E_NOTICE about undefined index.

Outside of that actual programming error, this wouldn't be a big issue because 
'live' production status. But. There is adminErrorHandler which ignores 
production status. On script response pages it does stuff no matter what the 
status is. An if the page happens to be set as 'text' (like async links) the 
page is killed and returned as HTTP 500.

Currently on PHP 5.2.17 it will actually even kills almost all async responses. 
Almost every page generates E_STRICT on that PHP version.

Todo;

* Change the error handlers to work entirely based on the production status. 
Each status has list of explicit error codes it will show. This prevents the 
differences in visible errors between different version and prevents errors on 
error codes that are not supported.
* Make sure that the adminErrorHandler kills script response pages only when it 
needs to. Especially makes sure it does not do anything when live mode is 
active, or there is no message to print out.

As a sidenote; error reporting on PHP 5.4 works fine and doesn't cause 
unexpected issues.

Original issue reported on code.google.com by jukka.m.svahn on 15 Mar 2013 at 12:22

GoogleCodeExporter commented 9 years ago
Just out of curiosity, any reason you can think of for having underscores in 
some of the $error array values (in pluginErrorHandler, for example)? They 
don't appear to be subject to gTxt().

Perhaps they should all be l10n-ised and be run through gTxt() before display? 
Worst case scenario is people would see 'user_warning' as opposed to 'User 
warning' whenever $textarray wasn't loaded.

Also, should the inline style attributes be removed in favour of putting the 
rules in the CSS file?

Original comment by stefdawson on 15 Mar 2013 at 1:03

GoogleCodeExporter commented 9 years ago
There is no reason for the underscores, I suppose. As gTxt() and error handers 
go, good idea, but error handlers are extremely delicate. It's normally the 
best practice to avoid adding dependencies to error handlers. The last thing 
you want to do is to invoke errors in an error handler.

For instance language loading bugs out, which then creates a infinite loop of 
errors. DB error after a DB error.

Translations and all extra cool processing is fine on very specific error 
handlers, like the tag markup ones, which basically are used almost like very 
specific exceptions. Those are just for markup tags, and the changes of 
something breaking are smaller. But when the error handler covers more base, it 
starts getting more and more dangerous.

As any changes go, could do if the Google's SVN servers worked. It's been up 
and down due to traffic, some say. Ugh, SVN. Why u no git.

Original comment by jukka.m.svahn on 15 Mar 2013 at 1:31

GoogleCodeExporter commented 9 years ago
Yeah, I did wonder about introducing dependencies. Doing without gTxt() under 
any "exceptional" circumstances (i.e. any error) is fine by me; although note 
that tagErrorHandler() uses gTxt() for the 'while_parsing_page_form' snippet 
already.

And yes, I've been playing Google Code yoyo this afternoon too. Grrr.

Original comment by stefdawson on 15 Mar 2013 at 2:10

GoogleCodeExporter commented 9 years ago
Ugh, no can edit comment. I meant to say "although I note that..."

Not that it matters much because you already mentioned it and I didn't read 
properly. New glasses / brain on their way to me right now.

Original comment by stefdawson on 15 Mar 2013 at 2:39

GoogleCodeExporter commented 9 years ago
Heh, can't blame anyone for not getting a stomach ache from ingesting my 
Engrish.

Original comment by jukka.m.svahn on 15 Mar 2013 at 3:32

GoogleCodeExporter commented 9 years ago
See http://forum.textpattern.com/viewtopic.php?id=39505, second part re: error 
suppression in list_list().

Original comment by r.wetzlmayr on 17 Mar 2013 at 7:13

GoogleCodeExporter commented 9 years ago
On Windows, the new error handlers cause a 'Warning "Invalid CRT parameters 
detected" in .../txp_prefs.php at line 504.'

Reason: We use strftime() with a '%e' format specifier that does not exist in 
Windows environments and used to catch these disparities with '@' error 
suppression.

Original comment by r.wetzlmayr on 17 Mar 2013 at 7:31

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Thanks Robert. r5435 restores suppression support. Just seemingly forgot the 
check, eh.

Original comment by jukka.m.svahn on 17 Mar 2013 at 9:24