Bug 1620879: Improve JSON error messages when JSON flag is on

JSON-encodes more information about the error or exception,
and adds an optional error number.

behatnotneeded: Can't test in Behat

Change-Id: I258e7a275d78c91a5f8cd638ab7f6a7590125a6d

htdocs/lib/errors.php

@@ -447,6 +447,14 @@ function censor_password_parameters(&$backtraceline) {
  * @todo this function should go away
  */
 function die_info($message) {
+
+    // Produce JSON output
+    if (defined('JSON')) {
+        $e = new SystemException($message);
+        $e->handle_exception();
+        exit;
+    }
+
     $smarty = smarty(array(), array(), array(), array('sidebars' => false));
     $smarty->assign('message', $message);
     $smarty->assign('type', 'info');
@@ -553,8 +561,12 @@ function exception ($e) {
 class MaharaException extends Exception {
 
     protected $log = true;
+    const DEFAULT_ERRCODE = 500;
 
-    public function __construct($message='', $code=0) {
+    public function __construct($message='', $code=null) {
+        if ($code === null) {
+            $code = static::DEFAULT_ERRCODE;
+        }
         parent::__construct($message, $code);
         if (!defined('MAHARA_CRASHING')) {
             define('MAHARA_CRASHING', true);
@@ -604,6 +616,32 @@ class MaharaException extends Exception {
         return $this->getMessage();
     }
 
+    /**
+     * Returns an array that will be JSON-encoded,
+     * for when there's an exception in a script
+     * that should give a JSON response.
+     */
+    public function render_json_exception() {
+        return array(
+            'error' => true,
+            'error_number' => $this->getCode(),
+            'error_name' => $this->get_error_name(),
+            'error_class' => get_class($this),
+            'error_message' => $this->getMessage(),
+            'error_rendered' => $this->render_exception()
+        );
+    }
+
+    /**
+     * A machine-readable, non-localized name for this error.
+     * (Defaults to the name of the exception class.)
+     *
+     * @return string
+     */
+    public function get_error_name() {
+        return get_class($this);
+    }
+
     public final function handle_exception() {
 
         if (!empty($this->log)) {
@@ -613,7 +651,7 @@ class MaharaException extends Exception {
         if (defined('JSON')) { // behave differently
             @header('Content-type: text/plain');
             @header('Pragma: no-cache');
-            echo json_encode(array('error' => true, 'message' => $this->render_exception()));
+            echo json_encode($this->render_json_exception());
             exit;
         }
 
@@ -760,6 +798,7 @@ class ConfigException extends MaharaException {
 class UserException extends MaharaException {
 
     protected $log = false;
+    const DEFAULT_ERRCODE = 400;
 
     public function render_exception() {
         return $this->get_string('message') . "\n\n" . $this->getMessage();
@@ -778,6 +817,7 @@ class UserException extends MaharaException {
  * that doesn't exist
  */
 class NotFoundException extends UserException {
+    const DEFAULT_ERRCODE = 404;
     public function strings() {
         return array_merge(parent::strings(),
                            array('message' => get_string('notfoundexception', 'error'),
@@ -819,6 +859,22 @@ class SQLException extends SystemException {
             log_warn($this->getMessage());
         }
     }
+
+    /**
+     * Returns an array that will be JSON-encoded,
+     * for when there's an exception in a script
+     * that should give a JSON response.
+     */
+    public function render_json_exception() {
+        return array(
+            'error' => true,
+            'error_number' => $this->getCode(),
+            'error_name' => $this->get_error_name(),
+            'error_class' => get_class($this),
+            'error_message' => get_config('productionmode') ? '' : $this->getMessage(),
+            'error_rendered' => get_config('productionmode') ? '' : $this->render_exception(),
+        );
+    }
 }
 
 /**
@@ -957,6 +1013,7 @@ class SkinNotFoundException extends NotFoundException {}
  * Exception - Access denied. Throw this if a user is trying to view something they can't
  */
 class AccessDeniedException extends UserException {
+    const DEFAULT_ERRCODE = 403;
     public function strings() {
         return array_merge(parent::strings(),
                            array('message' => get_string('accessdeniedexception', 'error'),
@@ -1016,6 +1073,7 @@ class GroupAccessDeniedException extends AccessDeniedException {
  * as the administrator
  */
 class AccessTotallyDeniedException extends UserException {
+    const DEFAULT_ERRCODE = 403;
     public function strings() {
         return array_merge(parent::strings(),
                            array('message' => get_string('accessdeniedexception', 'error'),

htdocs/lib/mahara.php

@@ -281,6 +281,18 @@ function ensure_internal_plugins_exist() {
     }
 }
 
+/**
+ * Check to see whether a language string is present in the
+ * lang files.
+ *
+ * @param string $identifier
+ * @param string $section
+ * @return boolean
+ */
+function string_exists($identifier, $section = 'mahara') {
+    return get_string($identifier, $section) !== '[[' . $identifier . '/' . $section . ']]';
+}
+
 function get_string($identifier, $section='mahara') {
 
     $variables = func_get_args();

htdocs/webservice/lib.php

@@ -2042,16 +2042,48 @@ function external_reload_component($component, $dir=true) {
  * web service handling code
  */
 class WebserviceException extends MaharaException {
+
+    public $errorcode = null;
+
     /**
      * Constructor
      * @param string $errorcode The name of the string to print
      * @param string $debuginfo optional debugging information
-     * @param object $a Extra words and phrases that might be required in the error string
+     * @param integer $errornumber A numerical identifier for the error (optional)
      */
-    function __construct($errorcode=null, $debuginfo = '', $a=null) {
-        parent::__construct(get_string($errorcode, 'auth.webservice', $a) . $debuginfo);
+    function __construct($errorcode = null, $debuginfo = '', $errornumber = null) {
+        $this->errorcode = rtrim($errorcode, '0123456789');
+
+        if (string_exists($errorcode, 'auth.webservice')) {
+            $message = get_string($errorcode, 'auth.webservice');
+        }
+        else {
+            $message = $errorcode;
+        }
+
+        if ($debuginfo) {
+            $message .= ' : ' . $debuginfo;
+        }
+
+        // In 15.04-16.04, the third parameter to this constructor was
+        // documented as an object. Nothing was done with this object,
+        // so it's unlikely that changing it broke anything. But just
+        // in case, make sure that this param, if provided, is cast
+        // to an integer.
+        if ($errornumber !== null) {
+            $errornumber = (int) $errornumber;
+        }
+
+        parent::__construct($message, $errornumber);
     }
-}
+
+    public function get_error_name() {
+        // Return the error lang string identifier. Trim off any integers
+        // from the end of it, in case we've added one in to notify
+        // translators of a change in the translated string
+        return $this->errorcode;
+    }
+ }
 
 /**
  * Web service parameter exception class
@@ -2059,29 +2091,19 @@ class WebserviceException extends MaharaException {
  * This exception must be thrown to the web service client when a web service parameter is invalid
  * The error string is gotten from webservice.php
  */
-class WebserviceParameterException extends MaharaException {
-    /**
-     * Constructor
-     * @param string $errorcode The name of the string from webservice.php to print
-     * @param string $debuginfo optional debugging information
-     * @param string $a The name of the parameter
-     */
-    function __construct($errorcode=null, $debuginfo = '', $a=null) {
-        parent::__construct(get_string($errorcode, 'auth.webservice', $a) . $debuginfo);
-    }
-}
+class WebserviceParameterException extends WebserviceException {}
 
 /**
  * Exception indicating programming error, must be fixed by a programer. For example
  * a core API might throw this type of exception if a plugin calls it incorrectly.
  */
-class WebserviceCodingException extends MaharaException {
+class WebserviceCodingException extends WebserviceException {
     /**
      * Constructor
      * @param string $debuginfo optional debugging information
      */
     function __construct($debuginfo='') {
-        parent::__construct(get_string('codingerror', 'auth.webservice') . $debuginfo);
+        parent::__construct('codingerror', $debuginfo);
     }
 }
 
@@ -2091,13 +2113,13 @@ class WebserviceCodingException extends MaharaException {
  * user submitted data in forms. It is more suitable
  * for WS and other low level stuff.
  */
-class WebserviceInvalidParameterException extends MaharaException {
+class WebserviceInvalidParameterException extends WebserviceException {
     /**
      * Constructor
      * @param string $debuginfo some detailed information
      */
-    function __construct($debuginfo=null) {
-        parent::__construct(get_string('invalidparameter', 'auth.webservice') . $debuginfo);
+    function __construct($debuginfo='') {
+        parent::__construct('invalidparameter', $debuginfo);
     }
 }
 
@@ -2107,25 +2129,25 @@ class WebserviceInvalidParameterException extends MaharaException {
  * user submitted data in forms. It is more suitable
  * for WS and other low level stuff.
  */
-class WebserviceInvalidResponseException extends MaharaException {
+class WebserviceInvalidResponseException extends WebserviceException {
     /**
      * Constructor
      * @param string $debuginfo some detailed information
      */
-    function __construct($debuginfo=null) {
-        parent::__construct(get_string('invalidresponse', 'auth.webservice', $debuginfo) . $debuginfo);
+    function __construct($debuginfo='') {
+        parent::__construct('invalidresponse', $debuginfo);
     }
 }
 
 /**
  * Exception indicating access control problem in web service call
  */
-class WebserviceAccessException extends MaharaException {
+class WebserviceAccessException extends WebserviceException {
     /**
      * Constructor
      * @param string $debuginfo some detailed information
      */
-    function __construct($debuginfo) {
-        parent::__construct(get_string('accessexception', 'auth.webservice') . $debuginfo);
+    function __construct($debuginfo='') {
+        parent::__construct('accessexception', $debuginfo);
     }
 }

htdocs/webservice/rest/locallib.php

@@ -252,7 +252,14 @@ class webservice_rest_server extends webservice_base_server {
     protected function send_error($ex=null) {
         $this->send_headers($this->format);
         if ($this->format == 'json') {
-            echo json_encode(array('exception' => get_class($ex), 'errorcode' => (isset($ex->errorcode) ? $ex->errorcode : $ex->getCode()), 'message' => $ex->getMessage(), 'debuginfo' => (isset($ex->debuginfo) ? $ex->debuginfo : ''))) . "\n";
+            $classname = get_class($ex);
+            if (!($ex instanceof MaharaException)) {
+                $ex = new SystemException("[{$classname}]: " . $ex->getMessage(), $ex->getCode());
+            }
+            echo json_encode(
+                $ex->render_json_exception(),
+                JSON_PRETTY_PRINT
+            );
         }
         else {
             $xml = '<?xml version="1.0" encoding="UTF-8" ?>' . "\n";