diff --git a/lib/Horde/Core/ActiveSync/Driver.php b/lib/Horde/Core/ActiveSync/Driver.php index 9654b694..7459663d 100644 --- a/lib/Horde/Core/ActiveSync/Driver.php +++ b/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() ) ); diff --git a/lib/Horde/Core/ActiveSync/Imap/Factory.php b/lib/Horde/Core/ActiveSync/Imap/Factory.php index bc42b82c..01d33f9f 100644 --- a/lib/Horde/Core/ActiveSync/Imap/Factory.php +++ b/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 + */ + 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 + * + * @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; + } + } diff --git a/lib/Horde/Core/Factory/ActiveSyncBackend.php b/lib/Horde/Core/Factory/ActiveSyncBackend.php index 4c030326..37ac9540 100644 --- a/lib/Horde/Core/Factory/ActiveSyncBackend.php +++ b/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.