Skip to content

Commit 1cbdeac

Browse files
paulbalandanpaulbalandan
authored
Fixes and enhancements to Exceptions
1 parent 4ab9d66 commit 1cbdeac

File tree

2 files changed

+91
-63
lines changed

2 files changed

+91
-63
lines changed

‎system/Debug/Exceptions.php

Lines changed: 46 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
use Config\Paths;
2020
use ErrorException;
2121
use Throwable;
22-
use function error_reporting;
2322

2423
/**
2524
* Exceptions manager
@@ -64,35 +63,25 @@ class Exceptions
6463
*/
6564
protected $response;
6665

67-
/**
68-
* Constructor.
69-
*/
7066
public function __construct(ExceptionsConfig $config, IncomingRequest $request, Response $response)
7167
{
7268
$this->ob_level = ob_get_level();
73-
7469
$this->viewPath = rtrim($config->errorViewPath, '\\/ ') . DIRECTORY_SEPARATOR;
75-
76-
$this->config = $config;
77-
70+
$this->config = $config;
7871
$this->request = $request;
7972
$this->response = $response;
8073
}
8174

8275
/**
8376
* Responsible for registering the error, exception and shutdown
8477
* handling of our application.
78+
*
79+
* @codeCoverageIgnore
8580
*/
8681
public function initialize()
8782
{
88-
// Set the Exception Handler
8983
set_exception_handler([$this, 'exceptionHandler']);
90-
91-
// Set the Error Handler
9284
set_error_handler([$this, 'errorHandler']);
93-
94-
// Set the handler for shutdown to catch Parse errors
95-
// Do we need this in PHP7?
9685
register_shutdown_function([$this, 'shutdownHandler']);
9786
}
9887

@@ -105,12 +94,8 @@ public function initialize()
10594
*/
10695
public function exceptionHandler(Throwable $exception)
10796
{
108-
[
109-
$statusCode,
110-
$exitCode,
111-
] = $this->determineCodes($exception);
97+
[$statusCode, $exitCode] = $this->determineCodes($exception);
11298

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

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

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

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

156141
/**
157142
* Checks to see if any errors have happened during shutdown that
158143
* need to be caught and handle them.
144+
*
145+
* @codeCoverageIgnore
159146
*/
160147
public function shutdownHandler()
161148
{
162149
$error = error_get_last();
163150

164-
// If we've got an error that hasn't been displayed, then convert
165-
// it to an Exception and use the Exception handler to display it
166-
// to the user.
167-
// Fatal Error?
168-
if ($error !== null && in_array($error['type'], [E_ERROR, E_CORE_ERROR, E_COMPILE_ERROR, E_PARSE], true)) {
169-
$this->exceptionHandler(new ErrorException($error['message'], $error['type'], 0, $error['file'], $error['line']));
151+
if ($error === null) {
152+
return;
153+
}
154+
155+
['type' => $type, 'message' => $message, 'file' => $file, 'line' => $line] = $error;
156+
157+
if (in_array($type, [E_ERROR, E_CORE_ERROR, E_COMPILE_ERROR, E_PARSE], true)) {
158+
$this->exceptionHandler(new ErrorException($message, $type, 0, $file, $line));
170159
}
171160
}
172161

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

225-
// Prepare the vars
226-
$vars = $this->collectVars($exception, $statusCode);
227-
extract($vars);
214+
if (! isset($viewFile)) {
215+
echo 'The error view files were not found. Cannot render exception trace.';
216+
217+
exit(1);
218+
}
228219

229-
// Render it
230220
if (ob_get_level() > $this->ob_level + 1) {
231221
ob_end_clean();
232222
}
233223

234-
ob_start();
235-
include $viewFile; // @phpstan-ignore-line
236-
$buffer = ob_get_contents();
237-
ob_end_clean();
238-
echo $buffer;
224+
echo(function () use ($exception, $statusCode, $viewFile): string {
225+
$vars = $this->collectVars($exception, $statusCode);
226+
extract($vars, EXTR_SKIP);
227+
228+
ob_start();
229+
include $viewFile;
230+
231+
return ob_get_clean();
232+
})();
239233
}
240234

241235
/**
@@ -244,7 +238,8 @@ protected function render(Throwable $exception, int $statusCode)
244238
protected function collectVars(Throwable $exception, int $statusCode): array
245239
{
246240
$trace = $exception->getTrace();
247-
if (! empty($this->config->sensitiveDataInTrace)) {
241+
242+
if ($this->config->sensitiveDataInTrace !== []) {
248243
$this->maskSensitiveData($trace, $this->config->sensitiveDataInTrace);
249244
}
250245

@@ -279,11 +274,11 @@ protected function maskSensitiveData(&$trace, array $keysToMask, string $path =
279274
}
280275
}
281276

282-
if (! is_iterable($trace) && is_object($trace)) {
277+
if (is_object($trace)) {
283278
$trace = get_object_vars($trace);
284279
}
285280

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

300295
if ($statusCode < 100 || $statusCode > 599) {
301-
$exitStatus = $statusCode + EXIT__AUTO_MIN; // 9 is EXIT__AUTO_MIN
302-
if ($exitStatus > EXIT__AUTO_MAX) { // 125 is EXIT__AUTO_MAX
303-
$exitStatus = EXIT_ERROR; // EXIT_ERROR
296+
$exitStatus = $statusCode + EXIT__AUTO_MIN;
297+
298+
if ($exitStatus > EXIT__AUTO_MAX) {
299+
$exitStatus = EXIT_ERROR;
304300
}
301+
305302
$statusCode = 500;
306303
} else {
307-
$exitStatus = 1; // EXIT_ERROR
304+
$exitStatus = EXIT_ERROR;
308305
}
309306

310-
return [
311-
$statusCode ?: 500,
312-
$exitStatus,
313-
];
307+
return [$statusCode, $exitStatus];
314308
}
315309

316310
//--------------------------------------------------------------------
317311
// Display Methods
318312
//--------------------------------------------------------------------
319313

320314
/**
321-
* Clean Path
322-
*
323315
* This makes nicer looking paths for the error output.
324316
*/
325317
public static function cleanPath(string $file): string
@@ -354,6 +346,7 @@ public static function describeMemory(int $bytes): string
354346
if ($bytes < 1024) {
355347
return $bytes . 'B';
356348
}
349+
357350
if ($bytes < 1048576) {
358351
return round($bytes / 1024, 2) . 'KB';
359352
}
@@ -390,18 +383,16 @@ public static function highlightFile(string $file, int $lineNumber, int $lines =
390383
$source = str_replace(["\r\n", "\r"], "\n", $source);
391384
$source = explode("\n", highlight_string($source, true));
392385
$source = str_replace('<br />', "\n", $source[1]);
393-
394386
$source = explode("\n", str_replace("\r\n", "\n", $source));
395387

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

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

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

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

413404
foreach ($source as $n => $row) {
414405
$spans += substr_count($row, '<span') - substr_count($row, '</span');
415-
416406
$row = str_replace(["\r", "\n"], ['', ''], $row);
417407

418408
if (($n + $start + 1) === $lineNumber) {
419409
preg_match_all('#<[^>]+>#', $row, $tags);
410+
420411
$out .= sprintf(
421412
"<span class='line highlight'><span class='number'>{$format}</span> %s\n</span>%s",
422413
$n + $start + 1,

‎tests/system/Debug/ExceptionsTest.php

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,64 @@
1111

1212
namespace CodeIgniter\Debug;
1313

14+
use CodeIgniter\Exceptions\PageNotFoundException;
1415
use CodeIgniter\Test\CIUnitTestCase;
16+
use CodeIgniter\Test\ReflectionHelper;
17+
use Config\Exceptions as ExceptionsConfig;
1518
use Config\Services;
19+
use RuntimeException;
1620

1721
/**
1822
* @internal
1923
*/
2024
final class ExceptionsTest extends CIUnitTestCase
2125
{
22-
public function testNew()
26+
use ReflectionHelper;
27+
28+
/**
29+
* @var Exceptions
30+
*/
31+
private $exception;
32+
33+
protected function setUp(): void
2334
{
24-
$actual = new Exceptions(new \Config\Exceptions(), Services::request(), Services::response());
25-
$this->assertInstanceOf(Exceptions::class, $actual);
35+
$this->exception = new Exceptions(new ExceptionsConfig(), Services::request(), Services::response());
36+
}
37+
38+
public function testDetermineViews(): void
39+
{
40+
$determineView = $this->getPrivateMethodInvoker($this->exception, 'determineView');
41+
42+
$this->assertSame('error_404.php', $determineView(PageNotFoundException::forControllerNotFound('Foo', 'bar'), ''));
43+
$this->assertSame('error_exception.php', $determineView(new RuntimeException('Exception'), ''));
44+
$this->assertSame('error_404.php', $determineView(new RuntimeException('foo', 404), 'app/Views/errors/cli'));
45+
}
46+
47+
public function testCollectVars(): void
48+
{
49+
$vars = $this->getPrivateMethodInvoker($this->exception, 'collectVars')(new RuntimeException('This.'), 404);
50+
51+
$this->assertIsArray($vars);
52+
$this->assertCount(7, $vars);
53+
54+
foreach (['title', 'type', 'code', 'message', 'file', 'line', 'trace'] as $key) {
55+
$this->assertArrayHasKey($key, $vars);
56+
}
57+
}
58+
59+
public function testDetermineCodes(): void
60+
{
61+
$determineCodes = $this->getPrivateMethodInvoker($this->exception, 'determineCodes');
62+
63+
$this->assertSame([500, 9], $determineCodes(new RuntimeException('This.')));
64+
$this->assertSame([500, 1], $determineCodes(new RuntimeException('That.', 600)));
65+
$this->assertSame([404, 1], $determineCodes(new RuntimeException('There.', 404)));
2666
}
2767

2868
/**
2969
* @dataProvider dirtyPathsProvider
30-
*
31-
* @param mixed $file
32-
* @param mixed $expected
3370
*/
34-
public function testCleanPaths($file, $expected)
71+
public function testCleanPaths(string $file, string $expected): void
3572
{
3673
$this->assertSame($expected, Exceptions::cleanPath($file));
3774
}
@@ -40,7 +77,7 @@ public function dirtyPathsProvider()
4077
{
4178
$ds = DIRECTORY_SEPARATOR;
4279

43-
return [
80+
yield from [
4481
[
4582
APPPATH . 'Config' . $ds . 'App.php',
4683
'APPPATH' . $ds . 'Config' . $ds . 'App.php',

0 commit comments

Comments
 (0)