Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.4: trigger_error() updates #4063

Merged
merged 3 commits into from
Nov 14, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
PHP 8.4: trigger_error() updates
  • Loading branch information
Girgias committed Nov 14, 2024
commit a8a02f940b561aec838cfdf55bc8d98bec5a3513
27 changes: 25 additions & 2 deletions reference/errorfunc/functions/trigger-error.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@
The designated error type for this error. It only works with the <constant>E_USER_<replaceable>*</replaceable></constant>
family of constants, and will default to <constant>E_USER_NOTICE</constant>.
</para>
<warning>
Passing <constant>E_USER_ERROR</constant> as the
<parameter>error_level</parameter> is now deprecated.
Throw an <exceptionname>Exception</exceptionname> or
call <function>exit</function> instead.
</warning>
Girgias marked this conversation as resolved.
Show resolved Hide resolved
</listitem>
</varlistentry>
</variablelist>
Expand Down Expand Up @@ -80,6 +86,22 @@
</row>
</thead>
<tbody>
<row>
<entry>8.4.0</entry>
<entry>
Passing <constant>E_USER_ERROR</constant> as the
<parameter>error_level</parameter> is now deprecated.
Throw an <exceptionname>Exception</exceptionname> or
call <function>exit</function> instead.
</entry>
</row>
<row>
<entry>8.4.0</entry>
<entry>
The function now has a return type of <type>true</type>
instead of <type>bool</type>.
</entry>
</row>
<row>
<entry>8.0.0</entry>
<entry>
Expand All @@ -103,8 +125,8 @@
<programlisting role="php">
<![CDATA[
<?php
if ($divisor == 0) {
trigger_error("Cannot divide by zero", E_USER_ERROR);
if (is_nan($divisor)) {
trigger_error("Cannot divide by NAN", E_USER_WARNING);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message makes not much sense, given that E_USER_WARNING would not abort, so the division will be carried out. Maybe:

Suggested change
trigger_error("Cannot divide by NAN", E_USER_WARNING);
trigger_error("Division by NAN", E_USER_WARNING);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, finding a good example is hard, because generally speaking there is no good reason to use trigger_error() nowadays. Either use an Exception or use error_log() or another logging facility. But PHP's internal error reporting mechanism which allows for side-effects is not a good idea.

That said, how about this:

$password = $_POST['password'] ?? '';
if ($password === '') {
  trigger_error("Using an empty password is unsafe", E_USER_WARNING);
}
$hash = password_hash($password, PASSWORD_DEFAULT);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would require that the error is shown to the user, though.

}
?>
]]>
Expand Down Expand Up @@ -132,6 +154,7 @@ if ($divisor == 0) {
<member><function>set_error_handler</function></member>
<member><function>restore_error_handler</function></member>
<member>The <link linkend="errorfunc.constants">error level constants</link></member>
<member>The <classname>Deprecated</classname> attribute</member>
Girgias marked this conversation as resolved.
Show resolved Hide resolved
</simplelist>
</para>
</refsect1>
Expand Down