fix(core): stop ActiveSync log spam and folder loss on IMAP outage#193
Merged
Conversation
When the mail server is unreachable during ActiveSync polling, a single IMAP connection failure was caught, logged at ERR/WARN, and re-thrown at every layer (IMAP factory, _getMailFolders, getFolders), producing several log lines per poll per device. getFolders() also returned an empty array, which made FOLDERSYNC diff against an empty hierarchy and could make clients drop all their mail folders. Classify transient connection failures (DISCONNECT / SERVER_CONNECT / LOGIN_UNAVAILABLE, detected via the preserved exception chain) in the IMAP factory: log once at NOTICE and throw Horde_ActiveSync_Exception_TemporaryFailure. Drop the redundant re-logging in the driver, and have getFolderList() return false on a temporary failure so the state engine skips folder-change detection this round and the device retries later instead of losing folders. Genuine (non-connection) errors are unchanged: still logged at ERR.
ralflang
requested changes
Jul 8, 2026
| // 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). | ||
| Horde::log(sprintf( |
Member
There was a problem hiding this comment.
Don't use Horde::log - inject a logger.
Member
|
Hard-error IMAP failures still return [] from getFolders(). That would still allow hard errors to trigger clients deleting folders. |
…d IMAP errors Incorporate review feedback on the ActiveSync IMAP outage handling: - Imap_Factory: inject a Horde_Log_Logger via the constructor instead of the static Horde::log() facade (wired from the injector in the ActiveSyncBackend factory); a null logger is used when none is given. - Driver::getFolders(): propagate hard IMAP errors instead of returning an empty/partial hierarchy. Returning [] let FOLDERSYNC diff against a reduced list and could make clients delete the user's mail folders - the same risk previously only guarded for transient failures. - Driver::getFolderList(): catch the base Horde_ActiveSync_Exception so any folder-enumeration failure (transient or hard) skips folder-change detection this round; the device retries later. Log severity is still decided once in the factory (NOTICE transient, ERR hard).
ralflang
approved these changes
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Horde_ActiveSync_Exception_TemporaryFailure.Motivation
With a device polling ActiveSync while the IMAP server is down, one connection failure was caught, logged, and re-thrown at every layer, spamming
horde.logat ERR/WARN once per poll per device.getFolders()returning[]also caused FOLDERSYNC to diff against an empty hierarchy, risking removal of the user's mail folders on the device.Changes
Horde_Core_ActiveSync_Imap_Factory::getMailboxes(): new_isTransientImapError()walks the exception chain (the underlyingHorde_Imap_Client_Exceptioncode survives inside wrappers such as the injector's "Cannot create IMP_Ftree"). Transient failures log once atNOTICEand throwHorde_ActiveSync_Exception_TemporaryFailure; other errors keep theERR+Horde_ActiveSync_Exceptionbehavior.Horde_Core_ActiveSync_Driver::_getMailFolders(): drop the duplicateERRre-log (kept a debug trace), re-throw preserving the exception type.Horde_Core_ActiveSync_Driver::getFolders(): propagateTemporaryFailureinstead of returning[]; downgrade the non-transient log to debug.Horde_Core_ActiveSync_Driver::getFolderList(): catchTemporaryFailureand returnfalse, the state engine's "skip folder diff" signal.Dependencies
Relies on horde/ActiveSync PR "report no folder changes when getFolderList() is false" so the
falsereturn is handled without acount(null)fatal. Merge that first.Test plan
horde.logshows a single NOTICE per poll (not the ERR/WARN cascade); FOLDERSYNC returns no changes with the same synckey; device keeps its folders and recovers when IMAP returns.