diff --git a/module/VuFind/src/VuFind/Action/Combined/ResultsAction.php b/module/VuFind/src/VuFind/Action/Combined/ResultsAction.php index 0b1dcedd1d15..c7039bd79859 100644 --- a/module/VuFind/src/VuFind/Action/Combined/ResultsAction.php +++ b/module/VuFind/src/VuFind/Action/Combined/ResultsAction.php @@ -89,21 +89,8 @@ public function action( } catch (\Exception $e) { // Prevent errors from any of the combined search results // from raising up to the user interface and instead just skip them - $baseMsg = "Failed get combined options for {$searchClassId}."; - $shortDetails = $e->getMessage(); - $fullDetails = (string)$e; - $this->logError( - $baseMsg, - [ - 'details' => [ - 1 => "$baseMsg $shortDetails", - 2 => "$baseMsg $shortDetails", - 3 => "$baseMsg $shortDetails", - 4 => "$baseMsg $fullDetails", - 5 => "$baseMsg $fullDetails", - ], - ] - ); + $this->logError("Failed to get combined options for {$searchClassId}."); + $this->logException($e); continue; } $adjustedRequest = $this->adjustQueryForSettings( diff --git a/module/VuFind/src/VuFind/Log/Logger.php b/module/VuFind/src/VuFind/Log/Logger.php index 51e085bfbc60..a109c4c9eb48 100644 --- a/module/VuFind/src/VuFind/Log/Logger.php +++ b/module/VuFind/src/VuFind/Log/Logger.php @@ -268,13 +268,55 @@ public function log($level, string|\Stringable $message, array $context = []): v ? self::LEVEL_MAP[$level] : $level; - if (is_array($message)) { - $context['vufind_log_details'] = $message; - $mainMonologMessage = 'Exception/Detailed log. See context for levels.'; - } else { - $mainMonologMessage = $message; + // If the details is set in context, fill in any missing parts + $context = $this->fillInMissingDetails($context); + + $this->monologLogger->log($monologLevel, $message, $context); + } + + /** + * If there are 'details' in the context that are missing any of the key keys + * (for each verbosity), use data from the next lower index, if that index is missing, + * instead use the next higher index. Leave blank as a last resort. + * + * @param mixed[] $context Additional context data + * + * @return mixed[] + */ + protected function fillInMissingDetails(array $context = []): array + { + if (!is_array($context['details'] ?? null)) { + return $context; + } + $details = $context['details']; + $levels = [1, 2, 3, 4, 5]; + $filledDetails = []; + + foreach ($levels as $level) { + // This level has data, no need to look further + if (($details[$level] ?? '') !== '') { + $filledDetails[$level] = $details[$level]; + continue; + } + + // Try prior index (Backfill) + if (($details[$level - 1] ?? '') !== '') { + $filledDetails[$level] = $details[$level - 1]; + continue; + } + + // Try next index (Frontfill) + if (($details[$level + 1] ?? '') !== '') { + $filledDetails[$level] = $details[$level + 1]; + continue; + } + + // Leave blank as last resort + $filledDetails[$level] = ''; } - $this->monologLogger->log($monologLevel, $mainMonologMessage, $context); + + $context['details'] = $filledDetails; + return $context; } /** @@ -299,10 +341,32 @@ public function debugNeeded($newState = null) * * @param \Exception $error Exception to log * @param \Laminas\Stdlib\Parameters $server Server metadata + * @param mixed $level Optional log level. Will determine from the + * exception if not provided. (e.g., 'err', 'warn') * * @return void */ - public function logException($error, $server) + public function logException($error, $server, $level = null) + { + $details = $this->getDetailsFromException($error, $server); + $this->log( + $level ?? $this->getSeverityFromException($error), + $details[1] ?? 'Exception/Detailed log. See context for levels.', + compact('details') + ); + } + + /** + * Extract the error data from the exception object and server data, + * if provided, and convert it into an array with keys for each of + * the 5 verbosity levels. + * + * @param \Exception $error Exception to log + * @param \Laminas\Stdlib\Parameters $server Server metadata + * + * @return array + */ + protected function getDetailsFromException($error, $server): array { // We need to build a variety of pieces so we can supply // information at five different verbosity levels: @@ -353,21 +417,13 @@ public function logException($error, $server) } } - $errorDetails = [ + return [ 1 => $baseError, 2 => $baseError . $basicServer, 3 => $baseError . $basicServer . $basicBacktrace, 4 => $baseError . $detailedServer . $basicBacktrace, 5 => $baseError . $detailedServer . $detailedBacktrace, ]; - - $this->log( - $this->getSeverityFromException($error), - $baseError, - [ - 'details' => $errorDetails, - ] - ); } /** diff --git a/module/VuFind/src/VuFind/Log/LoggerAwareTrait.php b/module/VuFind/src/VuFind/Log/LoggerAwareTrait.php index b479833e4a86..ddd7633e7d0c 100644 --- a/module/VuFind/src/VuFind/Log/LoggerAwareTrait.php +++ b/module/VuFind/src/VuFind/Log/LoggerAwareTrait.php @@ -34,6 +34,7 @@ use Psr\Log\LogLevel; use function get_class; +use function is_array; /** * Implementation of PSR-3 \Psr\Log\LoggerAwareTrait with some additional convenience methods. @@ -87,14 +88,20 @@ protected function logError($msg, array $context = [], $prependClass = true) /** * Log an exception. * - * @param \Exception $exception Exception to log + * @param \Exception $exception Exception to log + * @param \Laminas\Stdlib\Parameters|array|null $server Optional server metadata + * @param mixed $level Optional log level. Will determine from the + * exception if not provided. (e.g., 'err', 'warn') * * @return void */ - public function logException(\Exception $exception): void + public function logException(\Exception $exception, $server = null, $level = null): void { if ($this->logger instanceof ExtendedLoggerInterface) { - $this->logger->logException($exception, new \Laminas\Stdlib\Parameters()); + if (is_array($server)) { + $server = new \Laminas\Stdlib\Parameters($server); + } + $this->logger->logException($exception, $server ?? new \Laminas\Stdlib\Parameters(), $level); } } diff --git a/module/VuFind/src/VuFind/View/Helper/Root/SearchBox.php b/module/VuFind/src/VuFind/View/Helper/Root/SearchBox.php index cdeeb6d0a4d5..38a949a9e5c9 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/SearchBox.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/SearchBox.php @@ -201,21 +201,8 @@ public function autocompleteFormattingRulesJson($activeSearchClass): string } catch (\Exception $e) { // Log a warning and ignore when we can't add the autocomplete rules for // any of the handlers - $baseMsg = "Could not determine autocomplete formatting rules for {$target}."; - $shortDetails = $e->getMessage(); - $fullDetails = (string)$e; - $this->logWarning( - $baseMsg, - [ - 'details' => [ - 1 => "$baseMsg $shortDetails", - 2 => "$baseMsg $shortDetails", - 3 => "$baseMsg $shortDetails", - 4 => "$baseMsg $fullDetails", - 5 => "$baseMsg $fullDetails", - ], - ] - ); + $this->logWarning("Could not determine autocomplete formatting rules for {$target}."); + $this->logException($e, level: 'warn'); } } } @@ -552,21 +539,8 @@ protected function getCombinedHandlers($activeSearchClass, $activeHandler, array } catch (\Exception $e) { // If we can't get the options or basic handlers for the search // target, then log it and don't add it to the search box - $baseMsg = "Missing required data for {$target}. Could not add to search box."; - $shortDetails = $e->getMessage(); - $fullDetails = (string)$e; - $this->logError( - $baseMsg, - [ - 'details' => [ - 1 => "$baseMsg $shortDetails", - 2 => "$baseMsg $shortDetails", - 3 => "$baseMsg $shortDetails", - 4 => "$baseMsg $fullDetails", - 5 => "$baseMsg $fullDetails", - ], - ] - ); + $this->logError("Missing required data for {$target}. Could not add to search box."); + $this->logException($e); continue; } if (empty($basic)) { diff --git a/module/VuFind/src/VuFind/View/Helper/Root/SearchTabs.php b/module/VuFind/src/VuFind/View/Helper/Root/SearchTabs.php index 06196b13b4c7..5ead95a76500 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/SearchTabs.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/SearchTabs.php @@ -149,21 +149,8 @@ public function getTabConfig( } } catch (\Exception $e) { // Log the error and just don't add tabs that we couldn't get the data for - $baseMsg = "Could not add tab for {$key}."; - $shortDetails = $e->getMessage(); - $fullDetails = (string)$e; - $this->logError( - $baseMsg, - [ - 'details' => [ - 1 => "$baseMsg $shortDetails", - 2 => "$baseMsg $shortDetails", - 3 => "$baseMsg $shortDetails", - 4 => "$baseMsg $fullDetails", - 5 => "$baseMsg $fullDetails", - ], - ] - ); + $this->logError("Could not add tab for {$key}."); + $this->logException($e); continue; } $tab = [ diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Log/LoggerTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Log/LoggerTest.php index db191452c28c..efe550bb6e55 100644 --- a/module/VuFind/tests/unit-tests/src/VuFindTest/Log/LoggerTest.php +++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Log/LoggerTest.php @@ -136,4 +136,55 @@ public function testLogException() $logger->logException($e, $fakeServer); } } + + /** + * Test fillInMissingDetails(). + * + * @return void + */ + public function testFillInMissingDetails() + { + $mockIpReader = $this->createMock(\VuFind\Net\UserIpReader::class); + $logger = $this->getMockBuilder(\VuFind\Log\Logger::class) + ->setConstructorArgs([$mockIpReader, new \Monolog\Logger('test')]) + ->onlyMethods(['log']) + ->getMock(); + + // Test 1: Gap in the middle (Index 3 missing) + $context = [ + 'details' => [ + 1 => 'low', + 2 => 'med', + // 3 is missing + 4 => 'high', + 5 => 'ultra', + ], + ]; + + $method = new \ReflectionMethod($logger, 'fillInMissingDetails'); + $method->setAccessible(true); + + $result = $method->invoke($logger, $context); + + $this->assertEquals('med', $result['details'][3], 'Index 3 should backfill from Index 2'); + + // Test 2: Missing start (Index 1 missing) + $context2 = ['details' => [2 => 'value']]; + $result2 = $method->invoke($logger, $context2); + $this->assertEquals('value', $result2['details'][1], 'Index 1 should frontfill from Index 2'); + + // Test 3: Empty string in index + $context3 = ['details' => [1 => 'data', 2 => '', 3 => 'more data']]; + $result3 = $method->invoke($logger, $context3); + $this->assertEquals('data', $result3['details'][2], 'Empty string should be treated as missing'); + + // Test 4: No index to backfill or frontfill from + $context4 = ['details' => [1 => 'data']]; + $result4 = $method->invoke($logger, $context4); + $this->assertEquals( + '', + $result4['details'][3], + 'Index 3-5 should be empty string since no near indexes to fill from' + ); + } }