[TYPO3-core] RFC #9355: Add an error and exception handler (backported from FLOW3)

[Ernesto Baschny [cron IT]]

Rupert Germann schrieb:

> here's v7 containing the following changes
> - removed the following files as they are not used/needed:
> class.t3lib_exception.php
> class.t3lib_error_error.php
> class.t3lib_error_exception.php
> class.t3lib_error_warning.php

I think this is not correct. The exception handling of TYPO3 should
provide the proper base classes for future use.

We need different Exception subclasses to be able to organize our
exceptions by category later. We need a "root" TYPO3 exception
(t3lib_exception) and a more specific exception that represents PHP
errors (t3lib_error_exception).

t3lib_error_error and t3lib_error_warning are probably there so that
other user-defined exceptions can subclass this. I wonder why these
don't subclass t3lib_exception? Using them, later the warnings
exceptions that "noone" catched could go to a log file, error-exceptions
go to the admin mail, etc.

> - functions handleException() in
> t3lib_error_DebugExceptionHandler and in
> t3lib_error_ProductionExceptionHandler are indentical, so I moved them
> to t3lib_error_abstractexceptionhandler. that replaces the original
> function whose output was never used.

I also did not unterstood it until I looked at FLOW3's implementation.
It was backported by Ingo and some stuff that we don't need
(systemLogging and lockManager) was stripped, so that handleException in
that backport doesn't make sense. See:

http://forge.typo3.org/repositories/entry/package-flow3/trunk/Classes/Error/AbstractExceptionHandler.php

I would propose even to add echoExceptionCLI and echoExceptionWeb to the
required methods in interface.t3lib_error_exceptionhandlerinterface.php
so that all implementations provide those two methods.

Either that or you have to provider those two functions in the Abstract
base-class too, else it will give out bugs if someone implements
t3lib_error_exceptionhandlerinterface and subclasses
t3lib_error_AbstractExceptionHandler but does not provide those two methods.

> - changed the default value to E_ALL ^ E_NOTICE ^ E_WARNING
> - added some error numbers to the field description of exceptionalErrors

Which ones? Where? Cannot find it by reading the diff from v4 to v7.

> - added a possibility to disable the exceptionhandler in BE, too. (by
> unsetting exceptionHandler)

This was already this way. You are now checking it twice (init.php):

if ($TYPO3_CONF_VARS['SC_OPTIONS']['errors']['exceptionHandler'] !== '') {
        if ($TYPO3_CONF_VARS['SC_OPTIONS']['errors']['exceptionHandler']
!== '') {

Same "if" nested. :)

> - moved the call to set_error_handler() from contructor to function
> setExceptionalErrors()
> - renamed function setExceptionalErrors() to
> setErrorHandlerForExceptionalErrors()

Ok, but I would keep setExceptionalErrors. This is already in the class
that is supposed to handle errors, this doesn't need to be announced in
the method name again. So I would opt for the shorter name (from FLOW3).

> - added some error numbers to fied description of exceptional errors

Again? :) where?

> - removed the FLOW paths in debugexceptionhandler
> - added errorlevel check in function handleError() to prevent it from
> outputting something on masked errors (@)

Yes, that is good.

Cheers,
Ernesto

> 
> 
> greets
> rupert
> 
> 
> Oliver Hader schrieb:
>> Great & thanks for your work on this.
>>
>> Here are some remarks:
>> * E_ALL&~(E_NOTICE|E_WARNING) -> E_ALL ^ E_NOTICE ^ E_WARNING
>> * E_ALL&~E_NOTICE -> E_ALL ^ E_NOTICE
>> * Unresolved backported FLOW3 issues:
>>   + t3lib_error_AbstractExceptionHandler defines with handleException()
>> but has no functionality - did I miss something
>>   + t3lib_error_DebugExceptionHandler::echoExceptionWeb() checks against
>> FLOW3's 'Packages/' in the file path
>>   + t3lib_error_DebugExceptionHandler::echoExceptionWeb() has a link to
>> typo3.org/go/exception/... that points to a FLOW3 message - I think we
>> should comment that out and add a "@todo" until we have own additional
>> messages there
>>
>> I suggest that we mark these unused FLOW3 parts with a proper "@todo"
>> and re-check them later on or remove them completely.
>>
>> Besides that:
>> +1 on reading and testing
>>
>> olly
>