Merge pull request #9707 from robbieaverill/pulls/4.7/exceptions

docs/en/02_Developer_Guides/00_Model/05_Extending_DataObjects.md

@@ -27,25 +27,23 @@ use SilverStripe\ORM\DataObject;
 class Player extends DataObject 
 {
     private static $has_many = [
-        "Teams" => "Team",
+        'Teams' => 'Team',
     ];
 
     public function onBeforeWrite() 
     {
         // check on first write action, aka "database row creation" (ID-property is not set)
-        if(!$this->isInDb()) {
+        if (!$this->isInDb()) {
             $currentPlayer = Security::getCurrentUser();
 
-            if(!$currentPlayer->IsTeamManager()) {
-                user_error('Player-creation not allowed', E_USER_ERROR);
-                exit();
+            if (!$currentPlayer->IsTeamManager()) {
+                throw new \Exception('Player-creation not allowed');
             }
         }
 
         // check on every write action
-        if(!$this->record['TeamID']) {
-            user_error('Cannot save player without a valid team', E_USER_ERROR);
-            exit();
+        if (!$this->record['TeamID']) {
+            throw new \Exception('Cannot save player without a valid team');
         }
 
         // CAUTION: You are required to call the parent-function, otherwise

docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md

@@ -115,7 +115,8 @@ developers know:
     * Things that will prevent an internal function from continuing.  Throw a warning and return null.
 
 *  **E_USER_ERROR:** Throwing one of these errors is going to take down the production site.  So you should only throw
-E_USER_ERROR if it's going to be **dangerous** or **impossible** to continue with the request.
+E_USER_ERROR if it's going to be **dangerous** or **impossible** to continue with the request. Note that it is
+preferable to now throw exceptions instead of `E_USER_ERROR`.
 
 ## Configuring error logging
 

src/Control/Controller.php

@@ -199,7 +199,7 @@ protected function afterHandleRequest()
     public function handleRequest(HTTPRequest $request)
     {
         if (!$request) {
-            user_error("Controller::handleRequest() not passed a request!", E_USER_ERROR);
+            throw new \RuntimeException('Controller::handleRequest() not passed a request!');
         }
 
         //set up the controller for the incoming request

src/Control/HTTPRequest.php

@@ -858,7 +858,7 @@ public function httpMethod()
     public function setHttpMethod($method)
     {
         if (!self::isValidHttpMethod($method)) {
-            user_error('HTTPRequest::setHttpMethod: Invalid HTTP method', E_USER_ERROR);
+            throw new \InvalidArgumentException('HTTPRequest::setHttpMethod: Invalid HTTP method');
         }
 
         $this->httpMethod = strtoupper($method);
@@ -912,12 +912,11 @@ public static function detect_method($origMethod, $postVars)
     {
         if (isset($postVars['_method'])) {
             if (!self::isValidHttpMethod($postVars['_method'])) {
-                user_error('HTTPRequest::detect_method(): Invalid "_method" parameter', E_USER_ERROR);
+                throw new InvalidArgumentException('HTTPRequest::detect_method(): Invalid "_method" parameter');
             }
             return strtoupper($postVars['_method']);
-        } else {
-            return $origMethod;
         }
+        return $origMethod;
     }
 
     /**

src/Control/RequestHandler.php

@@ -187,7 +187,7 @@ public function handleRequest(HTTPRequest $request)
             }
             $action = "index";
         } elseif (!is_string($action)) {
-            user_error("Non-string method name: " . var_export($action, true), E_USER_ERROR);
+            throw new InvalidArgumentException("Non-string method name: " . var_export($action, true));
         }
 
         $classMessage = Director::isLive() ? 'on this handler' : 'on class ' . static::class;

src/Core/Extensible.php

@@ -183,17 +183,17 @@ public static function add_extension($classOrExtension, $extension = null)
         }
         $extensionClass = $matches[1];
         if (!class_exists($extensionClass)) {
-            user_error(
-                sprintf('Object::add_extension() - Can\'t find extension class for "%s"', $extensionClass),
-                E_USER_ERROR
-            );
+            throw new InvalidArgumentException(sprintf(
+                'Object::add_extension() - Can\'t find extension class for "%s"',
+                $extensionClass
+            ));
         }
 
-        if (!is_subclass_of($extensionClass, 'SilverStripe\\Core\\Extension')) {
-            user_error(
-                sprintf('Object::add_extension() - Extension "%s" is not a subclass of Extension', $extensionClass),
-                E_USER_ERROR
-            );
+        if (!is_subclass_of($extensionClass, Extension::class)) {
+            throw new InvalidArgumentException(sprintf(
+                'Object::add_extension() - Extension "%s" is not a subclass of Extension',
+                $extensionClass
+            ));
         }
 
         // unset some caches
@@ -334,7 +334,7 @@ public static function get_extra_config_sources($class = null)
         $sources = [];
 
         foreach ($extensions as $extension) {
-            list($extensionClass, $extensionArgs) = ClassInfo::parse_class_spec($extension);
+            [$extensionClass, $extensionArgs] = ClassInfo::parse_class_spec($extension);
             // Strip service name specifier
             $extensionClass = strtok($extensionClass, '.');
             $sources[] = $extensionClass;

src/Dev/CsvBulkLoader.php

@@ -344,7 +344,7 @@ protected function processRecord($record, $columnMap, &$results, $preview = fals
                 }
             } elseif (strpos($fieldName, '.') !== false) {
                 // we have a relation column with dot notation
-                list($relationName, $columnName) = explode('.', $fieldName);
+                [$relationName, $columnName] = explode('.', $fieldName);
                 // always gives us an component (either empty or existing)
                 $relationObj = $obj->getComponent($relationName);
                 if (!$preview) {
@@ -445,15 +445,19 @@ public function findExistingObject($record, $columnMap = [])
                 } elseif ($SNG_objectClass->hasMethod($duplicateCheck['callback'])) {
                     $existingRecord = $SNG_objectClass->{$duplicateCheck['callback']}($record[$fieldName], $record);
                 } else {
-                    user_error("CsvBulkLoader::processRecord():"
-                        . " {$duplicateCheck['callback']} not found on importer or object class.", E_USER_ERROR);
+                    throw new \RuntimeException(
+                        "CsvBulkLoader::processRecord():"
+                        . " {$duplicateCheck['callback']} not found on importer or object class."
+                    );
                 }
 
                 if ($existingRecord) {
                     return $existingRecord;
                 }
             } else {
-                user_error('CsvBulkLoader::processRecord(): Wrong format for $duplicateChecks', E_USER_ERROR);
+                throw new \InvalidArgumentException(
+                    'CsvBulkLoader::processRecord(): Wrong format for $duplicateChecks'
+                );
             }
         }
 

src/Dev/SapphireTest.php

@@ -480,11 +480,11 @@ protected function idFromFixture($className, $identifier)
         $id = $state->getFixtureFactory(static::class)->getId($className, $identifier);
 
         if (!$id) {
-            user_error(sprintf(
+            throw new \InvalidArgumentException(sprintf(
                 "Couldn't find object '%s' (class: %s)",
                 $identifier,
                 $className
-            ), E_USER_ERROR);
+            ));
         }
 
         return $id;
@@ -519,11 +519,11 @@ protected function objFromFixture($className, $identifier)
         $obj = $state->getFixtureFactory(static::class)->get($className, $identifier);
 
         if (!$obj) {
-            user_error(sprintf(
+            throw new \InvalidArgumentException(sprintf(
                 "Couldn't find object '%s' (class: %s)",
                 $identifier,
                 $className
-            ), E_USER_ERROR);
+            ));
         }
 
         return $obj;
@@ -1021,16 +1021,16 @@ public static function start()
 
             $app = new HTTPApplication($kernel);
             $flush = array_key_exists('flush', $request->getVars());
-    
+
             // Custom application
             $app->execute($request, function (HTTPRequest $request) {
                 // Start session and execute
                 $request->getSession()->init($request);
-    
+
                 // Invalidate classname spec since the test manifest will now pull out new subclasses for each internal class
                 // (e.g. Member will now have various subclasses of DataObjects that implement TestOnly)
                 DataObject::reset();
-    
+
                 // Set dummy controller;
                 $controller = Controller::create();
                 $controller->setRequest($request);

src/Forms/CompositeField.php

@@ -251,11 +251,10 @@ public function collateDataFields(&$list, $saveableOnly = false)
                     if (isset($list[$name])) {
                         $fieldClass = get_class($field);
                         $otherFieldClass = get_class($list[$name]);
-                        user_error(
+                        throw new \RuntimeException(
                             "collateDataFields() I noticed that a field called '$name' appears twice in"
                              . " your form: '{$formName}'.  One is a '{$fieldClass}' and the other is a"
-                             . " '{$otherFieldClass}'",
-                            E_USER_ERROR
+                             . " '{$otherFieldClass}'"
                         );
                     }
                     $list[$name] = $field;

src/Forms/FieldList.php

@@ -165,15 +165,12 @@ protected function fieldNameError(FormField $field, $functionName)
             $errorSuffix = '';
         }
 
-        user_error(
-            sprintf(
-                "%s() I noticed that a field called '%s' appears twice%s",
-                $functionName,
-                $field->getName(),
-                $errorSuffix
-            ),
-            E_USER_ERROR
-        );
+        throw new \RuntimeException(sprintf(
+            "%s() I noticed that a field called '%s' appears twice%s",
+            $functionName,
+            $field->getName(),
+            $errorSuffix
+        ));
     }
 
     protected function flushFieldsCache()
@@ -213,9 +210,8 @@ protected function collateDataFields(&$list, $saveableOnly = false)
                     } else {
                         $errSuffix = '';
                     }
-                    user_error(
-                        "collateDataFields() I noticed that a field called '$name' appears twice$errSuffix.",
-                        E_USER_ERROR
+                    throw new \RuntimeException(
+                        "collateDataFields() I noticed that a field called '$name' appears twice$errSuffix."
                     );
                 }
                 $list[$name] = $field;
@@ -493,10 +489,9 @@ public function findOrMakeTab($tabName, $title = null)
                         ? " named '{$parentPointer->getName()}'"
                         : null;
                     $parentPointerClass = get_class($parentPointer);
-                    user_error(
+                    throw new \InvalidArgumentException(
                         "FieldList::addFieldToTab() Tried to add a tab to object"
-                        . " '{$parentPointerClass}'{$withName} - '{$part}' didn't exist.",
-                        E_USER_ERROR
+                        . " '{$parentPointerClass}'{$withName} - '{$part}' didn't exist."
                     );
                 }
             }

src/Forms/FormField.php

@@ -1414,12 +1414,7 @@ public function rootFieldList()
         }
 
         $class = static::class;
-        user_error(
-            "rootFieldList() called on {$class} object without a containerFieldList",
-            E_USER_ERROR
-        );
-
-        return null;
+        throw new \RuntimeException("rootFieldList() called on {$class} object without a containerFieldList");
     }
 
     /**

src/Forms/SelectField.php

@@ -173,7 +173,7 @@ protected function getListMap($source)
             $source = $source->toArray();
         }
         if (!is_array($source) && !($source instanceof ArrayAccess)) {
-            user_error('$source passed in as invalid type', E_USER_ERROR);
+            throw new \InvalidArgumentException('$source passed in as invalid type');
         }
 
         return $source;

src/Forms/TreeMultiselectField.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Forms;
 
+use http\Exception\InvalidArgumentException;
 use SilverStripe\Control\Controller;
 use SilverStripe\Core\Convert;
 use SilverStripe\ORM\ArrayList;
@@ -252,10 +253,9 @@ public function saveInto(DataObjectInterface $record)
         $saveDest = $record->$fieldName();
         if (!$saveDest) {
             $recordClass = get_class($record);
-            user_error(
+            throw new \RuntimeException(
                 "TreeMultiselectField::saveInto() Field '$fieldName' not found on"
-                . " {$recordClass}.{$record->ID}",
-                E_USER_ERROR
+                . " {$recordClass}.{$record->ID}"
             );
         }
 

src/ORM/Connect/Database.php

@@ -431,9 +431,8 @@ public function manipulate($manipulation)
                     break;
 
                 default:
-                    user_error(
-                        "SS_Database::manipulate() Can't recognise command '{$writeInfo['command']}'",
-                        E_USER_ERROR
+                    throw new \InvalidArgumentException(
+                        "SS_Database::manipulate() Can't recognise command '{$writeInfo['command']}'"
                     );
             }
         }

src/ORM/Connect/MySQLiConnector.php

@@ -2,9 +2,9 @@
 
 namespace SilverStripe\ORM\Connect;
 
-use SilverStripe\Core\Config\Config;
 use mysqli;
 use mysqli_stmt;
+use SilverStripe\Core\Config\Config;
 
 /**
  * Connector for MySQL using the MySQLi method
@@ -238,11 +238,9 @@ public function parsePreparedParameters($parameters, &$blobs)
                 case 'array':
                 case 'unknown type':
                 default:
-                    user_error(
-                        "Cannot bind parameter \"$value\" as it is an unsupported type ($phpType)",
-                        E_USER_ERROR
+                    throw new \InvalidArgumentException(
+                        "Cannot bind parameter \"$value\" as it is an unsupported type ($phpType)"
                     );
-                    break;
             }
             $values[] = $value;
         }

src/ORM/DataList.php

@@ -1274,7 +1274,7 @@ public function offsetGet($key)
      */
     public function offsetSet($key, $value)
     {
-        user_error("Can't alter items in a DataList using array-access", E_USER_ERROR);
+        throw new \BadMethodCallException("Can't alter items in a DataList using array-access");
     }
 
     /**
@@ -1284,6 +1284,6 @@ public function offsetSet($key, $value)
      */
     public function offsetUnset($key)
     {
-        user_error("Can't alter items in a DataList using array-access", E_USER_ERROR);
+        throw new \BadMethodCallException("Can't alter items in a DataList using array-access");
     }
 }

src/ORM/DataObject.php

@@ -1391,8 +1391,10 @@ protected function preWrite()
         $this->brokenOnWrite = true;
         $this->onBeforeWrite();
         if ($this->brokenOnWrite) {
-            user_error(static::class . " has a broken onBeforeWrite() function."
-                . " Make sure that you call parent::onBeforeWrite().", E_USER_ERROR);
+            throw new LogicException(
+                static::class . " has a broken onBeforeWrite() function."
+                . " Make sure that you call parent::onBeforeWrite()."
+            );
         }
     }
 
@@ -1737,8 +1739,10 @@ public function delete()
         $this->brokenOnDelete = true;
         $this->onBeforeDelete();
         if ($this->brokenOnDelete) {
-            user_error(static::class . " has a broken onBeforeDelete() function."
-                . " Make sure that you call parent::onBeforeDelete().", E_USER_ERROR);
+            throw new LogicException(
+                static::class . " has a broken onBeforeDelete() function."
+                . " Make sure that you call parent::onBeforeDelete()."
+            );
         }
 
         // Deleting a record without an ID shouldn't do anything
@@ -2874,7 +2878,7 @@ public function setField($fieldName, $val)
     public function setCastedField($fieldName, $value)
     {
         if (!$fieldName) {
-            user_error("DataObject::setCastedField: Called without a fieldName", E_USER_ERROR);
+            throw new InvalidArgumentException("DataObject::setCastedField: Called without a fieldName");
         }
         $fieldObj = $this->dbObject($fieldName);
         if ($fieldObj) {