From a2c3bcac947456fed24aca4484b637ce4747dbd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@clue.engineering>
Date: Sun, 14 Feb 2021 20:26:08 +0100
Subject: [PATCH 1/3] Catch all Exceptions and rejected promises

---
 examples/index.php  |   4 +
 src/App.php         |  56 +++++++++-
 tests/AppTest.php   | 244 ++++++++++++++++++++++++++++++++++++++++++++
 tests/acceptance.sh |   1 +
 4 files changed, 303 insertions(+), 2 deletions(-)

diff --git a/examples/index.php b/examples/index.php
index 114d707..7e002ee 100644
--- a/examples/index.php
+++ b/examples/index.php
@@ -129,5 +129,9 @@
     );
 });
 
+$app->get('/error', function () {
+    return null;
+});
+
 $app->run();
 $loop->run();
diff --git a/src/App.php b/src/App.php
index d2ac668..e189f1f 100644
--- a/src/App.php
+++ b/src/App.php
@@ -300,7 +300,11 @@ private function sendResponse(ServerRequestInterface $request, ResponseInterface
     /**
      * @param ServerRequestInterface $request
      * @param Dispatcher $dispatcher
-     * @return ResponseInterface|PromiseInterface<ResponseInterface>
+     * @return ResponseInterface|PromiseInterface<ResponseInterface,void>
+     *     Returns a response or a Promise which eventually fulfills with a
+     *     response. This method never throws or resolves a rejected promise.
+     *     If the request can not be routed or the handler fails, it will be
+     *     turned into a valid error response before returning.
      */
     private function handleRequest(ServerRequestInterface $request, Dispatcher $dispatcher)
     {
@@ -326,7 +330,29 @@ private function handleRequest(ServerRequestInterface $request, Dispatcher $disp
                     $request = $request->withAttribute($key, rawurldecode($value));
                 }
 
-                return $handler($request);
+                try {
+                    $response = $handler($request);
+                } catch (\Throwable $e) {
+                    return $this->errorHandlerException($e);
+                }
+
+                if ($response instanceof ResponseInterface) {
+                    return $response;
+                } elseif ($response instanceof PromiseInterface) {
+                    return $response->then(function ($response) {
+                        if (!$response instanceof ResponseInterface) {
+                            return $this->errorHandlerResponse($response);
+                        }
+                        return $response;
+                    }, function ($e) {
+                        if (!$e instanceof \Throwable) {
+                            $e = new \UnexpectedValueException('Handler rejected with ' . $this->describeType($e));
+                        }
+                        return $this->errorHandlerException($e);
+                    });
+                } else {
+                    return $this->errorHandlerResponse($response);
+                }
         }
     } // @codeCoverageIgnore
 
@@ -403,4 +429,30 @@ private function errorMethodNotAllowed(ServerRequestInterface $request): Respons
             implode(', ', $request->getAttribute('allowed'))
         )->withHeader('Allowed', implode(', ', $request->getAttribute('allowed')));
     }
+
+    private function errorHandlerException(\Throwable $e): ResponseInterface
+    {
+        return $this->error(
+            500,
+            'Uncaught <code>' . get_class($e) . '</code> from handler: ' . $e->getMessage()
+        );
+    }
+
+    private function errorHandlerResponse($value): ResponseInterface
+    {
+        return $this->error(
+            500,
+            'Handler returned invalid value (<code>' . $this->describeType($value) . '</code>)'
+        );
+    }
+
+    private function describeType($value): string
+    {
+        if ($value === null) {
+            return 'null';
+        } elseif (\is_scalar($value) && !\is_string($value)) {
+            return \var_export($value, true);
+        }
+        return \is_object($value) ? \get_class($value) : \gettype($value);
+    }
 }
diff --git a/tests/AppTest.php b/tests/AppTest.php
index 256c0fb..870b528 100644
--- a/tests/AppTest.php
+++ b/tests/AppTest.php
@@ -9,6 +9,7 @@
 use React\EventLoop\LoopInterface;
 use React\Http\Message\Response;
 use React\Http\Message\ServerRequest;
+use React\Promise\PromiseInterface;
 
 class AppTest extends TestCase
 {
@@ -403,6 +404,46 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsResponseFrom
         $this->assertEquals("OK\n", (string) $response->getBody());
     }
 
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithResponseWhenHandlerReturnsPromiseWhichFulfillsWithResponse()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = function () {
+            return \React\Promise\resolve(new Response(
+                200,
+                [
+                    'Content-Type' => 'text/html'
+                ],
+                "OK\n"
+            ));
+        };
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $promise = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $promise = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var PromiseInterface $promise */
+        $this->assertInstanceOf(PromiseInterface::class, $promise);
+
+        $response = null;
+        $promise->then(function ($value) use (&$response) {
+            $response = $value;
+        });
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(200, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("OK\n", (string) $response->getBody());
+    }
+
     public function testHandleRequestWithDispatcherWithRouteFoundAndRouteVariablesReturnsResponseFromHandlerWithRouteVariablesAssignedAsRequestAttributes()
     {
         $loop = $this->createMock(LoopInterface::class);
@@ -437,6 +478,209 @@ public function testHandleRequestWithDispatcherWithRouteFoundAndRouteVariablesRe
         $this->assertEquals("Hello alice\n", (string) $response->getBody());
     }
 
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenHandlerThrowsException()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = function () {
+            throw new RuntimeException('Foo');
+        };
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $response = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from handler: Foo\n", (string) $response->getBody());
+    }
+
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsPromiseWhichRejectsWithException()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = function () {
+            return \React\Promise\reject(new RuntimeException('Foo'));
+        };
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $promise = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $promise = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var PromiseInterface $promise */
+        $this->assertInstanceOf(PromiseInterface::class, $promise);
+
+        $response = null;
+        $promise->then(function ($value) use (&$response) {
+            $response = $value;
+        });
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from handler: Foo\n", (string) $response->getBody());
+    }
+
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsPromiseWhichRejectsWithNull()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = function () {
+            return \React\Promise\reject('');
+        };
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $promise = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $promise = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var PromiseInterface $promise */
+        $this->assertInstanceOf(PromiseInterface::class, $promise);
+
+        $response = null;
+        $promise->then(function ($value) use (&$response) {
+            $response = $value;
+        });
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>UnexpectedValueException</code> from handler: Handler rejected with string\n", (string) $response->getBody());
+    }
+
+
+    public function provideInvalidReturnValue()
+    {
+        return [
+            [
+                null,
+                'null',
+            ],
+            [
+                'hello',
+                'string'
+            ],
+            [
+                42,
+                '42'
+            ],
+            [
+                1.0,
+                '1.0'
+            ],
+            [
+                false,
+                'false'
+            ],
+            [
+                [],
+                'array'
+            ],
+            [
+                (object)[],
+                'stdClass'
+            ],
+            [
+                tmpfile(),
+                'resource'
+            ]
+        ];
+    }
+
+    /**
+     * @dataProvider provideInvalidReturnValue
+     * @param mixed $value
+     * @param string $name
+     */
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenHandlerReturnsWrongValue($value, $name)
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = function () use ($value) {
+            return $value;
+        };
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $response = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): Handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
+    }
+
+    /**
+     * @dataProvider provideInvalidReturnValue
+     * @param mixed $value
+     * @param string $name
+     */
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsPromiseWhichFulfillsWithWrongValue($value, $name)
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = function () use ($value) {
+            return \React\Promise\resolve($value);
+        };
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $promise = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $promise = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var PromiseInterface $promise */
+        $this->assertInstanceOf(PromiseInterface::class, $promise);
+
+        $response = null;
+        $promise->then(function ($value) use (&$response) {
+            $response = $value;
+        });
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): Handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
+    }
+
     public function testLogRequestResponsePrintsRequestLogWithCurrentDateAndTime()
     {
         $loop = $this->createMock(LoopInterface::class);
diff --git a/tests/acceptance.sh b/tests/acceptance.sh
index bf1c9a2..ba73085 100755
--- a/tests/acceptance.sh
+++ b/tests/acceptance.sh
@@ -17,6 +17,7 @@ out=$(curl -v $base/ 2>&1);         match "HTTP/.* 200" && match -iv "Content-Ty
 out=$(curl -v $base/invalid 2>&1);  match "HTTP/.* 404" && match -iP "Content-Type: text/html[\r\n]"
 out=$(curl -v $base// 2>&1);        match "HTTP/.* 404"
 out=$(curl -v $base/ 2>&1 -X POST); match "HTTP/.* 405"
+out=$(curl -v $base/error 2>&1);    match "HTTP/.* 500" && match -iP "Content-Type: text/html[\r\n]"
 
 out=$(curl -v $base/uri 2>&1);                          match "HTTP/.* 200" && match "$base/uri"
 out=$(curl -v $base/uri/ 2>&1);                         match "HTTP/.* 200" && match "$base/uri/"

From d5da3bd835b7bc2ab3e01b5f456757806d4baf69 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@clue.engineering>
Date: Tue, 16 Feb 2021 15:24:38 +0100
Subject: [PATCH 2/3] Include name of request handler class/function on error

---
 composer.json                     |  15 ++--
 src/App.php                       |  33 ++++++---
 tests/AppTest.php                 | 115 ++++++++++++++++++++++++++++--
 tests/FilesystemHandlerTest.php   |   7 +-
 tests/Stub/InvalidHandlerStub.php |  21 ++++++
 5 files changed, 167 insertions(+), 24 deletions(-)
 create mode 100644 tests/Stub/InvalidHandlerStub.php

diff --git a/composer.json b/composer.json
index e7c7332..b73ee20 100644
--- a/composer.json
+++ b/composer.json
@@ -10,11 +10,6 @@
             "email": "christian@clue.engineering"
         }
     ],
-    "autoload": {
-        "psr-4": {
-            "Frugal\\": "src/"
-        }
-    },
     "require": {
         "php": ">=7.1",
         "nikic/fast-route": "^1.3",
@@ -23,5 +18,15 @@
     },
     "require-dev": {
         "phpunit/phpunit": "^9.5 || ^7.5"
+    },
+    "autoload": {
+        "psr-4": {
+            "Frugal\\": "src/"
+        }
+    },
+    "autoload-dev": {
+        "psr-4": {
+            "Frugal\\Tests\\": "tests/"
+        }
     }
 }
diff --git a/src/App.php b/src/App.php
index e189f1f..9ea68de 100644
--- a/src/App.php
+++ b/src/App.php
@@ -333,25 +333,25 @@ private function handleRequest(ServerRequestInterface $request, Dispatcher $disp
                 try {
                     $response = $handler($request);
                 } catch (\Throwable $e) {
-                    return $this->errorHandlerException($e);
+                    return $this->errorHandlerException($e, $handler);
                 }
 
                 if ($response instanceof ResponseInterface) {
                     return $response;
                 } elseif ($response instanceof PromiseInterface) {
-                    return $response->then(function ($response) {
+                    return $response->then(function ($response) use ($handler) {
                         if (!$response instanceof ResponseInterface) {
-                            return $this->errorHandlerResponse($response);
+                            return $this->errorHandlerResponse($response, $handler);
                         }
                         return $response;
-                    }, function ($e) {
+                    }, function ($e) use ($handler) {
                         if (!$e instanceof \Throwable) {
                             $e = new \UnexpectedValueException('Handler rejected with ' . $this->describeType($e));
                         }
-                        return $this->errorHandlerException($e);
+                        return $this->errorHandlerException($e, $handler);
                     });
                 } else {
-                    return $this->errorHandlerResponse($response);
+                    return $this->errorHandlerResponse($response, $handler);
                 }
         }
     } // @codeCoverageIgnore
@@ -430,19 +430,19 @@ private function errorMethodNotAllowed(ServerRequestInterface $request): Respons
         )->withHeader('Allowed', implode(', ', $request->getAttribute('allowed')));
     }
 
-    private function errorHandlerException(\Throwable $e): ResponseInterface
+    private function errorHandlerException(\Throwable $e, callable $handler): ResponseInterface
     {
         return $this->error(
             500,
-            'Uncaught <code>' . get_class($e) . '</code> from handler: ' . $e->getMessage()
+            'Uncaught <code>' . \get_class($e) . '</code> from ' . \lcfirst($this->describeHandler($handler)) . ': ' . $e->getMessage()
         );
     }
 
-    private function errorHandlerResponse($value): ResponseInterface
+    private function errorHandlerResponse($value, callable $handler): ResponseInterface
     {
         return $this->error(
             500,
-            'Handler returned invalid value (<code>' . $this->describeType($value) . '</code>)'
+            $this->describeHandler($handler) . ' returned invalid value (<code>' . $this->describeType($value) . '</code>)'
         );
     }
 
@@ -455,4 +455,17 @@ private function describeType($value): string
         }
         return \is_object($value) ? \get_class($value) : \gettype($value);
     }
+
+    private function describeHandler(callable $handler): string
+    {
+        if (\is_object($handler) && !$handler instanceof \Closure) {
+            return '<code>' . \get_class($handler) . '</code>';
+        } elseif (\is_string($handler)) {
+            return '<code>' . $handler . '()</code>';
+        } elseif (\is_array($handler)) {
+            return '<code>' . (\is_string($handler[0]) ? $handler[0] : \get_class($handler[0])) . '::' . $handler[1] . '()</code>';
+        }
+
+        return 'Request handler';
+    }
 }
diff --git a/tests/AppTest.php b/tests/AppTest.php
index 870b528..21efdb6 100644
--- a/tests/AppTest.php
+++ b/tests/AppTest.php
@@ -1,5 +1,7 @@
 <?php
 
+namespace Frugal\Tests;
+
 use FastRoute\Dispatcher;
 use FastRoute\RouteCollector;
 use Frugal\App;
@@ -10,6 +12,9 @@
 use React\Http\Message\Response;
 use React\Http\Message\ServerRequest;
 use React\Promise\PromiseInterface;
+use ReflectionMethod;
+use ReflectionProperty;
+use Frugal\Tests\Stub\InvalidHandlerStub;
 
 class AppTest extends TestCase
 {
@@ -486,7 +491,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $request = new ServerRequest('GET', 'http://localhost/users');
 
         $handler = function () {
-            throw new RuntimeException('Foo');
+            throw new \RuntimeException('Foo');
         };
 
         $dispatcher = $this->createMock(Dispatcher::class);
@@ -501,7 +506,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from handler: Foo\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from request handler: Foo\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsPromiseWhichRejectsWithException()
@@ -512,7 +517,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
         $request = new ServerRequest('GET', 'http://localhost/users');
 
         $handler = function () {
-            return \React\Promise\reject(new RuntimeException('Foo'));
+            return \React\Promise\reject(new \RuntimeException('Foo'));
         };
 
         $dispatcher = $this->createMock(Dispatcher::class);
@@ -535,7 +540,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from handler: Foo\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from request handler: Foo\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsPromiseWhichRejectsWithNull()
@@ -569,7 +574,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Uncaught <code>UnexpectedValueException</code> from handler: Handler rejected with string\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>UnexpectedValueException</code> from request handler: Handler rejected with string\n", (string) $response->getBody());
     }
 
 
@@ -639,7 +644,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Request handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
     }
 
     /**
@@ -678,7 +683,103 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Request handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
+    }
+
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenClassHandlerReturnsStringValue()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = new InvalidHandlerStub();
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $response = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub</code> returned invalid value (<code>null</code>)\n", (string) $response->getBody());
+    }
+
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenClassMethodHandlerReturnsStringValue()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = [new InvalidHandlerStub(), 'index'];
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $response = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub::index()</code> returned invalid value (<code>null</code>)\n", (string) $response->getBody());
+    }
+
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenStaticClassMethodHandlerReturnsStringValue()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = [InvalidHandlerStub::class, 'static'];
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $response = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub::static()</code> returned invalid value (<code>null</code>)\n", (string) $response->getBody());
+    }
+
+    public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenGlobalFunctionHandlerReturnsStringValue()
+    {
+        $loop = $this->createMock(LoopInterface::class);
+        $app = new App($loop);
+
+        $request = new ServerRequest('GET', 'http://localhost/users');
+
+        $handler = 'gettype';
+
+        $dispatcher = $this->createMock(Dispatcher::class);
+        $dispatcher->expects($this->once())->method('dispatch')->with('GET', '/users')->willReturn([\FastRoute\Dispatcher::FOUND, $handler, []]);
+
+        // $response = $app->handleRequest($request, $dispatcher);
+        $ref = new ReflectionMethod($app, 'handleRequest');
+        $ref->setAccessible(true);
+        $response = $ref->invoke($app, $request, $dispatcher);
+
+        /** @var ResponseInterface $response */
+        $this->assertInstanceOf(ResponseInterface::class, $response);
+        $this->assertEquals(500, $response->getStatusCode());
+        $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
+        $this->assertEquals("500 (Internal Server Error): <code>gettype()</code> returned invalid value (<code>string</code>)\n", (string) $response->getBody());
     }
 
     public function testLogRequestResponsePrintsRequestLogWithCurrentDateAndTime()
diff --git a/tests/FilesystemHandlerTest.php b/tests/FilesystemHandlerTest.php
index 3e8600f..6cd207c 100644
--- a/tests/FilesystemHandlerTest.php
+++ b/tests/FilesystemHandlerTest.php
@@ -1,9 +1,12 @@
 <?php
 
-use PHPUnit\Framework\TestCase;
+namespace Frugal\Tests;
+
 use Frugal\FilesystemHandler;
-use React\Http\Message\ServerRequest;
+use PHPUnit\Framework\TestCase;
 use Psr\Http\Message\ResponseInterface;
+use React\Http\Message\ServerRequest;
+use ReflectionMethod;
 
 class FilesystemHandlerTest extends TestCase
 {
diff --git a/tests/Stub/InvalidHandlerStub.php b/tests/Stub/InvalidHandlerStub.php
new file mode 100644
index 0000000..b1a538d
--- /dev/null
+++ b/tests/Stub/InvalidHandlerStub.php
@@ -0,0 +1,21 @@
+<?php
+
+namespace Frugal\Tests\Stub;
+
+class InvalidHandlerStub
+{
+    public function __invoke()
+    {
+        return null;
+    }
+
+    public function index()
+    {
+        return null;
+    }
+
+    public static function static()
+    {
+        return null;
+    }
+}

From c2441c6ad38a82e8c9cbb29f68e39ce4ea9a8f55 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@clue.engineering>
Date: Wed, 17 Feb 2021 07:46:37 +0100
Subject: [PATCH 3/3] Include file name and line on error

---
 examples/index.php  |  3 +++
 src/App.php         | 33 +++++++++++++++++++++++----------
 tests/AppTest.php   | 25 ++++++++++++++++---------
 tests/acceptance.sh |  3 ++-
 4 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/examples/index.php b/examples/index.php
index 7e002ee..ef3013a 100644
--- a/examples/index.php
+++ b/examples/index.php
@@ -130,6 +130,9 @@
 });
 
 $app->get('/error', function () {
+    throw new RuntimeException('Error');
+});
+$app->get('/error/null', function () {
     return null;
 });
 
diff --git a/src/App.php b/src/App.php
index 9ea68de..b39bbd3 100644
--- a/src/App.php
+++ b/src/App.php
@@ -345,10 +345,11 @@ private function handleRequest(ServerRequestInterface $request, Dispatcher $disp
                         }
                         return $response;
                     }, function ($e) use ($handler) {
-                        if (!$e instanceof \Throwable) {
-                            $e = new \UnexpectedValueException('Handler rejected with ' . $this->describeType($e));
+                        if ($e instanceof \Throwable) {
+                            return $this->errorHandlerException($e, $handler);
+                        } else {
+                            return $this->errorHandlerResponse(\React\Promise\reject($e), $handler);
                         }
-                        return $this->errorHandlerException($e, $handler);
                     });
                 } else {
                     return $this->errorHandlerResponse($response, $handler);
@@ -432,9 +433,11 @@ private function errorMethodNotAllowed(ServerRequestInterface $request): Respons
 
     private function errorHandlerException(\Throwable $e, callable $handler): ResponseInterface
     {
+        $where = ' (<code title="See ' . $e->getFile() . ' line ' . $e->getLine() . '">' . \basename($e->getFile()) . ':' . $e->getLine() . '</code>)';
+
         return $this->error(
             500,
-            'Uncaught <code>' . \get_class($e) . '</code> from ' . \lcfirst($this->describeHandler($handler)) . ': ' . $e->getMessage()
+            'Uncaught <code>' . \get_class($e) . '</code> from ' . \lcfirst($this->describeHandler($handler, false)) . $where . ': ' . $e->getMessage()
         );
     }
 
@@ -442,7 +445,7 @@ private function errorHandlerResponse($value, callable $handler): ResponseInterf
     {
         return $this->error(
             500,
-            $this->describeHandler($handler) . ' returned invalid value (<code>' . $this->describeType($value) . '</code>)'
+            $this->describeHandler($handler, true) . ' returned invalid value (<code>' . $this->describeType($value) . '</code>)'
         );
     }
 
@@ -456,16 +459,26 @@ private function describeType($value): string
         return \is_object($value) ? \get_class($value) : \gettype($value);
     }
 
-    private function describeHandler(callable $handler): string
+    private function describeHandler(callable $handler, bool $linkSourceFile): string
     {
         if (\is_object($handler) && !$handler instanceof \Closure) {
-            return '<code>' . \get_class($handler) . '</code>';
+            $ref = new \ReflectionMethod($handler, '__invoke');
+            $name = '<code>' . \get_class($handler) . '</code>';
         } elseif (\is_string($handler)) {
-            return '<code>' . $handler . '()</code>';
+            $ref = new \ReflectionFunction($handler);
+            $name = '<code>' . $handler . '()</code>';
         } elseif (\is_array($handler)) {
-            return '<code>' . (\is_string($handler[0]) ? $handler[0] : \get_class($handler[0])) . '::' . $handler[1] . '()</code>';
+            $ref = new \ReflectionMethod($handler[0], $handler[1]);
+            $name = '<code>' . (\is_string($handler[0]) ? $handler[0] : \get_class($handler[0])) . '::' . $handler[1] . '()</code>';
+        } else {
+            $ref = new \ReflectionFunction($handler);
+            $name = 'Request handler';
+        }
+
+        if ($linkSourceFile && !$ref->isInternal()) {
+            $name .= ' (<code title="See ' . $ref->getFileName() .' line ' . $ref->getStartLine() .'">' . \basename($ref->getFileName()) . ':' . $ref->getStartLine() . '</code>)';
         }
 
-        return 'Request handler';
+        return $name;
     }
 }
diff --git a/tests/AppTest.php b/tests/AppTest.php
index 21efdb6..49a78e4 100644
--- a/tests/AppTest.php
+++ b/tests/AppTest.php
@@ -490,6 +490,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = __LINE__ + 2;
         $handler = function () {
             throw new \RuntimeException('Foo');
         };
@@ -506,7 +507,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from request handler: Foo\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from request handler (<code title=\"See " . __FILE__ . " line $line\">AppTest.php:$line</code>): Foo\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsPromiseWhichRejectsWithException()
@@ -516,6 +517,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = __LINE__ + 2;
         $handler = function () {
             return \React\Promise\reject(new \RuntimeException('Foo'));
         };
@@ -540,7 +542,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from request handler: Foo\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Uncaught <code>RuntimeException</code> from request handler (<code title=\"See " . __FILE__ . " line $line\">AppTest.php:$line</code>): Foo\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhichFulfillsWithInternalServerErrorResponseWhenHandlerReturnsPromiseWhichRejectsWithNull()
@@ -550,6 +552,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = __LINE__ + 1;
         $handler = function () {
             return \React\Promise\reject('');
         };
@@ -574,10 +577,9 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Uncaught <code>UnexpectedValueException</code> from request handler: Handler rejected with string\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Request handler (<code title=\"See " . __FILE__ . " line $line\">AppTest.php:$line</code>) returned invalid value (<code>React\Promise\RejectedPromise</code>)\n", (string) $response->getBody());
     }
 
-
     public function provideInvalidReturnValue()
     {
         return [
@@ -628,6 +630,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = __LINE__ + 1;
         $handler = function () use ($value) {
             return $value;
         };
@@ -644,7 +647,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Request handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Request handler (<code title=\"See " . __FILE__ . " line $line\">AppTest.php:$line</code>) returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
     }
 
     /**
@@ -659,6 +662,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = __LINE__ + 1;
         $handler = function () use ($value) {
             return \React\Promise\resolve($value);
         };
@@ -683,7 +687,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsPromiseWhich
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): Request handler returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): Request handler (<code title=\"See " . __FILE__ . " line $line\">AppTest.php:$line</code>) returned invalid value (<code>$name</code>)\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenClassHandlerReturnsStringValue()
@@ -693,6 +697,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = 7;
         $handler = new InvalidHandlerStub();
 
         $dispatcher = $this->createMock(Dispatcher::class);
@@ -707,7 +712,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub</code> returned invalid value (<code>null</code>)\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub</code> (<code title=\"See " . __DIR__ . DIRECTORY_SEPARATOR . "Stub" . DIRECTORY_SEPARATOR . "InvalidHandlerStub.php line $line\">InvalidHandlerStub.php:$line</code>) returned invalid value (<code>null</code>)\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenClassMethodHandlerReturnsStringValue()
@@ -717,6 +722,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = 12;
         $handler = [new InvalidHandlerStub(), 'index'];
 
         $dispatcher = $this->createMock(Dispatcher::class);
@@ -731,7 +737,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub::index()</code> returned invalid value (<code>null</code>)\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub::index()</code> (<code title=\"See " . __DIR__ . DIRECTORY_SEPARATOR . "Stub" . DIRECTORY_SEPARATOR . "InvalidHandlerStub.php line $line\">InvalidHandlerStub.php:$line</code>) returned invalid value (<code>null</code>)\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenStaticClassMethodHandlerReturnsStringValue()
@@ -741,6 +747,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
 
         $request = new ServerRequest('GET', 'http://localhost/users');
 
+        $line = 17;
         $handler = [InvalidHandlerStub::class, 'static'];
 
         $dispatcher = $this->createMock(Dispatcher::class);
@@ -755,7 +762,7 @@ public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServ
         $this->assertInstanceOf(ResponseInterface::class, $response);
         $this->assertEquals(500, $response->getStatusCode());
         $this->assertEquals('text/html', $response->getHeaderLine('Content-Type'));
-        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub::static()</code> returned invalid value (<code>null</code>)\n", (string) $response->getBody());
+        $this->assertEquals("500 (Internal Server Error): <code>Frugal\Tests\Stub\InvalidHandlerStub::static()</code> (<code title=\"See " . __DIR__ . DIRECTORY_SEPARATOR . "Stub" . DIRECTORY_SEPARATOR . "InvalidHandlerStub.php line $line\">InvalidHandlerStub.php:$line</code>) returned invalid value (<code>null</code>)\n", (string) $response->getBody());
     }
 
     public function testHandleRequestWithDispatcherWithRouteFoundReturnsInternalServerErrorResponseWhenGlobalFunctionHandlerReturnsStringValue()
diff --git a/tests/acceptance.sh b/tests/acceptance.sh
index ba73085..b3fcc57 100755
--- a/tests/acceptance.sh
+++ b/tests/acceptance.sh
@@ -17,7 +17,8 @@ out=$(curl -v $base/ 2>&1);         match "HTTP/.* 200" && match -iv "Content-Ty
 out=$(curl -v $base/invalid 2>&1);  match "HTTP/.* 404" && match -iP "Content-Type: text/html[\r\n]"
 out=$(curl -v $base// 2>&1);        match "HTTP/.* 404"
 out=$(curl -v $base/ 2>&1 -X POST); match "HTTP/.* 405"
-out=$(curl -v $base/error 2>&1);    match "HTTP/.* 500" && match -iP "Content-Type: text/html[\r\n]"
+out=$(curl -v $base/error 2>&1);        match "HTTP/.* 500" && match -iP "Content-Type: text/html[\r\n]"
+out=$(curl -v $base/error/null 2>&1);   match "HTTP/.* 500" && match -iP "Content-Type: text/html[\r\n]"
 
 out=$(curl -v $base/uri 2>&1);                          match "HTTP/.* 200" && match "$base/uri"
 out=$(curl -v $base/uri/ 2>&1);                         match "HTTP/.* 200" && match "$base/uri/"