Page MenuHomePhabricator

Treat all PHP language-level errors as exceptions by default
ClosedPublic

Authored by epriestley on Mar 20 2020, 7:38 PM.

Details

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.

Diff Detail

Repository
rARC Arcanist
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Mar 20 2020, 7:38 PM
epriestley requested review of this revision.Mar 20 2020, 7:38 PM
epriestley updated this revision to Diff 50133.Mar 20 2020, 7:40 PM
  • For completeness, explicitly handle "Undefined offset" (raised for undefined numeric offsets) in addition to "Undefined index" (raised for undefined string indexes).
This revision was not accepted when it landed; it landed in state Needs Review.Mar 22 2020, 7:41 PM
This revision was automatically updated to reflect the committed changes.