Skip to content

Commit

Permalink
Fixes and enhancements to Exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
@paulbalandan
paulbalandan authored Sep 4, 2021
1 parent 4ab9d66 commit 1cbdeac
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 63 deletions.
101 changes: 46 additions & 55 deletions system/Debug/Exceptions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use Config\Paths;
use ErrorException;
use Throwable;
use function error_reporting;

/**
* Exceptions manager
Expand Down Expand Up @@ -64,35 +63,25 @@ class Exceptions
*/
protected $response;

/**
* Constructor.
*/
public function __construct(ExceptionsConfig $config, IncomingRequest $request, Response $response)
{
$this->ob_level = ob_get_level();

$this->viewPath = rtrim($config->errorViewPath, '\\/ ') . DIRECTORY_SEPARATOR;

$this->config = $config;

$this->config = $config;
$this->request = $request;
$this->response = $response;
}

/**
* Responsible for registering the error, exception and shutdown
* handling of our application.
*
* @codeCoverageIgnore
*/
public function initialize()
{
// Set the Exception Handler
set_exception_handler([$this, 'exceptionHandler']);

// Set the Error Handler
set_error_handler([$this, 'errorHandler']);

// Set the handler for shutdown to catch Parse errors
// Do we need this in PHP7?
register_shutdown_function([$this, 'shutdownHandler']);
}

Expand All @@ -105,12 +94,8 @@ public function initialize()
*/
public function exceptionHandler(Throwable $exception)
{
[
$statusCode,
$exitCode,
] = $this->determineCodes($exception);
[$statusCode, $exitCode] = $this->determineCodes($exception);

// Log it
if ($this->config->log === true && ! in_array($statusCode, $this->config->ignoreCodes, true)) {
log_message('critical', $exception->getMessage() . "\n{trace}", [
'trace' => $exception->getTraceAsString(),
Expand All @@ -119,8 +104,7 @@ public function exceptionHandler(Throwable $exception)

if (! is_cli()) {
$this->response->setStatusCode($statusCode);
$header = "HTTP/{$this->request->getProtocolVersion()} {$this->response->getStatusCode()} {$this->response->getReason()}";
header($header, true, $statusCode);
header(sprintf('HTTP/%s %s %s', $this->request->getProtocolVersion(), $this->response->getStatusCode(), $this->response->getReasonPhrase()), true, $statusCode);

if (strpos($this->request->getHeaderLine('accept'), 'text/html') === false) {
$this->respond(ENVIRONMENT === 'development' ? $this->collectVars($exception, $statusCode) : '', $statusCode)->send();
Expand All @@ -142,31 +126,36 @@ public function exceptionHandler(Throwable $exception)
* This seems to be primarily when a user triggers it with trigger_error().
*
* @throws ErrorException
*
* @codeCoverageIgnore
*/
public function errorHandler(int $severity, string $message, ?string $file = null, ?int $line = null)
{
if (! (error_reporting() & $severity)) {
return;
}

// Convert it to an exception and pass it along.
throw new ErrorException($message, 0, $severity, $file, $line);
}

/**
* Checks to see if any errors have happened during shutdown that
* need to be caught and handle them.
*
* @codeCoverageIgnore
*/
public function shutdownHandler()
{
$error = error_get_last();

// If we've got an error that hasn't been displayed, then convert
// it to an Exception and use the Exception handler to display it
// to the user.
// Fatal Error?
if ($error !== null && in_array($error['type'], [E_ERROR, E_CORE_ERROR, E_COMPILE_ERROR, E_PARSE], true)) {
$this->exceptionHandler(new ErrorException($error['message'], $error['type'], 0, $error['file'], $error['line']));
if ($error === null) {
return;
}

['type' => $type, 'message' => $message, 'file' => $file, 'line' => $line] = $error;

if (in_array($type, [E_ERROR, E_CORE_ERROR, E_COMPILE_ERROR, E_PARSE], true)) {
$this->exceptionHandler(new ErrorException($message, $type, 0, $file, $line));
}
}

Expand Down Expand Up @@ -222,20 +211,25 @@ protected function render(Throwable $exception, int $statusCode)
$viewFile = $altPath . $altView;
}

// Prepare the vars
$vars = $this->collectVars($exception, $statusCode);
extract($vars);
if (! isset($viewFile)) {
echo 'The error view files were not found. Cannot render exception trace.';

exit(1);
}

// Render it
if (ob_get_level() > $this->ob_level + 1) {
ob_end_clean();
}

ob_start();
include $viewFile; // @phpstan-ignore-line
$buffer = ob_get_contents();
ob_end_clean();
echo $buffer;
echo(function () use ($exception, $statusCode, $viewFile): string {
$vars = $this->collectVars($exception, $statusCode);
extract($vars, EXTR_SKIP);

ob_start();
include $viewFile;

return ob_get_clean();
})();
}

/**
Expand All @@ -244,7 +238,8 @@ protected function render(Throwable $exception, int $statusCode)
protected function collectVars(Throwable $exception, int $statusCode): array
{
$trace = $exception->getTrace();
if (! empty($this->config->sensitiveDataInTrace)) {

if ($this->config->sensitiveDataInTrace !== []) {
$this->maskSensitiveData($trace, $this->config->sensitiveDataInTrace);
}

Expand Down Expand Up @@ -279,11 +274,11 @@ protected function maskSensitiveData(&$trace, array $keysToMask, string $path =
}
}

if (! is_iterable($trace) && is_object($trace)) {
if (is_object($trace)) {
$trace = get_object_vars($trace);
}

if (is_iterable($trace)) {
if (is_array($trace)) {
foreach ($trace as $pathKey => $subarray) {
$this->maskSensitiveData($subarray, $keysToMask, $path . '/' . $pathKey);
}
Expand All @@ -298,28 +293,25 @@ protected function determineCodes(Throwable $exception): array
$statusCode = abs($exception->getCode());

if ($statusCode < 100 || $statusCode > 599) {
$exitStatus = $statusCode + EXIT__AUTO_MIN; // 9 is EXIT__AUTO_MIN
if ($exitStatus > EXIT__AUTO_MAX) { // 125 is EXIT__AUTO_MAX
$exitStatus = EXIT_ERROR; // EXIT_ERROR
$exitStatus = $statusCode + EXIT__AUTO_MIN;

if ($exitStatus > EXIT__AUTO_MAX) {
$exitStatus = EXIT_ERROR;
}

$statusCode = 500;
} else {
$exitStatus = 1; // EXIT_ERROR
$exitStatus = EXIT_ERROR;
}

return [
$statusCode ?: 500,
$exitStatus,
];
return [$statusCode, $exitStatus];
}

//--------------------------------------------------------------------
// Display Methods
//--------------------------------------------------------------------

/**
* Clean Path
*
* This makes nicer looking paths for the error output.
*/
public static function cleanPath(string $file): string
Expand Down Expand Up @@ -354,6 +346,7 @@ public static function describeMemory(int $bytes): string
if ($bytes < 1024) {
return $bytes . 'B';
}

if ($bytes < 1048576) {
return round($bytes / 1024, 2) . 'KB';
}
Expand Down Expand Up @@ -390,18 +383,16 @@ public static function highlightFile(string $file, int $lineNumber, int $lines =
$source = str_replace(["\r\n", "\r"], "\n", $source);
$source = explode("\n", highlight_string($source, true));
$source = str_replace('<br />', "\n", $source[1]);

$source = explode("\n", str_replace("\r\n", "\n", $source));

// Get just the part to show
$start = $lineNumber - (int) round($lines / 2);
$start = $start < 0 ? 0 : $start;
$start = max($lineNumber - (int) round($lines / 2), 0);

// Get just the lines we need to display, while keeping line numbers...
$source = array_splice($source, $start, $lines, true); // @phpstan-ignore-line

// Used to format the line number in the source
$format = '% ' . strlen(sprintf('%s', $start + $lines)) . 'd';
$format = '% ' . strlen((string) ($start + $lines)) . 'd';

$out = '';
// Because the highlighting may have an uneven number
Expand All @@ -412,11 +403,11 @@ public static function highlightFile(string $file, int $lineNumber, int $lines =

foreach ($source as $n => $row) {
$spans += substr_count($row, '<span') - substr_count($row, '</span');

$row = str_replace(["\r", "\n"], ['', ''], $row);

if (($n + $start + 1) === $lineNumber) {
preg_match_all('#<[^>]+>#', $row, $tags);

$out .= sprintf(
"<span class='line highlight'><span class='number'>{$format}</span> %s\n</span>%s",
$n + $start + 1,
Expand Down
53 changes: 45 additions & 8 deletions tests/system/Debug/ExceptionsTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,64 @@

namespace CodeIgniter\Debug;

use CodeIgniter\Exceptions\PageNotFoundException;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\ReflectionHelper;
use Config\Exceptions as ExceptionsConfig;
use Config\Services;
use RuntimeException;

/**
* @internal
*/
final class ExceptionsTest extends CIUnitTestCase
{
public function testNew()
use ReflectionHelper;

/**
* @var Exceptions
*/
private $exception;

protected function setUp(): void
{
$actual = new Exceptions(new \Config\Exceptions(), Services::request(), Services::response());
$this->assertInstanceOf(Exceptions::class, $actual);
$this->exception = new Exceptions(new ExceptionsConfig(), Services::request(), Services::response());
}

public function testDetermineViews(): void
{
$determineView = $this->getPrivateMethodInvoker($this->exception, 'determineView');

$this->assertSame('error_404.php', $determineView(PageNotFoundException::forControllerNotFound('Foo', 'bar'), ''));
$this->assertSame('error_exception.php', $determineView(new RuntimeException('Exception'), ''));
$this->assertSame('error_404.php', $determineView(new RuntimeException('foo', 404), 'app/Views/errors/cli'));
}

public function testCollectVars(): void
{
$vars = $this->getPrivateMethodInvoker($this->exception, 'collectVars')(new RuntimeException('This.'), 404);

$this->assertIsArray($vars);
$this->assertCount(7, $vars);

foreach (['title', 'type', 'code', 'message', 'file', 'line', 'trace'] as $key) {
$this->assertArrayHasKey($key, $vars);
}
}

public function testDetermineCodes(): void
{
$determineCodes = $this->getPrivateMethodInvoker($this->exception, 'determineCodes');

$this->assertSame([500, 9], $determineCodes(new RuntimeException('This.')));
$this->assertSame([500, 1], $determineCodes(new RuntimeException('That.', 600)));
$this->assertSame([404, 1], $determineCodes(new RuntimeException('There.', 404)));
}

/**
* @dataProvider dirtyPathsProvider
*
* @param mixed $file
* @param mixed $expected
*/
public function testCleanPaths($file, $expected)
public function testCleanPaths(string $file, string $expected): void
{
$this->assertSame($expected, Exceptions::cleanPath($file));
}
Expand All @@ -40,7 +77,7 @@ public function dirtyPathsProvider()
{
$ds = DIRECTORY_SEPARATOR;

return [
yield from [
[
APPPATH . 'Config' . $ds . 'App.php',
'APPPATH' . $ds . 'Config' . $ds . 'App.php',
Expand Down

0 comments on commit 1cbdeac

Please sign in to comment.