I'm inclined to say this PR is the wrong approach (not to mention breaks the tests because the code added is done before validating we have a proper error object).

If an error is logged to PHP's error handler before calling JFactory::getApplication() for the first but not after that call, then there is something in our application boot process mucking with the error reporting that should be fixed. A trigger_error() call should not be necessary. Also consider that the first JFactory::getApplication() call happens after the framework has been bootstrapped, which includes an error_reporting() call and in any case where our error reporting configuration isn't set to "none", ini_set('display_errors', 1); is called as well.

Additionally, basing this solely on whether there is a 0/null/empty code isn't good logic either. We have a lot of Exceptions that don't set codes, and Exception's constructor defaults the code to a 0/null/empty value. So you will cause trigger_error() to be called for a vast majority of Exception objects versus fixing whatever it is you're actually trying to fix.

@mbabker

We can use a 500 code in the constructor as default. Regarding the Exceptions without codes, I do not know: may be we can add codes to the Exceptions (500, for instance).

I tested it when the error reporting is set to "Simple" -- it does not show any errors.

We can use a 500 code in the constructor as default. Regarding the Exceptions without codes, I do not know: may be we can add codes to the Exceptions (500, for instance).

We can't guarantee every Exception will always have a code (third party libraries included with core, extension developers, etc.). So the behavior must be based on other logic.

I tested it when the error reporting is set to "Simple" -- it does not show any errors.

If you're referring to my comment about the broken tests, nothing in the CMS by default calls JErrorPage::render() without a correct object. But, the signature is untyped and is a public static method, so in theory it can be called with anything, and it accounts for PHP 7 adding an interface on top of the base Exception class so it can't be typehinted again until PHP 7 is the minimum supported version. So you have to put any code that is dependent on the $error parameter being a Exception/Throwable object into the $isException if conditional.

If you can really get something logged to the PHP error logs before the framework is initialized versus after (basically before/after includes/framework.php is loaded), then the steps taken to initialize the framework must be reviewed to determine the cause. There are only three possibilities:

• us registering a custom Exception handler through set_exception_handler(), which would correctly cause Exception/Throwable objects to not be logged because we're catching and handling those at our level, this should not affect non-Exception/Throwable based errors though and if it is there is a bug in the core PHP engine
• us calling error_reporting() to change the error reporting level based on the CMS user configuration
• us calling ini_set('display_errors', 1) to change whether errors are displayed based on the CMS user configuration

Ok, I understand!

My logic is simple -- if the exception is without the code -- it is not supposed to be rendered, on the contrary, it is supposed to be logged in the server log. If you need to render the error message, use the code in the Exception constructor.

But I do not insist on my solution and hope that someone will find another, better solution. This PR as an option to be considered, no more.

@mbabker

I changed this PR. not only the Throwable interface we have in PHP7, but also we have the Error class http://php.net/manual/en/class.error.php. The Exceptions are not belong to the Error class, but the Error class belongs to the Throwable -- that is why we have a problem. Please, recheck my PR and write your conclusion.

Also I removed the trigger_error call. Because it is not required.

This basically makes us lose error page handling for Error objects. Can't say I'm a fan of that personally.

We have a JLog call in the internal error handling. As long as the error isn't before the onAfterInitialise event, you could set up a JLog logger through a plugin to do extra logging, including trigger_error() or error_log() calls.

So I still don't think this PR is actually improving things much. In some ways it worsens things because we stop handling PHP 7 Errors and go back to only handling the Exception class chain.

@mbabker

I think, JLog is not intended to log the following errors:

• TypeError
• ParseError
• AssertionError
• ArithmeticError
• DivisionByZeroError

All of them belongs to the Error class.

I cannot imagine who wants to have this errors rendered with the Joomla! error page renderer. This is also a security issue. JLog is for authorisation purposes, deprecated items etc, but Joomla! should not manage internal server errors.

Can you, please, explain. What is the practical usage of throwing the Error objects and render it with the Joomla! error page handler?

It's a side effect of PHP 7 converting errors to Throwable objects. IMO
there is no issue with our logs recording this data, other frameworks do it
and I have seen zero issue reports from users of those platforms (and in
some ways it's better than writing to an error_log file that's usually web
accessible on web hosts as JLog adds some protection to its written files).

On Sat, May 13, 2017 at 8:27 AM Philip Sorokin notifications@github.com
wrote:

@mbabker https://github.com/mbabker

I think, JLog is not intended to log the following errors:

• TypeError
• ParseError
• AssertionError
• ArithmeticError
• DivisionByZeroError

I cannot imagine who wants to have this errors rendered with the Error
page rendered. This is also a security issue. JLog is for authorisation
purposes, deprecated items etc, but Joomla! should not manage internal
server errors.

Can you, please, explain. What is the practical usage of throwing the
Error objects and render it with the Error page handler?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#15994 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoZmRl6cq6klzpJ_Bb1IAZceMdrh_ks5r5a-1gaJpZM4NZ6eN
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

@mbabker

I wrote the final variant of this PR that now targets a specific error class. Please, recheck it. We still can render Error objects except for the following internal server errors:

• TypeError
• ParseError
• AssertionError
• ArithmeticError
• DivisionByZeroError

The problem with JLog is that it does not contain fatal errors -- they are not written, as if they do not exist. It is very difficult to track such errors: you have no file, no line number, and the JLog, as well as the server log is clear. The exclusion of above mentioned error types from Joomla! error handler resolves this issue.

By default there isn't anything logging the error category that is used
when we try to log the error with JLog. That's why there's no recorded
message.

I don't see a benefit to removing support for PHP 7 Error objects from our
handler, but if people really feel that strongly about us not handling what
equates to an uncaught Exception then so be it.

On Sat, May 13, 2017 at 9:25 AM Philip Sorokin notifications@github.com
wrote:

@mbabker https://github.com/mbabker

I wrote the final variant of this PR that now targets a specific error
class. Please, recheck it. We still can render Error objects except for the
following internal server errors:

• TypeError
• ParseError
• AssertionError
• ArithmeticError
• DivisionByZeroError

The problem with JLog is that it does not contain fatal errors -- they
are not written, as if they do not exist. It is very difficult to track
such errors: you have no file, no line number, and the JLog, as well as the
server log is clear. The exclusion of above mentioned error types from
Joomla! error handler resolves this issue.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#15994 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXM__yV7TMCWrU0EN0UomW778sjnks5r5b1zgaJpZM4NZ6eN
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

I don't see a benefit to removing support for PHP 7 Error objects from our
handler

The last variant of this PR removes only internal errors, but we can still use extended Error classes. I do not know, who can use extended error classes, but it is possible with this PR.

PHP7 is only a year old, before, in PHP5 the Errors logged to the server log. Why change this behaviour?

We didn't change the behavior. PHP core did by converting errors to a
catchable object that can be handled through an Exception handler, we
adapted our handler to catch these objects. That's the extent of it, Error
objects are now caught the same way an Exception is. And IMO this is a
better behavior than letting it go unhandled resulting in a WSOD or vague
message to a user.

On Sat, May 13, 2017 at 9:44 AM Philip Sorokin notifications@github.com
wrote:

I don't see a benefit to removing support for PHP 7 Error objects from our
handler

The last variant of this PR removes only internal errors, but we can still
use extended Error classes. I do not know, who can use extended error
classes, but it is possible with this PR.

PHP7 is only a year old, before, in PHP5 the Errors logged to the server
log. Why change this behaviour?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#15994 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoYN57RmL6yEzVq7rXZdMC8QwO43aks5r5cHagaJpZM4NZ6eN
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

The behaviour was changed, because when using PHP5 with Joomla! fatal errors are not rendered: only white screen shows to a user. But in PHP7 fatal errors become error 0, expands to everyone, and not even logged, I cannot track the parse and syntax error, as well as other fatal errors without swithing off the Joomla! error handler. I can write a plugin for my usage, but what should others do? They see error 0 message and do not know what to do.

We didn't make the change though. PHP did. And our handling of PHP's B/C
change is IMO fine.

We can add a logger to our bootstrap to ensure that everything is written
to our log files (which IIRC already includes a stack trace and the error
message). But I don't think we should make our handler only handle
Exceptions and ignore Errors.

On Sat, May 13, 2017 at 10:28 AM Philip Sorokin notifications@github.com
wrote:

The behaviour was changed, because when using PHP5 with Joomla! fatal
errors are not rendered: only white screen shows to a user. But in PHP7
fatal errors become error 0, expands to everyone, and not even logged,
I cannot track the parse and syntax error, as well as other fatal errors
without swithing off the Joomla! error handler. I can write a plugin for my
usage, but what should others do? They see error 0 message and do not
know what to do.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#15994 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfobyyAHmRQMOkoS5P9jcza3k6Lfzuks5r5cwhgaJpZM4NZ6eN
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

I think, not all types of Error objects should be handeled by CMS. CMS is CMS -- server is server, PHP is PHP... their internal (fatal) errors must be logged in the according logs. But if you want to pass Error objects to Joomla! handler, you can do the following:

class MyOddError extends Error {}
throw new MyOddError();

Then convince the entire PHP ecosystem they're handling things wrong. No application or framework that I've checked or work with (Drupal, Symfony, Laravel, Concrete5, WordPress) ignores PHP 7 Error objects in their Exception handler. We aren't either. To me the behavior we have now is the correct behavior.

Your issue is the errors aren't logged. I've provided advice on possible solutions for this (we add a logger in our bootstrap to ensure they get logged or a user plugin). I don't see the fix for this issue as "we change our error handling to ignore a specific object type".

Then convince the entire PHP ecosystem they're handling things wrong

In my opinion, PHP provides various methods to achieve the result you need. There is no a single conception, and any conception can be reconsidered. Not only we need to log fatal errors, but we need to log fatal errors in PHP or/and SERVER logs. And we also need to not expand the fatal errors to everyone with renderer (sorry for my English). Why do we need server logs, if we have CMS logs? May be the PHP ecosystem should remove their server logs and read the notices, warnings, errors in CMS/framework logs? And if fatal errors should not be rendered, why do we need to throw them in the CMS error handler?

With PHP 7 converting warnings/errors to Error objects, it actually improves both the user and developer experience IMO. With PHP 5, those conditions in an adequately configured environment would more often than not result in a white screen of death. In PHP 7, those Error objects can now be processed through your callback defined through set_exception_handler(). For most applications, that results in an attempt to log the error to the application log handler (which may or may not be configured to also log to PHP's error log) and attempts to render a useful error page (in the case of our default CMS templates that's just echoing Throwable::getMessage(), other templates may create their own error page layouts), or you could do all sorts of other things by decorating the Exception handler we set by default (so you could send a notification to a sysadmin if need be). With platforms like Symfony and Laravel, they also go an extra step to try and show some more useful debugging information (Symfony's great at pointing out that you're missing a use statement for namespaced class imports, or suggesting a method to use in the case of an undefined method call). Handling these Error objects at the application layer instead of relying on PHP internals to do so is not a bad thing.

Like I said, the one thing we are missing is the fact we don't have a default logger wired up for our Exception handler, so the JLog::add() call that happens by default does nothing useful. If we changed that, what would the issue be at that point, aside from the fact that the error message is written to JPATH_ROOT . '/administrator/logs/error.php' versus /var/log/apache2/error.log and using a different file/message format?

By now, Joomla! cannot proccess errors gracefully in Symfony way. I suggest introducing default behaviour that can be extended with non-standard features. The default (basic) behaviour is the dump white screen that makes the developer life much easier, because the errors will be tracked.

Another features can be added as extensions or addons. Users would enable the features by themselves in the admin panel, for Instance. Moreover, third-party developers can write different error handlers to proccess errors, and users can use special plugins. But now Joomla! cannot proccess errors at all.

CMS is CMS -- server is server, PHP is PHP... their internal (fatal) errors must be logged in the according logs.

There is a mistake in your argumentation. The error comes from the CMS, not from PHP and not from the server. Because the CMS (or one of the extension) has faulty code. Thus it's absolutely fine that this error is also handled by the CMS as long as it is possible. It's no different to other errors which happen through the runtime of the CMS. It's an exception that is thrown and should be caught and handled.
We can discuss about how it should be handled for sure (eg don't show the detailed message to the user. Show a generic error and log it instead), but we certainly should handle it.
Anything is better than just a white screen of dead.

Anything is better than just a white screen of dead.

White screen can be used in development purposes. When you enable error reporting, you can see errors on the white screen. But when error reporting is off and you set a header header('HTTP/1.1 500 Internal Server Error'); (what I suggest for specific Error objects) there is no white screen -- in the browser users can see this error, and we forgot about search engines. Error 0 and the server response 200 is not a desirable response for SEO.

As I said, we can add in the future a beutiful error handler, that can be enabled by the user in the admin panel. The practical usage of beautiful screen instead of white screen with fatal errors is debatable. If you have fatal error, your site is dead, and you dress it up in beautiful funeral suit.

Not setting the right HTTP status code doesn't mean we have to change the
types of objects supported by the error handler, it means we fix the status
header handling.

And yes, from an end user perspective, a "beautiful funeral suit" is
preferred, it communicates something to the user. That alone is better
than a WSOD which communicates nothing. The aim at all times should be to
tell the user something.

On Sat, May 13, 2017 at 12:18 PM Philip Sorokin notifications@github.com
wrote:

Anything is better than just a white screen of dead.

White screen can be used in development purposes. When you enable error
reporting, you can see errors on the white screen. But when error reporting
is off and you set a header header('HTTP/1.1 500 Internal Server Error');
(what I suggest for specific Error objects) there is no white screen -- in
the browser users can see this error, and we forgot about search engines.
Error 0 and the server response 200 is not a desirable response for SEO.

As I said, we can add in the future a beutiful error handler, that can be
enabled by the user in the admin panel. The practical usage of beautiful
screen instead of white screen with fatal errors is debatable. If you have
fatal error, your site is dead, and you dress it up in beautiful funeral
suit.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#15994 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoXYJAne9ikC5fuMuD2NbopmEMbseks5r5eXUgaJpZM4NZ6eN
.

--

• Michael Please pardon any errors, this message was sent from my iPhone.

By the way, we also forgot about server defined error pages, e.g. in .htaccess you can write:

ErrorDocument 500 /err404.html

The same in Nginx and other servers. But it is only possible when you are using standard php error handler for fatal errors.

Those server error pages are usually worse than anything Joomla can offer. And of course Joomla error pages can do a lot more (and are easier to customise) than what those server error pages can do.
That's no argument.

Honestly. PHP7 is changing the way those errors will be handled, which is a good thing.
Developers will adapt to it, and it's no problem to do that.

Developers will adapt to it, and it's no problem to do that.

Right, in fact, I am developing a component now and turned off Joomla! error handler, In future, I will replace it with a plugin that will throw Errors correctly, with right response codes for search engines and user-friendly 404 -- the only page I need to be user-friendly.

But I would ask you to add a default error handler in Joomla! That would simply work. As for beautiful fatal errors, if they not procecced properly, in my opinion, the handler must be rewritten.

The behavior is expected for PHP 7+, that what for a fatal errors was replaced by an exceptions. To be able to catch the errors on the application level. And everyone asked for it, for years.
I do not see a reason of discussion , really.

The discussion is about making error handler work. In future, it can be extended to proccess fatal errors gracefully, but now it is not working completely. I am not against the application error proccessing, but also want to have some options to select, and to track the errors without switching off the Joomla! handler.

I'm very much in favour of implementation of a real error handler with logging in Joomla.

Imagine you developed something, tested it locally and it doesn't work in some hoster's environment. At other hosters it works. Sad enough that at one of largest (the largest in between ,,,) in Germany in normal hosting packets you do not have access to error log.

I use some homebrewn solution to get around those issues but, it would be much better to have a reliable error handling system within CMS.

Just my few Cents about discussion about to do or not.

cu

@dirigit,

Shells and malware would read and write your error logs to hide itself. I insist on the fact, that having error logs in the public_html folder is not secure. But you are right -- it can be useful in development; however, it is dangerous in production. And I suggest Joomla! does not change a server security policy.

@
You are absolutely right. Therefore it's a wise decision to have tmp and log directory outside of public html at least.

Much more dangerous is kind of headless access to backend which can not be blocked with htaccess or the like. This has been possible some months ago with Joomla. That's reason why I'm no friend of hardcoded paths. Being able to set them individual is much more secure.

@dirigit, It does not matter whether the log is in the public_html or not. I mean, the log must be out of the scope of the local PHP user. The integrity implies that logs are written by root or a sudoer, which is possible only if the logs are written by the server/php, not CMS.

i am closing this because of the comments by @mbabker above