From 3a7c3101be1e678995b49fd83ab45d1fef855bc6 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 11 Apr 2022 15:46:03 +0100 Subject: [PATCH 1/9] Add default exception handler --- src/App.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/App.php b/src/App.php index 6bacb570..aed96bb8 100755 --- a/src/App.php +++ b/src/App.php @@ -606,6 +606,20 @@ public function execute(Route $route, Request $request): self } } } catch (\Throwable $e) { + if (self::$errors === ['*' => []]) { // If no error handler are set + $response = $this->getResources(['response'])["response"]; + $response->setStatusCode(500); + $response->json([ + 'message' => $e->getMessage(), + 'stacktrace' => $e->getTrace() + ]); + + \fwrite(STDERR, "\033[31mException: " . $e->getMessage() . "\033[0m\n"); + \fwrite(STDERR, "Stacktrace: \n" . $e->getTraceAsString() . "\n"); + + return $this; + } + foreach ($groups as $group) { if (isset(self::$errors[$group])) { foreach (self::$errors[$group] as $error) { // Group error hooks From 2e62abdf8d7c06571e3265e9b916f2a5c1b2b4af Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 3 May 2022 15:45:26 +0100 Subject: [PATCH 2/9] Update App.php --- src/App.php | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/App.php b/src/App.php index aed96bb8..eacb530a 100755 --- a/src/App.php +++ b/src/App.php @@ -606,18 +606,20 @@ public function execute(Route $route, Request $request): self } } } catch (\Throwable $e) { - if (self::$errors === ['*' => []]) { // If no error handler are set - $response = $this->getResources(['response'])["response"]; - $response->setStatusCode(500); - $response->json([ - 'message' => $e->getMessage(), - 'stacktrace' => $e->getTrace() - ]); - - \fwrite(STDERR, "\033[31mException: " . $e->getMessage() . "\033[0m\n"); - \fwrite(STDERR, "Stacktrace: \n" . $e->getTraceAsString() . "\n"); - - return $this; + if (empty(self::$errors['*'])) { // If no error handler is set then add a default one. + self::$errors['*'][] = [ + 'callback' => function (\Throwable $error, Response $response) { + $response->setStatusCode(500); + $response->json([ + 'message' => $error->getMessage(), + 'stacktrace' => $error->getTrace() + ]); + $response->send(); + \fwrite(STDERR, "\033[31mException: " . $error->getMessage() . "\033[0m\n"); + \fwrite(STDERR, "Stacktrace: \n" . $error->getTraceAsString() . "\n"); + }, + 'resources' => ['error', 'response'] + ]; } foreach ($groups as $group) { From d2decd1668589016205d86660082fecf8500082a Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 9 May 2022 10:58:14 +0100 Subject: [PATCH 3/9] Update App.php --- src/App.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/App.php b/src/App.php index eacb530a..ed88eeff 100755 --- a/src/App.php +++ b/src/App.php @@ -614,7 +614,6 @@ public function execute(Route $route, Request $request): self 'message' => $error->getMessage(), 'stacktrace' => $error->getTrace() ]); - $response->send(); \fwrite(STDERR, "\033[31mException: " . $error->getMessage() . "\033[0m\n"); \fwrite(STDERR, "Stacktrace: \n" . $error->getTraceAsString() . "\n"); }, From 839bca0ced2d959c3debc053c49a040f74475773 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 10 May 2022 11:52:09 +0100 Subject: [PATCH 4/9] Fix tests --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 62421710..ef34aadb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,4 @@ -dist: xenial +dist: focal arch: - amd64 From 404043d9219d6b748764fdf4f1e5b5241b4268e8 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 10 May 2022 13:54:23 +0100 Subject: [PATCH 5/9] Fix tests --- tests/AppTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/AppTest.php b/tests/AppTest.php index 1eb2cab4..e762afbf 100755 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -310,6 +310,13 @@ public function testSetRoute() { $this->assertEquals($this->app->getRoute(), $route); } + + /** + * @runInSeparateProcess + * + * We need to use this because PHPUnit prints to the output while creating headers, please refer to this stackoverflow issue for more details: + * @link https://stackoverflow.com/questions/13875761/phpunit-output-with-header-exceptions-stderr-no-result + */ public function testRun() { // Test head requests @@ -339,6 +346,12 @@ public function testRun() $this->assertStringNotContainsString('HELLO', $result); } + /** + * @runInSeparateProcess + * + * We need to use this because PHPUnit prints to the output while creating headers, please refer to this stackoverflow issue for more details: + * @link https://stackoverflow.com/questions/13875761/phpunit-output-with-header-exceptions-stderr-no-result + */ public function testRunAlias() { // Test head requests From 9f3ae0a394ba34b7b3ed64408bdd481e0c75dc50 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Tue, 10 May 2022 14:39:29 +0100 Subject: [PATCH 6/9] Add Tests --- tests/AppTest.php | 1 - tests/e2e/Client.php | 4 ---- tests/e2e/ResponseTest.php | 6 ++++++ tests/e2e/server.php | 7 +++++++ 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/AppTest.php b/tests/AppTest.php index e762afbf..41f38d15 100755 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -338,7 +338,6 @@ public function testRun() $this->app->run(new Request(), new Response()); $result = \ob_get_contents(); \ob_end_clean(); - $_SERVER['REQUEST_METHOD'] = $method; $_SERVER['REQUEST_URI'] = $uri; diff --git a/tests/e2e/Client.php b/tests/e2e/Client.php index 19a70d8e..0db6a5ce 100644 --- a/tests/e2e/Client.php +++ b/tests/e2e/Client.php @@ -83,10 +83,6 @@ public function call(string $method, string $path = '', array $headers = [], arr $responseHeaders['status-code'] = $responseStatus; - if ($responseStatus === 500) { - echo 'Server error('.$method.': '.$path.'. Params: '.json_encode($params).'): '.json_encode($responseBody)."\n"; - } - return [ 'headers' => $responseHeaders, 'body' => $responseBody diff --git a/tests/e2e/ResponseTest.php b/tests/e2e/ResponseTest.php index a23b15a3..bdd3faec 100644 --- a/tests/e2e/ResponseTest.php +++ b/tests/e2e/ResponseTest.php @@ -39,4 +39,10 @@ public function testRedirect() $response = $this->client->call(Client::METHOD_GET, '/redirect'); $this->assertEquals('Hello World!', $response['body']); } + + public function testException() + { + $response = $this->client->call(Client::METHOD_GET, '/exception'); + $this->assertEquals(500, $response['headers']['status-code']); + } } diff --git a/tests/e2e/server.php b/tests/e2e/server.php index e9d900b8..d4402ebe 100644 --- a/tests/e2e/server.php +++ b/tests/e2e/server.php @@ -34,6 +34,13 @@ $response->redirect('/'); }); +App::get('/exception') + ->inject('response') + ->action(function($response) { + /** @var Utopia/Response $response */ + throw new Exception('Exception!'); + }); + $request = new Request(); $response = new Response(); From 14d9c24f5662acba2880278ec58e5ecb24fc334c Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Wed, 11 May 2022 10:42:43 +0100 Subject: [PATCH 7/9] Add handled exception tests --- tests/e2e/ResponseTest.php | 6 ++++++ tests/e2e/server.php | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/tests/e2e/ResponseTest.php b/tests/e2e/ResponseTest.php index bdd3faec..2190a818 100644 --- a/tests/e2e/ResponseTest.php +++ b/tests/e2e/ResponseTest.php @@ -45,4 +45,10 @@ public function testException() $response = $this->client->call(Client::METHOD_GET, '/exception'); $this->assertEquals(500, $response['headers']['status-code']); } + + public function testHandledException() + { + $response = $this->client->call(Client::METHOD_GET, '/handledException'); + $this->assertEquals('Handled Exception.', $response['body']); + } } diff --git a/tests/e2e/server.php b/tests/e2e/server.php index d4402ebe..ad91aa6b 100644 --- a/tests/e2e/server.php +++ b/tests/e2e/server.php @@ -41,6 +41,19 @@ throw new Exception('Exception!'); }); +App::get('/handledException') + ->inject('response') + ->action(function($response) { + /** @var Utopia/Response $response */ + + App::error(function ($error, $response) { + /** @var Utopia/Response $response */ + $response->send('Handled Exception.'); + }, ['error', 'response']); + + throw new Exception('Exception!'); + }); + $request = new Request(); $response = new Response(); From ca61da36d04046ae1baf799ed8e3019271eab659 Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Mon, 1 Aug 2022 09:44:14 +0100 Subject: [PATCH 8/9] Update AppTest.php --- tests/AppTest.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/AppTest.php b/tests/AppTest.php index 45e4f788..5f5e6d54 100755 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -454,6 +454,7 @@ public function testRun() $this->app->run(new Request(), new Response()); $result = \ob_get_contents(); \ob_end_clean(); + $_SERVER['REQUEST_METHOD'] = $method; $_SERVER['REQUEST_URI'] = $uri; @@ -461,12 +462,6 @@ public function testRun() $this->assertStringNotContainsString('HELLO', $result); } - /** - * @runInSeparateProcess - * - * We need to use this because PHPUnit prints to the output while creating headers, please refer to this stackoverflow issue for more details: - * @link https://stackoverflow.com/questions/13875761/phpunit-output-with-header-exceptions-stderr-no-result - */ public function testRunAlias() { // Test head requests From f421cbea12729e23145e69aa7918a29d881d7aec Mon Sep 17 00:00:00 2001 From: Bradley Schofield Date: Thu, 4 Aug 2022 13:08:29 +0100 Subject: [PATCH 9/9] Update App.php --- src/App.php | 52 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/src/App.php b/src/App.php index 1c806395..38213b85 100755 --- a/src/App.php +++ b/src/App.php @@ -575,21 +575,8 @@ public function execute(Route $route, Request $request): self } } } - } catch (\Throwable $e) { - if (empty(self::$errors['*'])) { // If no error handler is set then add a default one. - self::$errors['*'][] = [ - 'callback' => function (\Throwable $error, Response $response) { - $response->setStatusCode(500); - $response->json([ - 'message' => $error->getMessage(), - 'stacktrace' => $error->getTrace() - ]); - \fwrite(STDERR, "\033[31mException: " . $error->getMessage() . "\033[0m\n"); - \fwrite(STDERR, "Stacktrace: \n" . $error->getTraceAsString() . "\n"); - }, - 'resources' => ['error', 'response'] - ]; - } + } catch (\Throwable $e) {; + $errorHandled = false; foreach ($groups as $group) { foreach (self::$errors as $error) { // Group error hooks @@ -601,6 +588,7 @@ public function execute(Route $route, Request $request): self try { $arguments = $this->getArguments($error, $values, $request->getParams()); \call_user_func_array($error->getAction(), $arguments); + $errorHandled = true; } catch (\Throwable $e) { throw new Exception('Error handler had an error', 0, $e); } @@ -617,11 +605,23 @@ public function execute(Route $route, Request $request): self try { $arguments = $this->getArguments($error, $values, $request->getParams()); \call_user_func_array($error->getAction(), $arguments); + $errorHandled = true; } catch (\Throwable $e) { throw new Exception('Error handler had an error', 0, $e); } } } + + if (!$errorHandled) { + $response = $this->getResource('response'); + $response->setStatusCode(500); + $response->json([ + 'message' => $e->getMessage(), + 'stacktrace' => $e->getTrace() + ]); + \fwrite(STDERR, "\033[31mException: " . $e->getMessage() . "\033[0m\n"); + \fwrite(STDERR, "Stacktrace: \n" . $e->getTraceAsString() . "\n"); + } } return $this; @@ -743,6 +743,7 @@ public function run(Request $request, Response $response): self } } } catch (\Throwable $e) { + $errorHandled = false; foreach (self::$errors as $error) { // Global error hooks /** @var Hook $error */ if(in_array('*', $error->getGroups())) { @@ -750,10 +751,22 @@ public function run(Request $request, Response $response): self return $e; }); \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams())); + $errorHandled = true; } } + + if (!$errorHandled) { + $response->setStatusCode(500); + $response->json([ + 'message' => $e->getMessage(), + 'stacktrace' => $e->getTrace() + ]); + \fwrite(STDERR, "\033[31mException: " . $e->getMessage() . "\033[0m\n"); + \fwrite(STDERR, "Stacktrace: \n" . $e->getTraceAsString() . "\n"); + } } } else { + $errorHandled = false; foreach (self::$errors as $error) { // Global error hooks /** @var Hook $error */ if(in_array('*', $error->getGroups())) { @@ -761,8 +774,17 @@ public function run(Request $request, Response $response): self return new Exception('Not Found', 404); }); \call_user_func_array($error->getAction(), $this->getArguments($error, [], $request->getParams())); + $errorHandled = true; } } + + if (!$errorHandled) { + $response->setStatusCode(404); + $response->json([ + 'message' => 'Not Found' + ]); + \fwrite(STDERR, "\033[31mException: " . 'Route not found' . "\033[0m\n"); + } } return $this;