Page MenuHomePhabricator

Sniff against use of trigger_error with E_USER_ERROR
Open, Needs TriagePublic

Description

As the reader may know, trigger_error() is generally what we reach for when we want to emit our own custom errors in a similar way as PHP's own non-throwable errors (such as the "PHP Notice" for an undefined variable that becomes null, or the "PHP Warning" when reading a file that isn't found or when calling a deprecated built-in function).

What you might not know is that when you pass E_USER_ERROR (instead of _NOTICE or _WARNING) it changes the behaviour from logging to stderr, to instead fatalling the PHP process. I believe this is most certainly not intended behaviour in our code bases. When someone intends to do that, I would expect one to either throw an exception or use exit(1). If there are extreme edge cases where this is truly intended and beneficial of these other approaches, one could always disable the inline sniff.

I learned about this issue via these three changes:

Event Timeline

Change 720034 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/WikimediaEvents@master] PrefUpdateInstrumentation: Restore returns and fix incorrect trigger_error

https://gerrit.wikimedia.org/r/720034

Change 720034 merged by jenkins-bot:

[mediawiki/extensions/WikimediaEvents@master] PrefUpdateInstrumentation: Restore returns and fix incorrect trigger_error

https://gerrit.wikimedia.org/r/720034