fix(core): stop ActiveSync log spam and folder loss on IMAP outage

lib/Horde/Core/ActiveSync/Driver.php

@@ -585,13 +585,27 @@ public function getWasteBasket($class)
     /**
      * Return an array of stats for the server's folder list.
      *
-     * @return array  An array of folder stats @see self::statFolder()
+     * @return array|boolean  An array of folder stats
+     *                        (@see self::statFolder()), or false if the folder
+     *                        hierarchy could not be built and folder change
+     *                        detection should be skipped this round.
      * @todo Horde 6 move to base class
      */
     public function getFolderList()
     {
         $this->_logger->meta('Horde_Core_ActiveSync_Driver::getFolderList()');
-        $folderlist = $this->getFolders();
+        try {
+            $folderlist = $this->getFolders();
+        } catch (Horde_ActiveSync_Exception $e) {
+            // The folder hierarchy could not be built - the mail server is
+            // temporarily unavailable or returned a hard error. Returning
+            // false tells the state engine to skip folder-change detection
+            // this round instead of computing a diff against an incomplete
+            // hierarchy (which would delete the client's mail folders). The
+            // device retries on the next sync. The root cause was already
+            // logged once at its appropriate severity by the IMAP factory.
+            return false;
+        }
         $folders = [];
         foreach ($folderlist as $f) {
             $folders[] = $this->statFolder(
@@ -610,6 +624,8 @@ public function getFolderList()
      * Return an array of the server's folder objects.
      *
      * @return array  An array of Horde_ActiveSync_Message_Folder objects.
+     * @throws Horde_ActiveSync_Exception_TemporaryFailure  If the mail server
+     *         is temporarily unreachable (transient connection failure).
      */
     public function getFolders()
     {
@@ -777,9 +793,17 @@ public function getFolders()
             try {
                 $folders = array_merge($folders, $this->_getMailFolders());
             } catch (Horde_ActiveSync_Exception $e) {
-                $this->_logger->err($e->getMessage());
+                // The mail folders could not be retrieved - either a transient
+                // outage (Horde_ActiveSync_Exception_TemporaryFailure) or a
+                // hard IMAP error. In neither case may we fall through to
+                // returning a partial hierarchy that omits the mail folders:
+                // FOLDERSYNC would diff against the reduced list and the client
+                // could delete the user's mail folders. Propagate so the caller
+                // (getFolderList()) can skip folder-change detection this round;
+                // the device retries later. The root cause was already logged
+                // once by the IMAP factory (NOTICE for transient, ERR for hard).
                 $this->_endBuffer();
-                return [];
+                throw $e;
             }
 
             $this->_endBuffer();
@@ -4122,9 +4146,14 @@ protected function _getMailFolders()
                 try {
                     $imap_folders = $this->_imap->getMailboxes(true);
                 } catch (Horde_ActiveSync_Exception $e) {
-                    $this->_logger->err(
+                    // The IMAP factory already logged the root cause once; do
+                    // not re-log the same failure at every layer. Keep a debug
+                    // trace and re-throw the original (which preserves the
+                    // Horde_ActiveSync_Exception_TemporaryFailure type on a
+                    // transient connection failure).
+                    $this->_logger->meta(
                         sprintf(
-                            'Problem loading mail folders from IMAP server: %s',
+                            'Mail folder list unavailable: %s',
                             $e->getMessage()
                         )
                     );

lib/Horde/Core/ActiveSync/Imap/Factory.php

@@ -7,8 +7,6 @@
  * @package   Core
  */
 
-use Horde\Core\Horde;
-
 /**
  * Horde_Core_ActiveSync_Imap_Factory implements a factory/builder for
  * providing a Horde_ActiveSync_Imap_Adapter object as well as building a tree
@@ -25,6 +23,30 @@ class Horde_Core_ActiveSync_Imap_Factory implements Horde_ActiveSync_Interface_I
     protected $_mailboxlist;
     protected $_specialMailboxlist;
 
+    /**
+     * Logger.
+     *
+     * @var Horde_Log_Logger
+     */
+    protected $_logger;
+
+    /**
+     * Constructor.
+     *
+     * @param array $params  Parameters:
+     *   - logger: (Horde_Log_Logger) The logger to use. Optional; a null
+     *             logger is used when none is supplied.
+     *
+     * @author Torben Dannhauer <torben@dannhauer.de>
+     */
+    public function __construct(array $params = [])
+    {
+        $this->_logger = (isset($params['logger'])
+            && $params['logger'] instanceof Horde_Log_Logger)
+            ? $params['logger']
+            : new Horde_Log_Logger(new Horde_Log_Handler_Null());
+    }
+
     /**
      * Return a Horde_Imap_Client
      *
@@ -86,7 +108,20 @@ public function getMailboxes($force = false)
                     }
                 }
             } catch (Horde_Exception $e) {
-                Horde::log(sprintf(
+                if ($this->_isTransientImapError($e)) {
+                    // The mail server is temporarily unreachable. This is an
+                    // expected, recoverable condition during ActiveSync
+                    // polling, so log it once here at a low severity and let
+                    // the caller defer the sync instead of treating it as a
+                    // hard error (which spams the log at every layer and can
+                    // make clients drop their mail folders).
+                    $this->_logger->log(sprintf(
+                        'Mail server temporarily unavailable while retrieving mailbox list: %s',
+                        $e->getMessage()
+                    ), Horde_Log::NOTICE);
+                    throw new Horde_ActiveSync_Exception_TemporaryFailure($e);
+                }
+                $this->_logger->log(sprintf(
                     'Error retrieving mailbox list: %s',
                     $e->getMessage()
                 ), Horde_Log::ERR);
@@ -119,7 +154,7 @@ public function getSpecialMailboxes()
             try {
                 $this->_specialMailboxlist = $registry->mail->getSpecialMailboxes();
             } catch (Horde_Exception $e) {
-                Horde::log(sprintf(
+                $this->_logger->log(sprintf(
                     'Error retrieving specialmailbox list: %s',
                     $e->getMessage()
                 ), Horde_Log::ERR);
@@ -174,4 +209,37 @@ public function getMsgFlags()
         return $msgFlags;
     }
 
+    /**
+     * Determine whether an exception represents a transient IMAP failure
+     * (mail server unreachable or connection dropped) as opposed to a hard
+     * error such as an authentication or configuration problem.
+     *
+     * The relevant Horde_Imap_Client_Exception is preserved in the previous
+     * exception chain even when wrapped by outer exceptions (e.g. the
+     * injector's "Cannot create IMP_Ftree"), so walk the chain to classify.
+     *
+     * @author Torben Dannhauer <torben@dannhauer.de>
+     *
+     * @param Throwable $e  The caught exception.
+     *
+     * @return boolean  True if the failure looks transient/connection related.
+     */
+    protected function _isTransientImapError($e)
+    {
+        $transient = [
+            Horde_Imap_Client_Exception::DISCONNECT,
+            Horde_Imap_Client_Exception::SERVER_CONNECT,
+            Horde_Imap_Client_Exception::LOGIN_UNAVAILABLE,
+        ];
+
+        for ($current = $e; $current !== null; $current = $current->getPrevious()) {
+            if ($current instanceof Horde_Imap_Client_Exception
+                && in_array($current->getCode(), $transient, true)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
 }

lib/Horde/Core/Factory/ActiveSyncBackend.php

@@ -31,7 +31,9 @@ public function create(Horde_Injector|Injector $injector)
 
         // Backend driver and dependencies
         $params = ['registry' => $registry];
-        $adapter_params = ['factory' => new Horde_Core_ActiveSync_Imap_Factory()];
+        $adapter_params = ['factory' => new Horde_Core_ActiveSync_Imap_Factory([
+            'logger' => $injector->getInstance('Horde_Log_Logger'),
+        ])];
 
         // Determine emailsync setting - force to off if we don't have a mail API.
         // Use local variable instead of mutating global $conf.