Merge pull request #156 from utopia-php/fix-catch-error-handler

fix: catch error handler
utopia-php · Dec 10, 2024 · 83b0628 · 83b0628
2 parents 45a5a2d + 68f5fb3
commit 83b0628
Show file tree

Hide file tree

Showing 4 changed files with 161 additions and 5 deletions.
diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml
@@ -12,5 +12,5 @@ jobs:
 
     - name: Run Linter
       run: |
-        docker run --rm -v $PWD:/app composer sh -c \
+        docker run --rm -v $PWD:/app composer:2.6 sh -c \
         "composer install --profile --ignore-platform-reqs && git config --global --add safe.directory /app && composer bench -- --progress=plain"
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -12,6 +12,6 @@ jobs:
 
     - name: Run CodeQL
       run: |
-        docker run --rm -v $PWD:/app composer sh -c \
+        docker run --rm -v $PWD:/app composer:2.6 sh -c \
         "composer install --profile --ignore-platform-reqs && composer check"
         
diff --git a/src/App.php b/src/App.php
@@ -800,7 +800,12 @@ private function runInternal(Request $request, Response $response): static
                         self::setResource('error', function () use ($e) {
                             return $e;
                         });
-                        \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams()));
+                        try {
+                            $arguments = $this->getArguments($error, [], $request->getParams());
+                            \call_user_func_array($error->getAction(), $arguments);
+                        } catch (\Throwable $e) {
+                            throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e);
+                        }
                     }
                 }
             }
@@ -838,7 +843,12 @@ private function runInternal(Request $request, Response $response): static
                         self::setResource('error', function () use ($e) {
                             return $e;
                         });
-                        \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams()));
+                        try {
+                            $arguments = $this->getArguments($error, [], $request->getParams());
+                            \call_user_func_array($error->getAction(), $arguments);
+                        } catch (\Throwable $e) {
+                            throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e);
+                        }
                     }
                 }
             }
@@ -848,7 +858,12 @@ private function runInternal(Request $request, Response $response): static
                     self::setResource('error', function () {
                         return new Exception('Not Found', 404);
                     });
-                    \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams()));
+                    try {
+                        $arguments = $this->getArguments($error, [], $request->getParams());
+                        \call_user_func_array($error->getAction(), $arguments);
+                    } catch (\Throwable $e) {
+                        throw new Exception('Error handler had an error: ' . $e->getMessage(), 500, $e);
+                    }
                 }
             }
         }

diff --git a/tests/AppTest.php b/tests/AppTest.php
@@ -659,4 +659,145 @@ public function testWildcardRoute(): void
         $_SERVER['REQUEST_METHOD'] = $method;
         $_SERVER['REQUEST_URI'] = $uri;
     }
+
+    public function testErrorHandlerFailure(): void
+    {
+        $this->app
+            ->error()
+            ->inject('error')
+            ->action(function ($error) {
+                throw new \Exception('Error handler failed');
+            });
+
+        $route = new Route('GET', '/path');
+        $route
+            ->action(function () {
+                throw new \Exception('Route action failed');
+            });
+
+        try {
+            $this->app->execute($route, new Request(), new Response());
+            $this->fail('Should have thrown an exception');
+        } catch (\Exception $e) {
+            $this->assertEquals('Error handler had an error: Error handler failed', $e->getMessage());
+            $this->assertEquals(500, $e->getCode());
+            $this->assertInstanceOf(\Exception::class, $e->getPrevious());
+            $this->assertEquals('Error handler failed', $e->getPrevious()->getMessage());
+        }
+    }
+
+    public function testOptionsHandlerFailure(): void
+    {
+        $this->app
+            ->error()
+            ->inject('error')
+            ->action(function ($error) {
+                throw new \Exception('Options error handler failed');
+            });
+
+        // Set up an options handler that throws
+        App::options()
+            ->action(function () {
+                throw new \Exception('Options handler failed');
+            });
+
+        $request = new UtopiaRequestTest();
+        $request->setMethod('OPTIONS');
+
+        try {
+            $this->app->run($request, new Response());
+            $this->fail('Should have thrown an exception');
+        } catch (\Exception $e) {
+            $this->assertEquals('Error handler had an error: Options error handler failed', $e->getMessage());
+            $this->assertEquals(500, $e->getCode());
+        }
+    }
+
+    public function testNotFoundErrorHandlerFailure(): void
+    {
+        // Set up error handler that throws for 404 cases
+        $this->app
+            ->error()
+            ->action(function () {
+                throw new \Exception('404 error handler failed');
+            });
+
+        $request = new UtopiaRequestTest();
+        $request->setMethod('GET');
+        $request->setURI('/nonexistent-path');
+
+        try {
+            $this->app->run($request, new Response());
+            $this->fail('Should have thrown an exception');
+        } catch (\Exception $e) {
+            $this->assertEquals('Error handler had an error: 404 error handler failed', $e->getMessage());
+            $this->assertEquals(500, $e->getCode());
+            $this->assertInstanceOf(\Exception::class, $e->getPrevious());
+            $this->assertEquals('404 error handler failed', $e->getPrevious()->getMessage());
+        }
+    }
+
+    public function testGroupErrorHandlerFailure(): void
+    {
+        // Set up group-specific error handler that throws
+        $this->app
+            ->error()
+            ->groups(['api'])
+            ->action(function () {
+                throw new \Exception('Group error handler failed');
+            });
+
+        $route = new Route('GET', '/api/test');
+        $route
+            ->groups(['api'])
+            ->action(function () {
+                throw new \Exception('Route action failed');
+            });
+
+        try {
+            $this->app->execute($route, new Request(), new Response());
+            $this->fail('Should have thrown an exception');
+        } catch (\Exception $e) {
+            $this->assertEquals('Error handler had an error: Group error handler failed', $e->getMessage());
+            $this->assertEquals(500, $e->getCode());
+            $this->assertInstanceOf(\Exception::class, $e->getPrevious());
+            $this->assertEquals('Group error handler failed', $e->getPrevious()->getMessage());
+        }
+    }
+
+    public function testErrorHandlerChaining(): void
+    {
+        // Set up multiple error handlers to test chaining behavior
+        $this->app
+            ->error()
+            ->groups(['api'])
+            ->action(function () {
+                throw new \Exception('First error handler failed');
+            });
+
+        $this->app
+            ->error()
+            ->action(function () {
+                throw new \Exception('Second error handler failed');
+            });
+
+        $route = new Route('GET', '/api/test');
+        $route
+            ->groups(['api'])
+            ->action(function () {
+                throw new \Exception('Original error');
+            });
+
+        try {
+            $this->app->execute($route, new Request(), new Response());
+            $this->fail('Should have thrown an exception');
+        } catch (\Exception $e) {
+            $this->assertEquals('Error handler had an error: First error handler failed', $e->getMessage());
+            $this->assertEquals(500, $e->getCode());
+
+            // Verify the error chain
+            $this->assertInstanceOf(\Exception::class, $e->getPrevious());
+            $this->assertEquals('First error handler failed', $e->getPrevious()->getMessage());
+        }
+    }
 }