Refactor error handling internally to follow RFC7807 (#362)

* Refactor error handling internally to follow RFC7807

* style fixes

app/Exceptions/AircraftNotAtAirport.php

@@ -2,11 +2,49 @@
 
 namespace App\Exceptions;
 
+use App\Models\Aircraft;
+
 /**
  * Class AircraftNotAtAirport
  */
-class AircraftNotAtAirport extends InternalError
+class AircraftNotAtAirport extends HttpException
 {
-    public const FIELD = 'aircraft_id';
     public const MESSAGE = 'The aircraft is not at the departure airport';
+
+    private $aircraft;
+
+    public function __construct(Aircraft $aircraft)
+    {
+        $this->aircraft = $aircraft;
+        parent::__construct(
+            400,
+            static::MESSAGE
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'aircraft-not-at-airport';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [
+            'aircraft_id' => $this->aircraft->id,
+        ];
+    }
 }

app/Exceptions/AircraftPermissionDenied.php

@@ -2,11 +2,51 @@
 
 namespace App\Exceptions;
 
-/**
- * Class AircraftPermissionDenied
- */
-class AircraftPermissionDenied extends InternalError
+use App\Models\Aircraft;
+use App\Models\User;
+
+class AircraftPermissionDenied extends HttpException
 {
-    public const FIELD = 'aircraft_id';
     public const MESSAGE = 'User is not allowed to fly this aircraft';
+
+    private $aircraft;
+    private $user;
+
+    public function __construct(User $user, Aircraft $aircraft)
+    {
+        $this->aircraft = $aircraft;
+        $this->user = $user;
+
+        parent::__construct(
+            400,
+            static::MESSAGE
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'aircraft-permission-denied';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [
+            'aircraft_id' => $this->aircraft->id,
+            'user_id'     => $this->user->id,
+        ];
+    }
 }

app/Exceptions/BidExists.php

@@ -1,26 +0,0 @@
-<?php
-
-namespace App\Exceptions;
-
-use Symfony\Component\HttpKernel\Exception\HttpException;
-
-/**
- * Class BidExists
- */
-class BidExists extends HttpException
-{
-    public function __construct(
-        string $message = null,
-        \Exception $previous = null,
-        int $code = 0,
-        array $headers = []
-    ) {
-        parent::__construct(
-            409,
-            'A bid already exists for this flight',
-            $previous,
-            $headers,
-            $code
-        );
-    }
-}

app/Exceptions/BidExistsForFlight.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace App\Exceptions;
+
+use App\Models\Flight;
+
+class BidExistsForFlight extends HttpException
+{
+    private $flight;
+
+    public function __construct(Flight $flight)
+    {
+        $this->flight = $flight;
+        parent::__construct(
+            409,
+            'A bid already exists for this flight'
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'bid-exists';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [
+            'flight_id' => $this->flight->id,
+        ];
+    }
+}

app/Exceptions/Converters/GenericException.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace App\Exceptions\Converters;
+
+use App\Exceptions\HttpException;
+use Exception;
+
+class GenericException extends HttpException
+{
+    private $exception;
+
+    public function __construct(Exception $exception)
+    {
+        $this->exception = $exception;
+        parent::__construct(
+            503,
+            $exception->getMessage()
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'internal-error';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        // Only add trace if in dev
+        if (config('app.env') === 'dev') {
+            return [
+                'trace' => $this->exception->getTrace()[0],
+            ];
+        }
+
+        return [];
+    }
+}

app/Exceptions/Converters/NotFound.php

@@ -0,0 +1,44 @@
+<?php
+
+namespace App\Exceptions\Converters;
+
+use App\Exceptions\HttpException;
+use Exception;
+
+class NotFound extends HttpException
+{
+    private $exception;
+
+    public function __construct(Exception $exception)
+    {
+        $this->exception = $exception;
+        parent::__construct(
+            404,
+            $exception->getMessage()
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'not-found';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [];
+    }
+}

app/Exceptions/Converters/SymfonyException.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace App\Exceptions\Converters;
+
+use App\Exceptions\HttpException;
+use Symfony\Component\HttpKernel\Exception\HttpException as SymfonyHttpException;
+
+class SymfonyException extends HttpException
+{
+    private $exception;
+
+    public function __construct(SymfonyHttpException $exception)
+    {
+        $this->exception = $exception;
+        parent::__construct(
+            $exception->getStatusCode(),
+            $exception->getMessage()
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'internal-error';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        // Only add trace if in dev
+        if (config('app.env') === 'dev') {
+            return [
+                'trace' => $this->exception->getTrace()[0],
+            ];
+        }
+
+        return [];
+    }
+}

app/Exceptions/Converters/ValidationException.php

@@ -0,0 +1,63 @@
+<?php
+
+namespace App\Exceptions\Converters;
+
+use App\Exceptions\HttpException;
+use Illuminate\Support\Facades\Log;
+use Illuminate\Validation\ValidationException as IlluminateValidationException;
+
+class ValidationException extends HttpException
+{
+    private $validationException;
+    private $errorDetail;
+    private $errors;
+
+    public function __construct(IlluminateValidationException $validationException)
+    {
+        $this->validationException = $validationException;
+        $this->processValidationErrors();
+
+        parent::__construct(
+            400,
+            'Validation exception'
+        );
+    }
+
+    private function processValidationErrors()
+    {
+        $error_messages = [];
+        $this->errors = $this->validationException->errors();
+        foreach ($this->errors as $field => $error) {
+            $error_messages[] = implode(', ', $error);
+        }
+
+        $this->errorDetail = implode(', ', $error_messages);
+        // Log::error('Validation errors', $this->errors);
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'validation-exception';
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->errorDetail;
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [
+            'errors' => $this->errors,
+        ];
+    }
+}

app/Exceptions/Handler.php

@@ -2,13 +2,21 @@
 
 namespace App\Exceptions;
 
+use App\Exceptions\Converters\GenericException;
+use App\Exceptions\Converters\NotFound;
+use App\Exceptions\Converters\SymfonyException;
+use App\Exceptions\Converters\ValidationException;
 use Exception;
+use Illuminate\Auth\Access\AuthorizationException;
 use Illuminate\Auth\AuthenticationException;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
-use Illuminate\Validation\ValidationException;
-use Log;
-use Symfony\Component\HttpKernel\Exception\HttpException;
+use Illuminate\Http\Request;
+use Illuminate\Session\TokenMismatchException;
+use Illuminate\Support\Facades\Log;
+use Illuminate\Validation\ValidationException as IlluminateValidationException;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\HttpException as SymfonyHttpException;
 use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
@@ -21,85 +29,56 @@ class Handler extends ExceptionHandler
      * A list of the exception types that should not be reported.
      */
     protected $dontReport = [
-        \Illuminate\Auth\AuthenticationException::class,
-        \Illuminate\Auth\Access\AuthorizationException::class,
-        \Symfony\Component\HttpKernel\Exception\HttpException::class,
-        \Illuminate\Database\Eloquent\ModelNotFoundException::class,
-        \Illuminate\Session\TokenMismatchException::class,
-        \Illuminate\Validation\ValidationException::class,
+        AuthenticationException::class,
+        AuthorizationException::class,
+        HttpException::class,
+        IlluminateValidationException::class,
+        ModelNotFoundException::class,
+        SymfonyHttpException::class,
+        TokenMismatchException::class,
     ];
 
-    /**
-     * Create an error message
-     *
-     * @param $status_code
-     * @param $message
-     *
-     * @return array
-     */
-    protected function createError($status_code, $message)
-    {
-        return [
-            'error' => [
-                'status'  => $status_code,
-                'message' => $message,
-            ],
-        ];
-    }
-
     /**
      * Render an exception into an HTTP response.
      *
-     * @param \Illuminate\Http\Request $request
-     * @param \Exception               $exception
+     * @param Request   $request
+     * @param Exception $exception
      *
      * @return mixed
      */
     public function render($request, Exception $exception)
     {
         if ($request->is('api/*')) {
-            $headers = [];
-
             Log::error('API Error', $exception->getTrace());
 
+            if ($exception instanceof HttpException) {
+                return $exception->getResponse();
+            }
+
+            /*
+             * Not of the HttpException abstract class. Map these into
+             */
+
             if ($exception instanceof ModelNotFoundException ||
                 $exception instanceof NotFoundHttpException) {
-                $error = $this->createError(404, $exception->getMessage());
+                $error = new NotFound($exception);
+                return $error->getResponse();
             }
 
             // Custom exceptions should be extending HttpException
-            elseif ($exception instanceof HttpException) {
-                $error = $this->createError(
-                    $exception->getStatusCode(),
-                    $exception->getMessage()
-                );
-
-                $headers = $exception->getHeaders();
+            if ($exception instanceof SymfonyHttpException) {
+                $error = new SymfonyException($exception);
+                return $error->getResponse();
             }
 
             // Create the detailed errors from the validation errors
-            elseif ($exception instanceof ValidationException) {
-                $error_messages = [];
-                $errors = $exception->errors();
-                foreach ($errors as $field => $error) {
-                    $error_messages[] = implode(', ', $error);
-                }
-
-                $message = implode(', ', $error_messages);
-                $error = $this->createError(400, $message);
-                $error['error']['errors'] = $errors;
-
-                Log::error('Validation errors', $errors);
-            } else {
-                $error = $this->createError(400, $exception->getMessage());
+            if ($exception instanceof IlluminateValidationException) {
+                $error = new ValidationException($exception);
+                return $error->getResponse();
             }
 
-            // Only add trace if in dev
-            if (config('app.env') === 'dev') {
-                $error['error']['trace'] = $exception->getTrace()[0];
-            }
-
-            return response()->json($error, $error['error']['status'], $headers);
+            $error = new GenericException($exception);
+            return $error->getResponse();
         }
 
         if ($exception instanceof HttpException
@@ -113,16 +92,16 @@ class Handler extends ExceptionHandler
     /**
      * Convert an authentication exception into an unauthenticated response.
      *
-     * @param \Illuminate\Http\Request                 $request
-     * @param \Illuminate\Auth\AuthenticationException $exception
+     * @param Request                 $request
+     * @param AuthenticationException $exception
      *
      * @return \Illuminate\Http\Response
      */
     protected function unauthenticated($request, AuthenticationException $exception)
     {
         if ($request->expectsJson() || $request->is('api/*')) {
-            $error = $this->createError(401, 'Unauthenticated');
-            return response()->json($error, 401);
+            $error = new Unauthenticated();
+            return $error->getResponse();
         }
 
         return redirect()->guest('login');
@@ -133,7 +112,7 @@ class Handler extends ExceptionHandler
      *
      * @param HttpException $e
      *
-     * @return \Illuminate\Http\Response|\Symfony\Component\HttpFoundation\Response
+     * @return \Illuminate\Http\Response|Response
      */
     protected function renderHttpException(HttpExceptionInterface $e)
     {

app/Exceptions/HttpException.php

@@ -0,0 +1,60 @@
+<?php
+
+namespace App\Exceptions;
+
+use Symfony\Component\HttpKernel\Exception\HttpException as SymfonyHttpException;
+
+abstract class HttpException extends SymfonyHttpException
+{
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    abstract public function getErrorType(): string;
+
+    /**
+     * Get the detailed error string
+     */
+    abstract public function getErrorDetails(): string;
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    abstract public function getErrorMetadata(): array;
+
+    /**
+     * Return the error message as JSON
+     */
+    public function getJson()
+    {
+        $response = [];
+
+        $response['type'] = config('phpvms.error_root').'/'.$this->getErrorType();
+        $response['title'] = $this->getMessage();
+        $response['details'] = $this->getErrorDetails();
+
+        // For backwards compatibility
+        $response['error'] = [
+            'status'  => $this->getStatusCode(),
+            'message' => $this->getErrorDetails(),
+        ];
+
+        return array_merge($response, $this->getErrorMetadata());
+    }
+
+    /**
+     * Return a response object that can be used by Laravel
+     *
+     * @return \Illuminate\Http\JsonResponse
+     */
+    public function getResponse()
+    {
+        return response()
+            ->json(
+                $this->getJson(),
+                $this->getStatusCode(),
+                [
+                    'content-type' => 'application/problem+json',
+                ]
+            );
+    }
+}

app/Exceptions/InternalError.php

@@ -2,9 +2,9 @@
 
 namespace App\Exceptions;
 
+use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Facades\Validator;
 use Illuminate\Validation\ValidationException;
-use Log;
-use Validator;
 
 /**
  * Show an internal error, bug piggyback off of the validation

app/Exceptions/PirepCancelled.php

@@ -2,25 +2,38 @@
 
 namespace App\Exceptions;
 
-use Symfony\Component\HttpKernel\Exception\HttpException;
+use App\Models\Pirep;
 
-/**
- * Class PirepCancelled
- */
 class PirepCancelled extends HttpException
 {
-    public function __construct(
-        string $message = null,
-        \Exception $previous = null,
-        int $code = 0,
-        array $headers = []
-    ) {
+    private $pirep;
+
+    public function __construct(Pirep $pirep)
+    {
+        $this->pirep = $pirep;
         parent::__construct(
             400,
-            'PIREP has been cancelled, updates are not allowed',
-            $previous,
-            $headers,
-            $code
+            'PIREP has been cancelled, updates are not allowed'
         );
     }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'pirep-cancelled';
+    }
+
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    public function getErrorMetadata(): array
+    {
+        return [
+            'pirep_id' => $this->pirep->id,
+        ];
+    }
 }

app/Exceptions/Unauthenticated.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace App\Exceptions;
+
+class Unauthenticated extends HttpException
+{
+    public function __construct()
+    {
+        parent::__construct(
+            401,
+            'User not authenticated'
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'unauthenticated';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [];
+    }
+}

app/Exceptions/UserNotAtAirport.php

@@ -2,11 +2,51 @@
 
 namespace App\Exceptions;
 
-/**
- * Class UserNotAtAirport
- */
-class UserNotAtAirport extends InternalError
+use App\Models\Airport;
+use App\Models\User;
+
+class UserNotAtAirport extends HttpException
 {
-    public const FIELD = 'dpt_airport_id';
     public const MESSAGE = 'Pilot is not at the departure airport';
+
+    private $airport;
+    private $user;
+
+    public function __construct(User $user, Airport $airport)
+    {
+        $this->airport = $airport;
+        $this->user = $user;
+
+        parent::__construct(
+            400,
+            static::MESSAGE
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'user-not-at-airport';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [
+            'airport_id' => $this->airport->id,
+            'user_id'    => $this->user->id,
+        ];
+    }
 }

app/Exceptions/UserPilotIdExists.php

@@ -2,8 +2,47 @@
 
 namespace App\Exceptions;
 
-class UserPilotIdExists extends InternalError
+use App\Models\User;
+
+class UserPilotIdExists extends HttpException
 {
-    public const FIELD = 'pilot_id';
     public const MESSAGE = 'A user with this pilot ID already exists';
+
+    private $user;
+
+    public function __construct(User $user)
+    {
+        $this->user = $user;
+
+        parent::__construct(
+            400,
+            static::MESSAGE
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'pilot-id-already-exists';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [
+            'user_id' => $this->user->id,
+        ];
+    }
 }