diff --git a/system/Common.php b/system/Common.php index 6e32cf354e20..7478db443d3b 100644 --- a/system/Common.php +++ b/system/Common.php @@ -913,6 +913,57 @@ function remove_invisible_characters(string $str, bool $urlEncoded = true): stri } } +if (! function_exists('render_backtrace')) { + /** + * Renders a backtrace in a nice string format. + * + * @param list + * }> $backtrace + */ + function render_backtrace(array $backtrace): string + { + $backtraces = []; + + foreach ($backtrace as $index => $trace) { + $frame = $trace + ['file' => '[internal function]', 'line' => 0, 'class' => '', 'type' => '', 'args' => []]; + + if ($frame['file'] !== '[internal function]') { + $frame['file'] = sprintf('%s(%s)', $frame['file'], $frame['line']); + } + + unset($frame['line']); + $idx = $index; + $idx = str_pad((string) ++$idx, 2, ' ', STR_PAD_LEFT); + + $args = implode(', ', array_map(static fn ($value): string => match (true) { + is_object($value) => sprintf('Object(%s)', $value::class), + is_array($value) => $value !== [] ? '[...]' : '[]', + $value === null => 'null', + is_resource($value) => sprintf('resource (%s)', get_resource_type($value)), + default => var_export($value, true), + }, $frame['args'])); + + $backtraces[] = sprintf( + '%s %s: %s%s%s(%s)', + $idx, + clean_path($frame['file']), + $frame['class'], + $frame['type'], + $frame['function'], + $args + ); + } + + return implode("\n", $backtraces); + } +} + if (! function_exists('request')) { /** * Returns the shared Request. diff --git a/system/Database/MySQLi/Connection.php b/system/Database/MySQLi/Connection.php index 60de6d8dc24e..f3449d4a0c65 100644 --- a/system/Database/MySQLi/Connection.php +++ b/system/Database/MySQLi/Connection.php @@ -326,7 +326,12 @@ protected function execute(string $sql) try { return $this->connID->query($this->prepQuery($sql), $this->resultMode); } catch (mysqli_sql_exception $e) { - log_message('error', (string) $e); + log_message('error', "{message}\nin {exFile} on line {exLine}.\n{trace}", [ + 'message' => $e->getMessage(), + 'exFile' => clean_path($e->getFile()), + 'exLine' => $e->getLine(), + 'trace' => render_backtrace($e->getTrace()), + ]); if ($this->DBDebug) { throw new DatabaseException($e->getMessage(), $e->getCode(), $e); diff --git a/system/Database/OCI8/Connection.php b/system/Database/OCI8/Connection.php index 53a5365bc90d..00da4492d745 100644 --- a/system/Database/OCI8/Connection.php +++ b/system/Database/OCI8/Connection.php @@ -226,7 +226,14 @@ protected function execute(string $sql) return $result; } catch (ErrorException $e) { - log_message('error', (string) $e); + $trace = array_slice($e->getTrace(), 2); // remove call to error handler + + log_message('error', "{message}\nin {exFile} on line {exLine}.\n{trace}", [ + 'message' => $e->getMessage(), + 'exFile' => clean_path($e->getFile()), + 'exLine' => $e->getLine(), + 'trace' => render_backtrace($trace), + ]); if ($this->DBDebug) { throw new DatabaseException($e->getMessage(), $e->getCode(), $e); diff --git a/system/Database/Postgre/Connection.php b/system/Database/Postgre/Connection.php index e808930663dc..e3368ed77792 100644 --- a/system/Database/Postgre/Connection.php +++ b/system/Database/Postgre/Connection.php @@ -205,7 +205,14 @@ protected function execute(string $sql) try { return pg_query($this->connID, $sql); } catch (ErrorException $e) { - log_message('error', (string) $e); + $trace = array_slice($e->getTrace(), 2); // remove the call to error handler + + log_message('error', "{message}\nin {exFile} on line {exLine}.\n{trace}", [ + 'message' => $e->getMessage(), + 'exFile' => clean_path($e->getFile()), + 'exLine' => $e->getLine(), + 'trace' => render_backtrace($trace), + ]); if ($this->DBDebug) { throw new DatabaseException($e->getMessage(), $e->getCode(), $e); diff --git a/system/Database/SQLSRV/Connection.php b/system/Database/SQLSRV/Connection.php index 95f75cf44768..f8fb3aa9f5db 100644 --- a/system/Database/SQLSRV/Connection.php +++ b/system/Database/SQLSRV/Connection.php @@ -155,8 +155,12 @@ public function getAllErrorMessages(): string $errors = []; foreach (sqlsrv_errors() as $error) { - $errors[] = $error['message'] - . ' SQLSTATE: ' . $error['SQLSTATE'] . ', code: ' . $error['code']; + $errors[] = sprintf( + '%s SQLSTATE: %s, code: %s', + $error['message'], + $error['SQLSTATE'], + $error['code'] + ); } return implode("\n", $errors); @@ -488,17 +492,23 @@ public function setDatabase(?string $databaseName = null) */ protected function execute(string $sql) { - $stmt = ($this->scrollable === false || $this->isWriteType($sql)) ? - sqlsrv_query($this->connID, $sql) : - sqlsrv_query($this->connID, $sql, [], ['Scrollable' => $this->scrollable]); + $stmt = ($this->scrollable === false || $this->isWriteType($sql)) + ? sqlsrv_query($this->connID, $sql) + : sqlsrv_query($this->connID, $sql, [], ['Scrollable' => $this->scrollable]); if ($stmt === false) { - $error = $this->error(); + $trace = debug_backtrace(); + $first = array_shift($trace); - log_message('error', $error['message']); + log_message('error', "{message}\nin {exFile} on line {exLine}.\n{trace}", [ + 'message' => $this->getAllErrorMessages(), + 'exFile' => clean_path($first['file']), + 'exLine' => $first['line'], + 'trace' => render_backtrace($trace), + ]); if ($this->DBDebug) { - throw new DatabaseException($error['message']); + throw new DatabaseException($this->getAllErrorMessages()); } } diff --git a/system/Database/SQLite3/Connection.php b/system/Database/SQLite3/Connection.php index 0d3290b38d9d..9a7dc53c6445 100644 --- a/system/Database/SQLite3/Connection.php +++ b/system/Database/SQLite3/Connection.php @@ -175,7 +175,12 @@ protected function execute(string $sql) ? $this->connID->exec($sql) : $this->connID->query($sql); } catch (Exception $e) { - log_message('error', (string) $e); + log_message('error', "{message}\nin {exFile} on line {exLine}.\n{trace}", [ + 'message' => $e->getMessage(), + 'exFile' => clean_path($e->getFile()), + 'exLine' => $e->getLine(), + 'trace' => render_backtrace($e->getTrace()), + ]); if ($this->DBDebug) { throw new DatabaseException($e->getMessage(), $e->getCode(), $e); diff --git a/system/Debug/Exceptions.php b/system/Debug/Exceptions.php index 7688d29f01e3..7c01dd4ac015 100644 --- a/system/Debug/Exceptions.php +++ b/system/Debug/Exceptions.php @@ -136,7 +136,7 @@ public function exceptionHandler(Throwable $exception) 'routeInfo' => $routeInfo, 'exFile' => clean_path($exception->getFile()), // {file} refers to THIS file 'exLine' => $exception->getLine(), // {line} refers to THIS line - 'trace' => self::renderBacktrace($exception->getTrace()), + 'trace' => render_backtrace($exception->getTrace()), ]); // Get the first exception. @@ -149,7 +149,7 @@ public function exceptionHandler(Throwable $exception) 'message' => $prevException->getMessage(), 'exFile' => clean_path($prevException->getFile()), // {file} refers to THIS file 'exLine' => $prevException->getLine(), // {line} refers to THIS line - 'trace' => self::renderBacktrace($prevException->getTrace()), + 'trace' => render_backtrace($prevException->getTrace()), ]); } } @@ -527,7 +527,7 @@ private function handleDeprecationError(string $message, ?string $file = null, ? 'message' => $message, 'errFile' => clean_path($file ?? ''), 'errLine' => $line ?? 0, - 'trace' => self::renderBacktrace($trace), + 'trace' => render_backtrace($trace), ] ); @@ -646,41 +646,4 @@ public static function highlightFile(string $file, int $lineNumber, int $lines = return '
' . $out . '
'; } - - private static function renderBacktrace(array $backtrace): string - { - $backtraces = []; - - foreach ($backtrace as $index => $trace) { - $frame = $trace + ['file' => '[internal function]', 'line' => '', 'class' => '', 'type' => '', 'args' => []]; - - if ($frame['file'] !== '[internal function]') { - $frame['file'] = sprintf('%s(%s)', $frame['file'], $frame['line']); - } - - unset($frame['line']); - $idx = $index; - $idx = str_pad((string) ++$idx, 2, ' ', STR_PAD_LEFT); - - $args = implode(', ', array_map(static fn ($value): string => match (true) { - is_object($value) => sprintf('Object(%s)', $value::class), - is_array($value) => $value !== [] ? '[...]' : '[]', - $value === null => 'null', - is_resource($value) => sprintf('resource (%s)', get_resource_type($value)), - default => var_export($value, true), - }, $frame['args'])); - - $backtraces[] = sprintf( - '%s %s: %s%s%s(%s)', - $idx, - clean_path($frame['file']), - $frame['class'], - $frame['type'], - $frame['function'], - $args - ); - } - - return implode("\n", $backtraces); - } } diff --git a/tests/system/CommonFunctionsTest.php b/tests/system/CommonFunctionsTest.php index f2c81684b623..4524edd5d56d 100644 --- a/tests/system/CommonFunctionsTest.php +++ b/tests/system/CommonFunctionsTest.php @@ -805,4 +805,14 @@ public function testIsWindowsUsingMock(): void $this->assertSame(str_contains(php_uname(), 'Windows'), is_windows()); $this->assertSame(defined('PHP_WINDOWS_VERSION_MAJOR'), is_windows()); } + + public function testRenderBacktrace(): void + { + $trace = (new RuntimeException('Test exception'))->getTrace(); + $renders = explode("\n", render_backtrace($trace)); + + foreach ($renders as $render) { + $this->assertMatchesRegularExpression('/^\s*\d* .+(?:\(\d+\))?: \S+(?:(?:\->|::)\S+)?\(.*\)$/', $render); + } + } } diff --git a/tests/system/Database/Live/ExecuteLogMessageFormatTest.php b/tests/system/Database/Live/ExecuteLogMessageFormatTest.php new file mode 100644 index 000000000000..7017cb91b53d --- /dev/null +++ b/tests/system/Database/Live/ExecuteLogMessageFormatTest.php @@ -0,0 +1,62 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Database\Live; + +use CodeIgniter\Test\CIUnitTestCase; +use CodeIgniter\Test\DatabaseTestTrait; +use CodeIgniter\Test\TestLogger; +use Config\Database; +use PHPUnit\Framework\Attributes\Group; +use PHPUnit\Framework\TestCase; +use ReflectionProperty; + +/** + * @internal + */ +#[Group('DatabaseLive')] +final class ExecuteLogMessageFormatTest extends TestCase +{ + public function testLogMessageWhenExecuteFailsShowFullStructuredBacktrace(): void + { + $db = Database::connect((new Database)->tests, false); + (new ReflectionProperty($db, 'DBDebug'))->setValue($db, false); + + $sql = 'SELECT * FROM some_table WHERE id = ? AND status = ? AND author = ?'; + $db->query($sql, [3, 'live', 'Rick']); + + $message = match ($db->DBDriver) { + 'MySQLi' => 'Table \'test.some_table\' doesn\'t exist', + 'Postgre' => 'pg_query(): Query failed: ERROR: relation "some_table" does not exist', + 'SQLite3' => 'Unable to prepare statement: no such table: some_table', + 'OCI8' => 'oci_execute(): ORA-00942: table or view does not exist', + 'SQLSRV' => '[Microsoft][ODBC Driver 18 for SQL Server][SQL Server]Invalid object name \'some_table\'. SQLSTATE: 42S02, code: 208', + default => 'Unknown DB error', + }; + $messageFromLogs = explode("\n", (new ReflectionProperty(TestLogger::class, 'op_logs'))->getValue()[0]['message']); + + $this->assertSame($message, array_shift($messageFromLogs)); + + if ($db->DBDriver === 'Postgre') { + $messageFromLogs = array_slice($messageFromLogs, 2); + } elseif ($db->DBDriver === 'OCI8') { + $messageFromLogs = array_slice($messageFromLogs, 1); + } + + $this->assertMatchesRegularExpression('/^in \S+ on line \d+\.$/', array_shift($messageFromLogs)); + + foreach ($messageFromLogs as $line) { + $this->assertMatchesRegularExpression('/^\s*\d* .+(?:\(\d+\))?: \S+(?:(?:\->|::)\S+)?\(.*\)$/', $line); + } + } +} diff --git a/tests/system/Debug/ExceptionsTest.php b/tests/system/Debug/ExceptionsTest.php index 00dbd94269b6..ebb00a6159d2 100644 --- a/tests/system/Debug/ExceptionsTest.php +++ b/tests/system/Debug/ExceptionsTest.php @@ -128,22 +128,6 @@ public function testDetermineCodes(): void $this->assertSame([500, EXIT_DATABASE], $determineCodes(new DatabaseException('This.'))); } - public function testRenderBacktrace(): void - { - $renderer = self::getPrivateMethodInvoker(Exceptions::class, 'renderBacktrace'); - $exception = new RuntimeException('This.'); - - $renderedBacktrace = $renderer($exception->getTrace()); - $renderedBacktrace = explode("\n", $renderedBacktrace); - - foreach ($renderedBacktrace as $trace) { - $this->assertMatchesRegularExpression( - '/^\s*\d* .+(?:\(\d+\))?: \S+(?:(?:\->|::)\S+)?\(.*\)$/', - $trace - ); - } - } - public function testMaskSensitiveData(): void { $maskSensitiveData = $this->getPrivateMethodInvoker($this->exception, 'maskSensitiveData'); diff --git a/utils/phpstan-baseline/missingType.iterableValue.neon b/utils/phpstan-baseline/missingType.iterableValue.neon index 4dac1bf252c4..ac9423e03f98 100644 --- a/utils/phpstan-baseline/missingType.iterableValue.neon +++ b/utils/phpstan-baseline/missingType.iterableValue.neon @@ -1,4 +1,4 @@ -# total 1672 errors +# total 1670 errors parameters: ignoreErrors: @@ -2417,11 +2417,6 @@ parameters: count: 1 path: ../../system/Debug/Exceptions.php - - - message: '#^Method CodeIgniter\\Debug\\Exceptions\:\:renderBacktrace\(\) has parameter \$backtrace with no value type specified in iterable type array\.$#' - count: 1 - path: ../../system/Debug/Exceptions.php - - message: '#^Method CodeIgniter\\Debug\\Exceptions\:\:respond\(\) has parameter \$data with no value type specified in iterable type array\.$#' count: 1