[#9270] - Error handling adjustments (Fix #9074)
- Closed
- 5 Mar 2016
- Medium
- Build: staging
- # 9270
- Diff
- mbabker:Error-Handler
- Success Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #9074.
Summary of Changes
-
JErrorPage::render()did not close the application correctly when rendering error pages - The
$evariable may not be defined inJErrorPage::render()when echoing a static text message instead of rendering aJDocumentErrorobject -
PlgSystemRedirect::handleError()cannot be used as both aJErrorcallback handler and a global Exception handler due to signature differences, namely passing references versus values; a newPlgSystemRedirect::handleException()method is added to be used as the global Exception handler and the actual error handling code is moved into a private method in the plugin - General
JErrorPageTestimprovements
Testing Instructions
With the redirect plugin enabled, 404's should still be logged correctly.
To test the Exception handler pre- and post- patch, after this line in JApplicationCms throw an Exception. If running PHP 7, pre-patch you'll get this warning and an uncaught Exception:
Warning: Parameter 1 to PlgSystemRedirect::handleError() expected to be a reference, value given in Unknown on line 0
Post patch, the error page should render correctly. PHP 5 is unaffected.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
?
|
||
| Rel_Number | 0 | ⇒ | 9074 |
| Relation Type | ⇒ | Pull Request for | |
| Labels | |||
| Category | ⇒ | Libraries |
No. PHP 5 should be tested as well mainly to make sure nothing changes.
I have tested this item 
successfully on 45a8bc2
Using php 5.6.x. Applied the patch and activated redirect plugin and trow a custom error after the line you pointed.
throw new RuntimeException('test', 404);Tested the frontend and all URI return a 404 and, after removing the line, i checked they appeared in the redirect component list view.
Side note not related to this PR itself but with error pages: i have the debug plugin activated and the debug console does not appear in error pages, also there are some issues with mod_languages (the mod_languages CSS is not loaded in error pages).
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9270.
Both known issues.
1) The debug plugin relies on plugin events that won't trigger on the error page to render it.
2) JDocumentError doesn't have the same rendering capabilities as JDocumentHtml, namely the <jdoc> parsers aren't there. Frankly the whole error page rendering system needs to go back to the drawing board because there are more document types than HTML in use today.
ok. understood. thanks for the explanation.
@wilsonge This needs to be tested & merged before stable. A combination of the redirect plugin being enabled plus an uncaught Throwable on a PHP 7 environment can result in a WSOD or PHP Fatal Error (depending on error reporting settings) when an error page should be rendered.
| Labels |
Added:
?
|
||
I have tested this item successfully on 45a8bc2
Test with PHP 5.6: No change to behavior without patch => success.
Test with PHP 7: Before patch, error page was not redered, after patch it is => success.
New redirect records are added on both PHP versions with exception + this patch => success.
Used same code for generating exception as andrepereiradasilva did.
Sorry it took me a while to setup a PHP 7 environment on my shared host, so I tested not earlier.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9270.
| Status | Pending | ⇒ | Ready to Commit |
| Labels | |||
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-03-05 23:44:04 |
| Closed_By | ⇒ | wilsonge |
| Milestone |
Added: |
||
| Labels |
Removed:
?
|
||
Merged - thanks Michael!
| Labels |
Removed:
?
|
||
if i understand correctly, this should be tested only with php 7, right?