[3.0] & [2.1]: Deprecated: Passing E_USER_ERROR to trigger_error() is deprecated since 8.4

[sbulen]

Basic Information

I've seen this error when doing some testing of a custom script...

Although it was a custom script, this one felt like it needed to be passed on...

Deprecated: Passing E_USER_ERROR to trigger_error() is deprecated since 8.4, throw an exception or call exit with a string message instead in yada\yada\Subs-Db-mysql.php on line 936

Steps to reproduce

1. In my example, I was calling createCategory() with not enough options.

Expected result

No response

Actual result

No response

Version/Git revision

3.0 Alpha 2 - current GH & 2.1

Database Engine

All

Database Version

8.4

PHP Version

8.4

Logs

No response

Additional Information

No response

[sbulen]

Sharing some thoughts here.

There are a lot of these throughout the code... They appear to fall into two categories:

Cat1: Improper internal calls; these are intended to let devs know they massively screwed up calling a function
Cat2: Forcing a hard, immediate exit

Cat1a - An example of a bad params:

SMF/Sources/Subs-Categories.php

Line 238 in b52a4ab

trigger_error($txt['cannot_move_to_deleted_category'], E_USER_ERROR);

Cat1b - Similar, but a step removed (don't want to miss these):

SMF/Sources/Subs-Db-mysql.php

Line 274 in b52a4ab

smf_db_error_backtrace('Wrong value type sent to the database. Datetime expected. (' . $matches[2] . ')', '', E_USER_ERROR, __FILE__, __LINE__);

Cat2 - Is intended to force a hard exit:

SMF/Sources/Security.php

Line 716 in b52a4ab

// We really should never fall through here, for very important reasons. Let's make sure.

SMF/Sources/Security.php

Line 146 in b52a4ab

trigger_error('No direct access...', E_USER_ERROR);

Note the the Drupal guys did a simple substitution of all E_USER_ERROR to E_USER_WARNING:
https://www.drupal.org/node/3466031

This change shifts error handling to a less severe level (E_USER_WARNING), allowing for continued execution while still notifying developers and site administrators of potential issues. This approach avoids hard crashes, particularly in scenarios that involve edge cases, and provides a more user-friendly error handling strategy.

So...

2.1 PROPOSAL: For 2.1, I propose we do similar... For all Cat1 examples, downgrade E_USER_ERROR to E_USER_WARNING. Change all Cat2 calls to exit().

The point of this change appears to be to start to force folks to use try/throw/catch universally & develop a more robust universal error handler that can handle anything from user warnings to the Cat1 & Cat2 examples above. 3.0 may even be 100% towards a "ONE TRUE HANDLER" already (which is pretty impressive, IMO)...

3.0 PROPOSAL: For 3.0, for all Cat1 examples, downgrade E_USER_ERROR to E_USER_WARNING. That may be all that is necessary. I don't think Cat2 exists anymore.

See also #8343 which I think is still a significant issue in 3.0 error handling, though.

[jdarwood007]

For 3.0, why not move to a throw?

[sbulen]

That would work.

I'm not sure if there is any benefit to throw over trigger_error()? Although it does feel like the 8.4 deprecations indicate a direction away from trigger_error()...