Deprecating trigger_error calls in the Tarot cards?

Re: Deprecating trigger_error calls in the Tarot cards?

Ambush Commander — Mon, 11 Apr 2011 15:05:16 -0400

I'll accept a patch that converts trigger_errors that are E_USER_ERROR can be converted to exceptions with no configuration flag. A patch that converts E_USER_WARNING trigger_errors to exceptions would be more interesting... in particular, a configuration flag won't necessarily work because a lot of this errors /come/ from the configuration handling system.

Re: Deprecating trigger_error calls in the Tarot cards?

Yzmir Ramirez — Mon, 11 Apr 2011 14:48:36 -0400

Just to be clear, would you be willing to consider a patch that converted all the trigger_error() calls to exceptions with a Configuration flag (default false so as to not change existing functionality)? Or would you think that would clutter up the code?

Re: Deprecating trigger_error calls in the Tarot cards?

Yzmir Ramirez — Fri, 08 Apr 2011 17:21:31 -0400

Thank you.

Re: Deprecating trigger_error calls in the Tarot cards?

Ambush Commander — Fri, 08 Apr 2011 17:20:31 -0400

Oh yes, iconv is another bastard w.r.t. errors. You can further assume that HTML Purifier will not throw exceptions on bad HTML. It will just do its best to clean it up :-)

Re: Deprecating trigger_error calls in the Tarot cards?

Yzmir Ramirez — Fri, 08 Apr 2011 17:00:43 -0400

Yeah I saw you guys doing that. You also do that around iconv. You did that because even when using the @ character to surpress the error messages the error handler function is still called. And then another you did to surpress the E_STRICT messages.

Very nice solutions.

Ok so for now I will go with the assumption that really crappy HTML will not generate trigger_errors(), but instead throw an Exception that can be caught.

Re: Deprecating trigger_error calls in the Tarot cards?

Ambush Commander — Fri, 08 Apr 2011 16:57:01 -0400

Bad HTML should never trigger a PHP warning. If it does, that is a definitive bug. (One caveat is that the DOM parser can emit errors; not our fault, but we overload the error handler temporarily to deal with this case.)

Re: Deprecating trigger_error calls in the Tarot cards?

Yzmir Ramirez — Fri, 08 Apr 2011 16:40:08 -0400

Hmm...what I'm trying to do is capture the bad HTML could not be handled for whatever reason. If it would be enough just to catch exceptions when in a production environment then that'll be golden.

I'm just concerned that when this is in a production environment that when there is some crazy-ass HTML passed that I couldn't log it and adjust the settings/filters accordingly.

Re: Deprecating trigger_error calls in the Tarot cards?

Ambush Commander — Fri, 08 Apr 2011 16:33:24 -0400

Some of the trigger errors are warnings intended to let developers know that they need to update their code. An exception would not be appropriate in those circumstances.

Clearly fatal errors should be converted into exceptions. Patches welcome.

Deprecating trigger_error calls in the Tarot cards?

Yzmir Ramirez — Fri, 08 Apr 2011 16:10:48 -0400

In some environments setting up your own PHP error_handler using set_error_handler() is not an option. Have you considered converting all the trigger_error() calls to throw exceptions?

There are 51 locations that have trigger_error calls and 23 that throw *Exception in 4.2.0.

Is that a 5.x change? Or something that could be rolled into ~~4.3.x~~ 4.4.0?