Merge pull request #28 from digiaonline/fix-exception-handling

Fix exception handling
digiaonline · Nov 24, 2017 · e8df268 · e8df268
2 parents baffc37 + d0b92ac
commit e8df268
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 8 deletions.
diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php
@@ -8,7 +8,6 @@
 use Illuminate\Http\JsonResponse;
 use Illuminate\Validation\ValidationException;
 use Laravel\Lumen\Exceptions\Handler as ExceptionHandler;
-use League\Flysystem\FileNotFoundException;
 use Symfony\Component\HttpKernel\Exception\HttpException;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
@@ -31,13 +30,21 @@ class Handler extends ExceptionHandler
         ValidationException::class,
     ];
 
+    /**
+     * @var array exceptions that should be mapped to NotFoundHttpException
+     */
+    protected $notFoundExceptions = [
+        \League\Flysystem\FileNotFoundException::class,
+        \League\Glide\Filesystem\FileNotFoundException::class,
+    ];
+
     /**
      * @inheritdoc
      */
     public function render($request, Exception $e)
     {
-        // Change FileNotFoundException to NotFoundHttpException so we get the correct HTTP status code 
-        if ($e instanceof FileNotFoundException) {
+        // Change some exceptions to NotFoundHttpException so we get the correct HTTP status code
+        if (in_array(get_class($e), $this->notFoundExceptions)) {
             $e = new NotFoundHttpException($e->getMessage(), $e);
         }
 
@@ -51,7 +58,21 @@ public function render($request, Exception $e)
             $data['trace'] = $e->getTrace();
         }
 
-        return new JsonResponse($data);
+        return new JsonResponse($data, $this->determineStatusCode($e));
+    }
+
+    /**
+     * @param Exception $e
+     *
+     * @return int
+     */
+    private function determineStatusCode(\Exception $e): int
+    {
+        if ($e instanceof HttpException) {
+            return $e->getStatusCode();
+        }
+
+        return 500;
     }
 
 }
diff --git a/tests/Exceptions/HandlerTest.php b/tests/Exceptions/HandlerTest.php
@@ -2,7 +2,6 @@
 
 namespace Nord\ImageManipulationService\Tests\Exceptions;
 
-use League\Flysystem\FileNotFoundException;
 use Nord\ImageManipulationService\Exceptions\Handler;
 use Nord\ImageManipulationService\Tests\TestCase;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -15,16 +14,32 @@ class HandlerTest extends TestCase
 {
 
     /**
-     * Tests that the exception is converted
+     * Tests that some exception is converted and that the correct HTTP status code is used
+     *
+     * @dataProvider fileNotFoundExceptionProvider
+     *
+     * @param \Exception $e
      */
-    public function testFileNotFoundException()
+    public function testFileNotFoundException(\Exception $e)
     {
         $handler  = new Handler();
-        $response = $handler->render(null, new FileNotFoundException('/'));
+        $response = $handler->render(null, $e);
 
         $data = json_decode((string)$response->getContent(), true);
 
         $this->assertEquals(NotFoundHttpException::class, $data['exception']);
+        $this->assertEquals(404, $response->getStatusCode());
+    }
+
+    /**
+     * Tests that HTTP 500 is returned normally when an exception is rendered
+     */
+    public function testNormalException()
+    {
+        $handler  = new Handler();
+        $response = $handler->render(null, new \InvalidArgumentException('Some error'));
+
+        $this->assertEquals(500, $response->getStatusCode());
     }
 
     /**
@@ -46,4 +61,15 @@ public function testHandleRender()
         $this->assertEquals(env('APP_DEBUG'), array_key_exists('trace', $data));
     }
 
+    /**
+     * @return array
+     */
+    public function fileNotFoundExceptionProvider(): array
+    {
+        return [
+            [new \League\Flysystem\FileNotFoundException('/')],
+            [new \League\Glide\Filesystem\FileNotFoundException('/')],
+        ];
+    }
+
 }