From 332c2a6288f0ff8082f571b3e3b48f0b085f7761 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 26 Feb 2025 16:03:21 +0000 Subject: [PATCH 1/3] Migrated throwable parsing from storm to winter --- modules/system/models/EventLog.php | 203 ++++++++++++++++++++++++++++- 1 file changed, 199 insertions(+), 4 deletions(-) diff --git a/modules/system/models/EventLog.php b/modules/system/models/EventLog.php index 2d2efc358e..22c2b24856 100644 --- a/modules/system/models/EventLog.php +++ b/modules/system/models/EventLog.php @@ -1,9 +1,11 @@ message = $message; @@ -59,6 +64,25 @@ public static function add(string $message, string $level = 'info', ?array $deta return $record; } + /** + * Creates an exception log record + */ + public static function addException(Throwable $throwable, string $level = 'error'): static + { + $record = new static; + $record->message = $throwable->getMessage(); + $record->level = $level; + $record->details = $record->getDetails($throwable); + + try { + $record->save(); + } + catch (Exception $ex) { + } + + return $record; + } + /** * Beautify level value. */ @@ -82,4 +106,175 @@ public function getSummaryAttribute(): string return Str::limit($matches[1] ?? '', 500); } + + + /** + * Constructs the details array for logging + * + * @param Throwable $throwable + * @return array + */ + public function getDetails(Throwable $throwable): array + { + return [ + 'logVersion' => static::EXCEPTION_LOG_VERSION, + 'exception' => $this->exceptionToArray($throwable), + 'environment' => $this->getEnviromentInfo(), + ]; + } + + /** + * Convert a throwable into an array of data for logging + * + * @param Throwable $throwable + * @return array + */ + protected function exceptionToArray(\Throwable $throwable): array + { + return [ + 'type' => $throwable::class, + 'message' => $throwable->getMessage(), + 'file' => $throwable->getFile(), + 'line' => $throwable->getLine(), + 'snippet' => $this->getSnippet($throwable->getFile(), $throwable->getLine()), + 'trace' => $this->exceptionTraceToArray($throwable->getTrace()), + 'stringTrace' => $throwable->getTraceAsString(), + 'code' => $throwable->getCode(), + 'previous' => $throwable->getPrevious() + ? $this->exceptionToArray($throwable->getPrevious()) + : null, + ]; + } + + /** + * Generate an array trace with extra data not provided by the default trace + * + * @param array $trace + * @return array + * @throws \ReflectionException + */ + protected function exceptionTraceToArray(array $trace): array + { + foreach ($trace as $index => $frame) { + if (!isset($frame['file']) && isset($frame['class'])) { + $ref = new ReflectionClass($frame['class']); + $frame['file'] = $ref->getFileName(); + + if (!isset($frame['line']) && isset($frame['function']) && !str_contains($frame['function'], '{')) { + foreach (file($frame['file']) as $line => $text) { + if (preg_match(sprintf('/function\s.*%s/', $frame['function']), $text)) { + $frame['line'] = $line + 1; + break; + } + } + } + } + + $trace[$index] = [ + 'file' => $frame['file'] ?? null, + 'line' => $frame['line'] ?? null, + 'function' => $frame['function'] ?? null, + 'class' => $frame['class'] ?? null, + 'type' => $frame['type'] ?? null, + 'snippet' => !empty($frame['file']) && !empty($frame['line']) + ? $this->getSnippet($frame['file'], $frame['line']) + : '', + 'in_app' => ($frame['file'] ?? null) ? $this->isInAppError($frame['file']) : false, + 'arguments' => array_map(function ($arg) { + if (is_numeric($arg)) { + return $arg; + } + if (is_string($arg)) { + return "'$arg'"; + } + if (is_null($arg)) { + return 'null'; + } + if (is_bool($arg)) { + return $arg ? 'true' : 'false'; + } + if (is_array($arg)) { + return 'Array'; + } + if (is_object($arg)) { + return get_class($arg); + } + if (is_resource($arg)) { + return 'Resource'; + } + }, $frame['args'] ?? []), + ]; + } + + return $trace; + } + + /** + * Get the code snippet referenced in a trace + * + * @param string $file + * @param int $line + * @return array + */ + protected function getSnippet(string $file, int $line): array + { + if (str_contains($file, ': eval()\'d code')) { + return []; + } + + $lines = file($file); + + if (count($lines) < static::EXCEPTION_SNIPPET_LINES) { + return $lines; + } + + return array_slice( + $lines, + $line - (static::EXCEPTION_SNIPPET_LINES / 2), + static::EXCEPTION_SNIPPET_LINES, + true + ); + } + + /** + * Get environment details to record with the exception + * + * @return array + */ + protected function getEnviromentInfo(): array + { + if (app()->runningInConsole()) { + return [ + 'context' => 'CLI', + 'testing' => app()->runningUnitTests(), + 'env' => app()->environment(), + ]; + } + + return [ + 'context' => 'Web', + 'backend' => method_exists(app(), 'runningInBackend') ? app()->runningInBackend() : false, + 'testing' => app()->runningUnitTests(), + 'url' => app('url')->current(), + 'method' => app('request')->method(), + 'env' => app()->environment(), + 'ip' => app('request')->ip(), + 'userAgent' => app('request')->header('User-Agent'), + ]; + } + + /** + * Helper to work out if a file should be considered "In App" or not + * + * @param string $file + * @return bool + */ + protected function isInAppError(string $file): bool + { + if (basename($file) === 'index.php' || basename($file) === 'artisan') { + return false; + } + + return !\Winter\Storm\Support\Str::startsWith($file, base_path('vendor')) && !Str::startsWith($file, base_path('modules')); + } } From 5ded270398e374b54794d1e102fee76807352827 Mon Sep 17 00:00:00 2001 From: Jack Wilkinson Date: Wed, 26 Feb 2025 16:04:23 +0000 Subject: [PATCH 2/3] Added event handler specifically for exceptions and prevented the default logger from writing exception logs --- modules/system/ServiceProvider.php | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/modules/system/ServiceProvider.php b/modules/system/ServiceProvider.php index 3a8b58f301..774694d0e0 100644 --- a/modules/system/ServiceProvider.php +++ b/modules/system/ServiceProvider.php @@ -356,9 +356,23 @@ protected function registerErrorHandler() protected function registerLogging() { Event::listen(\Illuminate\Log\Events\MessageLogged::class, function ($event) { + if (!EventLog::useLogging()) { + return; + } + + $details = $event->context ?? null; + + // This allows for preventing db logging in cases where we don't want all log messages logged to the DB. + if (isset($details['skipDatabaseLog']) && $details['skipDatabaseLog']) { + return; + } + + EventLog::add($event->message, $event->level, $details); + }); + + Event::listen('exception.report', function (\Throwable $throwable) { if (EventLog::useLogging()) { - $details = $event->context ?? null; - EventLog::add($event->message, $event->level, $details); + EventLog::addException($throwable); } }); } From 6cdcf1a5dee97efa76552f8de0dbeef2a64591f0 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Wed, 26 Feb 2025 10:42:54 -0600 Subject: [PATCH 3/3] Apply suggestions from code review --- modules/system/models/EventLog.php | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/modules/system/models/EventLog.php b/modules/system/models/EventLog.php index 22c2b24856..027f19e564 100644 --- a/modules/system/models/EventLog.php +++ b/modules/system/models/EventLog.php @@ -107,12 +107,8 @@ public function getSummaryAttribute(): string return Str::limit($matches[1] ?? '', 500); } - /** * Constructs the details array for logging - * - * @param Throwable $throwable - * @return array */ public function getDetails(Throwable $throwable): array { @@ -125,11 +121,8 @@ public function getDetails(Throwable $throwable): array /** * Convert a throwable into an array of data for logging - * - * @param Throwable $throwable - * @return array */ - protected function exceptionToArray(\Throwable $throwable): array + protected function exceptionToArray(Throwable $throwable): array { return [ 'type' => $throwable::class, @@ -149,8 +142,6 @@ protected function exceptionToArray(\Throwable $throwable): array /** * Generate an array trace with extra data not provided by the default trace * - * @param array $trace - * @return array * @throws \ReflectionException */ protected function exceptionTraceToArray(array $trace): array @@ -211,10 +202,6 @@ protected function exceptionTraceToArray(array $trace): array /** * Get the code snippet referenced in a trace - * - * @param string $file - * @param int $line - * @return array */ protected function getSnippet(string $file, int $line): array { @@ -238,8 +225,6 @@ protected function getSnippet(string $file, int $line): array /** * Get environment details to record with the exception - * - * @return array */ protected function getEnviromentInfo(): array { @@ -265,9 +250,6 @@ protected function getEnviromentInfo(): array /** * Helper to work out if a file should be considered "In App" or not - * - * @param string $file - * @return bool */ protected function isInAppError(string $file): bool { @@ -275,6 +257,6 @@ protected function isInAppError(string $file): bool return false; } - return !\Winter\Storm\Support\Str::startsWith($file, base_path('vendor')) && !Str::startsWith($file, base_path('modules')); + return !Str::startsWith($file, base_path('vendor')) && !Str::startsWith($file, base_path('modules')); } }