Skip to content

Commit

Permalink
Merge pull request #32 from midahp/improved-error-handling
Browse files Browse the repository at this point in the history 
Improved error handling
  • Loading branch information
@ralflang
ralflang authored Oct 25, 2023
2 parents dde2b8c + 94d7a6c commit 036eabe
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 68 deletions.
129 changes: 86 additions & 43 deletions lib/Horde/ErrorHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,57 +87,19 @@ public static function fatal($error)

if (!headers_sent()) {
header('Content-type: text/html; charset=UTF-8');
header('HTTP/1.1 500: Internal Server Error');
}
echo <<< HTML
<html>
<head><title>Horde :: Fatal Error</title></head>
<body style="background:#fff; color:#000">
HTML;

ob_start();

try {
$admin = (isset($registry) && $registry->isAdmin());

echo '<h1>' . Horde_Core_Translation::t("A fatal error has occurred") . '</h1>';

if (is_object($error) && method_exists($error, 'getMessage')) {
echo '<h3>' . htmlspecialchars($error->getMessage()) . '</h3>';
} elseif (is_string($error)) {
echo '<h3>' . htmlspecialchars($error) . '</h3>';
}

if ($admin) {
if ($error instanceof Throwable ||
$error instanceof Exception) {
$trace = $error;
$file = $error->getFile();
$line = $error->getLine();
} else {
$trace = debug_backtrace();
$calling = array_shift($trace);
$file = $calling['file'];
$line = $calling['line'];
}
printf(Horde_Core_Translation::t("in %s:%d"), $file, $line);
echo '<div id="backtrace"><pre>' .
strval(new Horde_Support_Backtrace($trace)) .
'</pre></div>';
if (is_object($error)) {
echo '<h3>' . Horde_Core_Translation::t("Details") . '</h3>';
echo '<h4>' . Horde_Core_Translation::t("The full error message is logged in Horde's log file, and is shown below only to administrators. Non-administrative users will not see error details.") . '</h4>';
ob_flush();
flush();
echo '<div id="details"><pre>' . htmlspecialchars(print_r($error, true)) . '</pre></div>';
}
} else {
echo '<h3>' . Horde_Core_Translation::t("Details have been logged for the administrator.") . '</h3>';
}
$errorHtml = self::getHtmlForError($error, $admin);
} catch (Exception $e) {
die($e);
}

ob_start();
echo $errorHtml;
ob_end_flush();
echo '</body></html>';
exit(1);
}

Expand Down Expand Up @@ -209,4 +171,85 @@ public static function catchFatalError()
}
}

/**
* Returns html for an error
*
* @param string|Throwable $error The error as string message or Throwable
* @param bool $isAdmin If true will also output the complete trace
* If $error is a Throwable its complete content will also be included in the output
* @return string The full html page for that error as a string
*/
public static function getHtmlForError($error, bool $isAdmin = false): string
{
if ($error instanceof Throwable) {
$message = htmlspecialchars($error->getMessage());

} else {
$message = $error;
}
$fatalErrorHasOccoured = Horde_Core_Translation::t('A fatal error has occurred');
if ($isAdmin){
$detailsHtml = self::getErrorDetailsAsHtml($error);
} else {
$detailsHtml = '<h3>' . Horde_Core_Translation::t("Details have been logged for the administrator.") . '</h3>';
}
return <<<HTMLDELIM
<html>
<head>
<title>Horde :: Fatal Error</title>
<meta charset="utf-8">
</head>
<body style="background:#fff; color:#000">
<h1>{$fatalErrorHasOccoured}</h1>
<h3>{$message}</h3>
{$detailsHtml}
</body>
</html>
HTMLDELIM;

}

/**
* Get the details of an error as html. This should usually only be output to admin users
*
* @param string|Throwable $error The error as string message or Throwable
* @return string The details of that error as html
*/
protected static function getErrorDetailsAsHtml($error): string
{
if ($error instanceof Throwable) {
$trace = strval(new Horde_Support_Backtrace($error));
$fileName = $error->getFile();
$lineNo = $error->getLine();

$translateDetails = Horde_Core_Translation::t('Details');
$translateLogInfo = Horde_Core_Translation::t('The full error message is logged in Horde\'s log file, and is shown below only to administrators. Non-administrative users will not see error details.');
$fullErrorAsHtml = htmlspecialchars(print_r($error, true));
$fullDetailsHtml = <<<HTMLDELIM
<h3>$translateDetails</h3>
<h4>{$translateLogInfo}</h4>
<div id="details">
<pre>$fullErrorAsHtml</pre>
</div>
HTMLDELIM;
} else {
$trace = debug_backtrace();
$calling = array_shift($trace);
$trace = strval(new Horde_Support_Backtrace($trace));
$fileName = $calling['file'];
$lineNo = $calling['line'];
$fullDetailsHtml = '';
}
$translateIn = sprintf(Horde_Core_Translation::t('in %s:%d'), $fileName, $lineNo);

return <<<HTMLDELIM
{$translateIn}
<div id="backtrace">
<pre>
{$trace}
</pre>
</div>
{$fullDetailsHtml}
HTMLDELIM;
}
}
21 changes: 18 additions & 3 deletions src/Middleware/AppFinder.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

use Exception;
use Horde;
use Horde\Http\ResponseFactory;
use Horde\Http\StreamFactory;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Server\MiddlewareInterface;
Expand All @@ -31,9 +33,17 @@
class AppFinder implements MiddlewareInterface
{
private Horde_Registry $registry;
public function __construct(Horde_Registry $registry)
{
private ResponseFactory $responseFactory;
private StreamFactory $streamFactory;

public function __construct(
Horde_Registry $registry,
ResponseFactory $responseFactory,
StreamFactory $streamFactory
) {
$this->registry = $registry;
$this->responseFactory = $responseFactory;
$this->streamFactory = $streamFactory;
}
/**
* Rebuild a path string to a common form
Expand Down Expand Up @@ -171,7 +181,12 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
// If we still found no app, give up
if (empty($found)) {
$path = $request->getUri()->getPath();
throw new \Exception("No App found for path: $path");
$msg = sprintf('No App found for path: %s', $path);
Horde::log($msg, 'INFO');
return $this->responseFactory->createResponse(
404,
'Not Found'
)->withBody($this->streamFactory->createStream($msg));
}
$prefix = $found['path'];

Expand Down
61 changes: 57 additions & 4 deletions src/Middleware/ErrorFilter.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@

namespace Horde\Core\Middleware;

use Horde;
use Horde\Http\ResponseFactory;
use Horde\Http\StreamFactory;
use Horde_ErrorHandler;
use Horde_Registry;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Horde_Registry;
use Horde_Application;
use Throwable;

/**
* ErrorFilter middleware
Expand All @@ -31,9 +35,58 @@
*/
class ErrorFilter implements MiddlewareInterface
{
protected Horde_Registry $registry;
protected ResponseFactory $responseFactory;
protected StreamFactory $streamFactory;

public function __construct(
Horde_Registry $registry,
ResponseFactory $responseFactory,
StreamFactory $streamFactory
) {
$this->registry = $registry;
$this->responseFactory = $responseFactory;
$this->streamFactory = $streamFactory;
}

public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{
// TODO:
return $handler->handle($request);
try {
return $handler->handle($request);
} catch (Throwable $throwable) {
Horde::log($throwable, 'EMERG');
return $this->getErrorResponse($request, $throwable);
}
}

protected function getErrorResponse(ServerRequestInterface $request, Throwable $throwable): ResponseInterface
{
$isAdmin = $this->registry->isAdmin();
$acceptsJson = in_array('application/json', array_map(fn($val) => strtolower($val), $request->getHeader('Accept')));
if ($acceptsJson){
return $this->getJsonResponse($throwable, $isAdmin);
} else {
return $this->getHtmlResponse($throwable, $isAdmin);
}
}

protected function getJsonResponse(Throwable $throwable, bool $isAdmin = false): ResponseInterface
{
$json = json_encode([
'message' => $throwable->getMessage(),
'code' => $throwable->getCode(),
'trace' => $isAdmin ? $throwable->getTrace() : [],
]);
$stream = $this->streamFactory->createStream($json);
return $this->responseFactory->createResponse(500, 'Internal Server Error')
->withBody($stream)
->withHeader('Content-Type', 'application/json');
}

protected function getHtmlResponse(Throwable $throwable, bool $isAdmin = false): ResponseInterface
{
$stream = $this->streamFactory->createStream(Horde_ErrorHandler::getHtmlForError($throwable, $isAdmin));
return $this->responseFactory->createResponse(500, 'Internal Server Error')
->withBody($stream);
}
}
10 changes: 6 additions & 4 deletions src/Middleware/HordeCore.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,15 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
$injector->setInstance(ResponseFactoryInterface::class, new ResponseFactory());
}


// Detect correct app
$registry = $injector->getInstance('Horde_Registry');
$request = $request->withAttribute('registry', $registry);
$handler->addMiddleware(new AppFinder($registry));
// First middleware should be ErrorFilter to catch all errors
$handler->addMiddleware(new ErrorFilter($registry, new ResponseFactory(), new StreamFactory()));
// Detect correct app
$handler->addMiddleware(new AppFinder($registry, new ResponseFactory(), new StreamFactory()));
// Find route inside detected app
$handler->addMiddleware(new AppRouter($registry, $injector->get('Horde_Routes_Mapper'), $injector));

$request = $request->withAttribute('registry', $registry);
return $handler->handle($request);
}
}
32 changes: 18 additions & 14 deletions test/Middleware/AppFinderTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

use Exception;
use Horde\Core\Middleware\AppFinder;

use Horde\Http\ResponseFactory;
use Horde\Http\StreamFactory;
use Horde\Test\TestCase;

use Horde_Registry;
Expand All @@ -27,7 +28,9 @@ class AppFinderTest extends TestCase
protected function getMiddleware()
{
return new AppFinder(
$this->registry
$this->registry,
new ResponseFactory(),
new StreamFactory()
);
}

Expand Down Expand Up @@ -86,8 +89,9 @@ public function testNoValidAppInPath()
});

$middleware = $this->getMiddleware();
$this->expectException(Exception::class);
$middleware->process($request, $this->handler);
//$this->expectException(Exception::class);
$response = $middleware->process($request, $this->handler);
$this->assertSame(404, $response->getStatusCode());
}

/**
Expand Down Expand Up @@ -137,8 +141,8 @@ public function testNoAppAvailable()
});

$middleware = $this->getMiddleware();
$this->expectException(Exception::class);
$middleware->process($request, $this->handler);
$response = $middleware->process($request, $this->handler);
$this->assertSame(404, $response->getStatusCode());
}

/**
Expand Down Expand Up @@ -188,8 +192,8 @@ public function testDifferntScheme()

$middleware = $this->getMiddleware();

$this->expectException(Exception::class);
$middleware->process($request, $this->handler);
$response = $middleware->process($request, $this->handler);
$this->assertSame(404, $response->getStatusCode());
}

/**
Expand All @@ -213,8 +217,8 @@ public function testDifferentHost()

$middleware = $this->getMiddleware();

$this->expectException(Exception::class);
$middleware->process($request, $this->handler);
$response = $middleware->process($request, $this->handler);
$this->assertSame(404, $response->getStatusCode());
}


Expand All @@ -236,8 +240,8 @@ public function testEmptyPath()
});

$middleware = $this->getMiddleware();
$this->expectException(Exception::class);
$middleware->process($request, $this->handler);
$response = $middleware->process($request, $this->handler);
$this->assertSame(404, $response->getStatusCode());
}

public function testFindAppBehindDifferentApp()
Expand Down Expand Up @@ -333,7 +337,7 @@ public function testDoNotFindWithoutAlias()
});

$middleware = $this->getMiddleware();
$this->expectException(Exception::class);
$middleware->process($request, $this->handler);
$response = $middleware->process($request, $this->handler);
$this->assertSame(404, $response->getStatusCode());
}
}

0 comments on commit 036eabe

Please sign in to comment.