[VUFIND-1793] Logger improvements

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(

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,
-            ]
-        );
     }
 
     /**

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);
         }
     }
 

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)) {

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 = [

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'
+        );
+    }
 }