From 8cccaaaf6b54574c88dc095fc9777f72f6d9345d Mon Sep 17 00:00:00 2001
From: Toby Zerner <toby.zerner@gmail.com>
Date: Mon, 14 Sep 2015 15:40:07 +0930
Subject: [PATCH] Improve API error handling

- Change 'path' key to 'source.pointer', as per spec
- Add 500 error detail if debug mode is on
---
 js/lib/components/Modal.js                    |  2 +-
 src/Api/Client.php                            | 10 ++++-
 src/Api/Middleware/JsonApiErrors.php          | 39 +++++++++++--------
 .../InvalidConfirmationTokenException.php     | 17 +++++++-
 src/Core/Exceptions/ValidationException.php   | 13 +++----
 5 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/js/lib/components/Modal.js b/js/lib/components/Modal.js
index 38a42c280..1d5b81493 100644
--- a/js/lib/components/Modal.js
+++ b/js/lib/components/Modal.js
@@ -129,7 +129,7 @@ export default class Modal extends Component {
     m.redraw();
 
     if (errors) {
-      this.$('form [name=' + errors[0].path + ']').select();
+      this.$('form [name=' + errors[0].source.pointer.replace('/data/attributes/', '') + ']').select();
     } else {
       this.$('form :input:first').select();
     }
diff --git a/src/Api/Client.php b/src/Api/Client.php
index ccaec307e..6f05dc2ff 100644
--- a/src/Api/Client.php
+++ b/src/Api/Client.php
@@ -12,6 +12,8 @@ namespace Flarum\Api;
 
 use Flarum\Core\Users\User;
 use Illuminate\Contracts\Container\Container;
+use Exception;
+use Flarum\Api\Middleware\JsonApiErrors;
 
 class Client
 {
@@ -41,7 +43,13 @@ class Client
         /** @var \Flarum\Api\Actions\Action $action */
         $action = $this->container->make($actionClass);
 
-        $response = $action->handle(new Request($input, $actor));
+        try {
+            $response = $action->handle(new Request($input, $actor));
+        } catch (Exception $e) {
+            $middleware = new JsonApiErrors();
+
+            $response = $middleware->handle($e);
+        }
 
         return new Response($response);
     }
diff --git a/src/Api/Middleware/JsonApiErrors.php b/src/Api/Middleware/JsonApiErrors.php
index 73adbf7dd..bf55f2b99 100644
--- a/src/Api/Middleware/JsonApiErrors.php
+++ b/src/Api/Middleware/JsonApiErrors.php
@@ -17,45 +17,52 @@ use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Zend\Diactoros\Response\JsonResponse;
 use Zend\Stratigility\ErrorMiddlewareInterface;
+use Flarum\Core;
+use Exception;
 
 class JsonApiErrors implements ErrorMiddlewareInterface
 {
     /**
      * {@inheritdoc}
      */
-    public function __invoke($error, Request $request, Response $response, callable $out = null)
+    public function __invoke($e, Request $request, Response $response, callable $out = null)
     {
-        if ($error instanceof JsonApiSerializable) {
-            $status = $error->getStatusCode();
+        return $this->handle($e);
+    }
 
-            $errors = $error->getErrors();
-        } else if ($error instanceof ValidationException) {
+    public function handle(Exception $e)
+    {
+        if ($e instanceof JsonApiSerializable) {
+            $status = $e->getStatusCode();
+
+            $errors = $e->getErrors();
+        } else if ($e instanceof ValidationException) {
             $status = 422;
 
-            $errors = $error->errors()->toArray();
+            $errors = $e->errors()->toArray();
             $errors = array_map(function ($field, $messages) {
                 return [
                     'detail' => implode("\n", $messages),
-                    'path' => $field,
+                    'source' => ['pointer' => '/data/attributes/' . $field],
                 ];
             }, array_keys($errors), $errors);
-        } else if ($error instanceof ModelNotFoundException) {
+        } else if ($e instanceof ModelNotFoundException) {
             $status = 404;
 
             $errors = [];
         } else {
             $status = 500;
 
-            // If it seems to be a valid HTTP status code, we pass on the
-            // exception's status.
-            $errorCode = $error->getCode();
-            if (is_int($errorCode) && $errorCode >= 400 && $errorCode < 600) {
-                $status = $errorCode;
+            $error = [
+                'code' => $status,
+                'title' => 'Internal Server Error'
+            ];
+
+            if (Core::inDebugMode()) {
+                $error['detail'] = (string) $e;
             }
 
-            $errors = [
-                ['title' => $error->getMessage()]
-            ];
+            $errors = [$error];
         }
 
         // JSON API errors must be collected in an array under the
diff --git a/src/Core/Exceptions/InvalidConfirmationTokenException.php b/src/Core/Exceptions/InvalidConfirmationTokenException.php
index 22ebed2b8..14c22bd56 100644
--- a/src/Core/Exceptions/InvalidConfirmationTokenException.php
+++ b/src/Core/Exceptions/InvalidConfirmationTokenException.php
@@ -12,6 +12,21 @@ namespace Flarum\Core\Exceptions;
 
 use Exception;
 
-class InvalidConfirmationTokenException extends Exception
+class InvalidConfirmationTokenException extends Exception implements JsonApiSerializable
 {
+    /**
+     * {@inheritdoc}
+     */
+    public function getStatusCode()
+    {
+        return 403;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getErrors()
+    {
+        return ['code' => 'invalid_confirmation_token'];
+    }
 }
diff --git a/src/Core/Exceptions/ValidationException.php b/src/Core/Exceptions/ValidationException.php
index 2de94c316..c9b40815c 100644
--- a/src/Core/Exceptions/ValidationException.php
+++ b/src/Core/Exceptions/ValidationException.php
@@ -27,9 +27,7 @@ class ValidationException extends Exception implements JsonApiSerializable
     }
 
     /**
-     * Return the HTTP status code to be used for this exception.
-     *
-     * @return int
+     * {@inheritdoc}
      */
     public function getStatusCode()
     {
@@ -37,15 +35,14 @@ class ValidationException extends Exception implements JsonApiSerializable
     }
 
     /**
-     * Return an array of errors, formatted as JSON-API error objects.
-     *
-     * @see http://jsonapi.org/format/#error-objects
-     * @return array
+     * {@inheritdoc}
      */
     public function getErrors()
     {
         return array_map(function ($path, $detail) {
-            return compact('path', 'detail');
+            $source = ['pointer' => '/data/attributes/' . $path];
+
+            return compact('source', 'detail');
         }, array_keys($this->messages), $this->messages);
     }
 }