Remove deprecation methods with trigger_error()


(Benni Mack) #1

Remove deprecation-related methods with trigger_error()

In PHP, there is trigger_error(), (see http://php.net/manual/en/function.trigger-error) which is intended to be used to trigger PHP errors / warnings etc. It can be called with the parameter E_USER_DEPRECATED (http://php.net/manual/en/errorfunc.constants.php) which is exactly there for deprecating user-generated framework code, like we do.

Hiding and showing the errors can be dealt with with our exception handlers, PHP’s system logs etc. and thus allows for methods to show deprecations even into Elastic Search by that (syslog).

I propose to replace our custom deprecation log logic completely with PHP’s internal method, and use that for 9.0.

Impact

  • We deprecate our deprecation methods in GeneralUtility (deprecationLog, getDeprecationLogFileName, logDeprecatedViewHelperAttribute, logDeprecatedFunction)
  • We remove the option $GLOBALS[‘TYPO3_CONF_VARS’][‘SYS’][‘enableDeprecationLog’]
  • We change all places we have already deprecated for v9 and use trigger_error() instead
  • We update our deprecation info in the contributors guide / coding guidelines

Pro

  • We can use PHP’s native error logging functionality, which is faster and offers more flexibility.
  • We don’t write log files into a public directory anymore by default

Con

  • We change rules that people understood and are comfortable with (using deprecation log)

Organizational

Topic Initiator: Benni Mack
Topic Mentor: Susi Moog


(Clemens Riccabona) #2

Nice approach! If there are standards or builtin functions we could use, I would vote nearly always for using them. This means in the end: we need less code which is pretty much the same as less errors, and most of the time also more performance.


(Claus Due) #3

I like this - and I don’t object to teaching people to use PHP’s deprecation logging instead. Besides, it puts the deprecation warnings in the same place for TYPO3 and all libraries. And yes, catching these can be dealt with through an error handler (but I still would vote +1 for not specially logging deprecations, i.e. catching these and putting them in a special deprecation log like we do now).


(Benni Mack) #4

For the sake of completeness I want to add some more details on it:

Question is: How would separate logging work then?

  • Some might have checked our ErrorHandler class available in TYPO3 Core, which was backported from TYPO3 Flow. This can be overridden by libraries or extensions or whatever by `TYPO3_CONF_VARS/SYS/errorHandler’.

A fairly easy task would be to extend the ErrorHandler class to be loaded which could even log into sys_log/beLog if people like that (I don’t ;)) but the same way could be used to implement the PSR-3 logging there and deal with that in this place. What I mean is: The ErrorHandler class (using PHP’s internal set_error_handler) allows to define exactly where to log a error/notice/warning/deprecation separately.

What I’m saying is: We’re not limiting ourselves to use our custom GeneralUtility anymore, but centralize deprecation logging to the same way we deal with other parts of logging as well, having one “we do it differently for this kind of logging” less.


(Philipp Gampe) #5

Sounds like a good reason to make the switch to trigger error. If we get a proper, configurable logging and separation of errors, warnings and deprecations out of this change, then we should definitely go for it :wink:


(Jigal Van Hemert) #6

Sounds good. There is one limitation we have to consider and that is the 1024 char limit of the message in trigger_error(). A quick look in one deprecation log showed already lines of 1759 chars… This happens because we include a trace in the deprecation log. But this is not something that prevents us from using this function.


(Alexander Opitz) #7

The trace in the message won’t be in the messages anymore. The traces will be handled by the error handler then.


(Alexander Opitz) #8

Eventually we should implement a base extension, which lets the integrator select which error type will be written where. This should handle most cases. If someone needs more, then he can implement this in his own extension.
This base extension would also be the documentation/reference implementation how we think this should work.


(Georg Ringer) #9

nice idea to get rid of custom things and to use common code


(Benni Mack) #10

This topic was automatically closed after 13 days. New replies are no longer allowed.