1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-25 08:12:40 +01:00

Treat all PHP language-level errors as exceptions by default

Summary:
Ref T13499. Currently, we throw most language errors as exceptions, but this list isn't exhaustive and errors we don't specifically make more severe are allowed to slip through.

This is generally undesirable, particularly in the case of "Undefined index:" errors. See T13499 for a specific case where this caused behavior to be more difficult to understand and diagnose than it should have been.

Make this the default behavior instead, except for "E_USER" errors, which we never expect to arise from first-party code.

This may be slightly too aggressive, but future changes can selectively reduce the severity of some types of errors if problems arise.

Test Plan:
  - Executed code which intentionally accessed an undefined index, got an exception.
  - Poked around Phabricator and Arcanist without any further issues cropping up, but I don't have a good way to develop confidence that the "reduced severity" list should genuinely be empty.

Maniphest Tasks: T13499

Differential Revision: https://secure.phabricator.com/D21044
This commit is contained in:
epriestley 2020-03-20 12:33:25 -07:00
parent 18799c1829
commit 4c12c3119b

View file

@ -195,6 +195,33 @@ final class PhutilErrorHandler extends Phobject {
return false;
}
// See T13499. If this is a user error arising from "trigger_error()" or
// similar, route it through normal error handling: this is probably the
// best match to authorial intent, since the code could choose to throw
// an exception instead if it wanted that behavior. Phabricator does not
// use "trigger_error()" so we never normally expect to reach this
// block in first-party code.
if (($num === E_USER_ERROR) ||
($num === E_USER_WARNING) ||
($num === E_USER_NOTICE)) {
$trace = debug_backtrace();
array_shift($trace);
self::dispatchErrorMessage(
self::ERROR,
$str,
array(
'file' => $file,
'line' => $line,
'context' => $ctx,
'error_code' => $num,
'trace' => $trace,
));
return;
}
// Convert typehint failures into exceptions.
if (preg_match('/^Argument (\d+) passed to (\S+) must be/', $str)) {
throw new InvalidArgumentException($str);
@ -221,18 +248,19 @@ final class PhutilErrorHandler extends Phobject {
throw new RuntimeException($str);
}
$trace = debug_backtrace();
array_shift($trace);
self::dispatchErrorMessage(
self::ERROR,
$str,
array(
'file' => $file,
'line' => $line,
'context' => $ctx,
'error_code' => $num,
'trace' => $trace,
));
// Convert undefined indexes into exceptions.
if (preg_match('/^Undefined index: /', $str)) {
throw new RuntimeException($str);
}
// Convert undefined offsets into exceptions.
if (preg_match('/^Undefined offset: /', $str)) {
throw new RuntimeException($str);
}
// See T13499. Convert all other runtime errors not handled in a more
// specific way into runtime exceptions.
throw new RuntimeException($str);
}
/**