Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 05ce6549a4 | |||
| 3daee6b278 | |||
| aacf321ef7 | |||
| 823de28f56 | |||
| 0269bc6d28 |
+1
-1
@@ -1,2 +1,2 @@
|
||||
parameters:
|
||||
app.version: '0.4.53'
|
||||
app.version: '0.4.54'
|
||||
|
||||
@@ -23,10 +23,20 @@
|
||||
> 7. 139 connexions IMAP (une/dossier) → throttling OVH → réutilisation d'1 connexion (`closeConnection()` sur l'interface) + reconnexion ciblée après dossier en erreur.
|
||||
> - Contrat front/back réaligné dans `frontend/services/mail.ts` (route `/mail/folders/{path}/messages`, mapping `messages→items`, `fromAddress→fromEmail`, détail plat→imbriqué).
|
||||
>
|
||||
> ### Bugs corrigés 2026-06-29 (spam GlitchTip `syncFolder[...] listMessages failed: Folder ... not found`)
|
||||
> Deux causes racines, ~170 erreurs/cycle (toutes les 10 min) sur la prod :
|
||||
> 1. **Double-encodage UTF7-IMAP** : `listFolders()` stocke `$folder->path` = nom **brut UTF7-IMAP** (webklex `Folder::$path`). `ImapMailProvider` appelait ensuite `$client->getFolder($path)` qui ré-encode UTF8→UTF7-IMAP (`Client::getFolderByPath`, `$utf7=false`) → le `&` (shift UTF7) est ré-encodé → dossiers à accents/specials introuvables. **Fix** : `getFolder($path, null, utf7: true)` partout dans `ImapMailProvider` (les paths sont déjà UTF7-IMAP). Résout les ~7 dossiers à encodage spécial qui étaient « skippés ».
|
||||
> 2. **Dossiers fantômes jamais purgés** : `syncFolderStructure()` gardait en DB les dossiers disparus du serveur (Trash vidé, dossiers RH supprimés) → re-tentés à chaque cycle → `listMessages` → "not found" → log error en boucle. **Fix** : `syncFolderStructure()` retourne le set des chemins **présents sur le serveur** ; `doSyncAll()` skip silencieusement les dossiers DB absents de ce set (gardés en DB pour les liens messages/tâches, mais plus synchronisés). Si `listFolders` échoue (retour `null`), fallback = sync de tous les dossiers connus (comportement historique).
|
||||
>
|
||||
> ### Bugs corrigés 2026-06-30 (spam GlitchTip `AUTHENTICATIONFAILED` + double-log)
|
||||
> 4 events GlitchTip pour **un seul** échec de dossier (`INBOX/RH/LUCILE NEAU`, release 0.4.54). Root cause **différente** du fix du 29/06 (le « Folder not found » est bien éteint). Deux amplificateurs dans `MailSyncService::syncFolder` :
|
||||
> 1. **Re-fetch après échec** : quand le `listMessages` initial échoue (`empty response`), le bloc de détection de suppression **rappelait `listMessages`** (`$remoteHeaders === null`). Ce 2ᵉ appel forçait une **reconnexion IMAP** que OVH refusait (`AUTHENTICATIONFAILED`, throttling) → 2 events parasites. **Fix** : le bloc deletion est gardé par `if (null !== $remoteHeaders)` — si le fetch a échoué, on saute la détection de suppression (impossible de differ sans liste distante fiable ; reprise au cycle suivant). Les credentials sont valides — l'auth-fail venait de la reconnexion, pas du mot de passe.
|
||||
> 2. **Double-log error** : provider (`ImapMailProvider::listMessages`, error) **et** service (`syncFolder[...] listMessages failed`, error) logguaient la même `MailProviderException` → 2 issues GlitchTip. **Fix** : le log service `MailProviderException` passe en `warning` (le provider reste la source unique au niveau `error` pour GlitchTip, ce qui couvre aussi les chemins HTTP où les controllers catchent sans logger). Net : 1 event GlitchTip par échec de dossier.
|
||||
> Test de régression : `MailSyncServiceTest::testSyncFolderDoesNotRefetchMessagesWhenInitialFetchFails` (assert `listMessages` appelé **une** seule fois).
|
||||
>
|
||||
> ### Points en suspens / à savoir
|
||||
> - **Mise à jour auto** = cron OS lançant `make mail-sync` toutes les 10 min (cf `docs/mail-cron-setup.md`). **Pas configuré en dev** — lancer à la main.
|
||||
> - **Bouton "Actualiser"** : dispatch async Messenger (`MailSyncRequested → async`). Sans worker `messenger:consume async` qui tourne, les demandes s'empilent sans s'exécuter. En prod : supervisor. En dev : lancer un worker.
|
||||
> - **~7 dossiers/139** à encodage spécial (ex: `INBOX/RH/.../SÉBASTIEN` en UTF7-modifié) ou réponses vides sont skippés proprement et réessayés au cycle suivant. Edge case webklex non bloquant.
|
||||
> - **Dépendance** : `webklex/php-imap ^6.2` tire des paquets Laravel (`illuminate/*` via `carbon ^3`) dans ce projet Symfony — fonctionnel mais à valider en review.
|
||||
> - 6 PHPUnit Notices (mocks sans expectations) non bloquantes.
|
||||
>
|
||||
|
||||
@@ -64,14 +64,22 @@ final class MailSyncService
|
||||
}
|
||||
}
|
||||
|
||||
public function syncFolderStructure(): void
|
||||
/**
|
||||
* Synchronise the local folder list with the server.
|
||||
*
|
||||
* @return null|array<string, true> the set of folder paths currently present
|
||||
* on the server, or null when the remote list
|
||||
* could not be fetched (the caller then falls
|
||||
* back to syncing every known folder)
|
||||
*/
|
||||
public function syncFolderStructure(): ?array
|
||||
{
|
||||
try {
|
||||
$remoteFolders = $this->provider->listFolders();
|
||||
} catch (MailProviderException $e) {
|
||||
$this->logger->error('syncFolderStructure: listFolders failed: '.$e->getMessage());
|
||||
|
||||
return;
|
||||
return null;
|
||||
}
|
||||
|
||||
$remotePathSet = [];
|
||||
@@ -95,16 +103,7 @@ final class MailSyncService
|
||||
|
||||
$this->entityManager->flush();
|
||||
|
||||
$allDbFolders = $this->folderRepository->findAllOrderedByPath();
|
||||
|
||||
foreach ($allDbFolders as $dbFolder) {
|
||||
if (!isset($remotePathSet[$dbFolder->getPath()])) {
|
||||
$this->logger->warning(sprintf(
|
||||
'syncFolderStructure: folder "%s" no longer exists on server — keeping in DB for safety',
|
||||
$dbFolder->getPath()
|
||||
));
|
||||
}
|
||||
}
|
||||
return $remotePathSet;
|
||||
}
|
||||
|
||||
public function syncFolder(MailFolder $folder): MailSyncReport
|
||||
@@ -152,7 +151,10 @@ final class MailSyncService
|
||||
|
||||
$this->entityManager->flush();
|
||||
} catch (MailProviderException $e) {
|
||||
$this->logger->error(sprintf('syncFolder[%s] listMessages failed: %s', $folder->getPath(), $e->getMessage()));
|
||||
// The provider already logged this at error level (single GlitchTip
|
||||
// issue covering both HTTP and sync paths). Log at warning here to keep
|
||||
// the folder context in the file logs without raising a duplicate issue.
|
||||
$this->logger->warning(sprintf('syncFolder[%s] listMessages failed: %s', $folder->getPath(), $e->getMessage()));
|
||||
$errors[] = $e->getMessage();
|
||||
} catch (Throwable $e) {
|
||||
$this->logger->error(sprintf('syncFolder[%s] unexpected error: %s', $folder->getPath(), $e->getMessage()));
|
||||
@@ -199,48 +201,52 @@ final class MailSyncService
|
||||
$this->logger->warning(sprintf('syncFolder[%s] flag resync failed: %s', $folder->getPath(), $e->getMessage()));
|
||||
}
|
||||
|
||||
try {
|
||||
$dbUids = $this->messageRepository->findAllUidsByFolder($folder);
|
||||
// Deletion detection needs a reliable remote message list. If the fetch
|
||||
// above failed ($remoteHeaders === null), skip it: re-fetching here would
|
||||
// only force an IMAP reconnect that trips OVH throttling
|
||||
// (AUTHENTICATIONFAILED) and turns one folder failure into a cascade of
|
||||
// GlitchTip errors. Deletions resume on the next cycle once the fetch
|
||||
// succeeds.
|
||||
if (null !== $remoteHeaders) {
|
||||
try {
|
||||
$dbUids = $this->messageRepository->findAllUidsByFolder($folder);
|
||||
|
||||
if ([] !== $dbUids) {
|
||||
if (null === $remoteHeaders) {
|
||||
$remoteHeaders = $this->provider->listMessages($folder->getPath(), limit: 5000, offset: 0);
|
||||
}
|
||||
|
||||
$remoteUidSet = [];
|
||||
foreach ($remoteHeaders as $h) {
|
||||
$remoteUidSet[$h->uid] = true;
|
||||
}
|
||||
|
||||
$toDelete = array_filter($dbUids, static fn (int $uid) => !isset($remoteUidSet[$uid]));
|
||||
$toDeleteCount = count($toDelete);
|
||||
$dbTotal = count($dbUids);
|
||||
|
||||
if ($toDeleteCount > (int) ($dbTotal * 0.5)) {
|
||||
$warningMsg = sprintf(
|
||||
'syncFolder[%s] suppression guard triggered: %d/%d would be deleted (>50%%) — aborting deletions',
|
||||
$folder->getPath(),
|
||||
$toDeleteCount,
|
||||
$dbTotal
|
||||
);
|
||||
$this->logger->warning($warningMsg);
|
||||
$errors[] = $warningMsg;
|
||||
} else {
|
||||
foreach ($toDelete as $uid) {
|
||||
$dbMessage = $this->messageRepository->findByFolderAndUid($folder, $uid);
|
||||
|
||||
if (null !== $dbMessage) {
|
||||
$this->entityManager->remove($dbMessage);
|
||||
++$deletedCount;
|
||||
}
|
||||
if ([] !== $dbUids) {
|
||||
$remoteUidSet = [];
|
||||
foreach ($remoteHeaders as $h) {
|
||||
$remoteUidSet[$h->uid] = true;
|
||||
}
|
||||
|
||||
$this->entityManager->flush();
|
||||
$toDelete = array_filter($dbUids, static fn (int $uid) => !isset($remoteUidSet[$uid]));
|
||||
$toDeleteCount = count($toDelete);
|
||||
$dbTotal = count($dbUids);
|
||||
|
||||
if ($toDeleteCount > (int) ($dbTotal * 0.5)) {
|
||||
$warningMsg = sprintf(
|
||||
'syncFolder[%s] suppression guard triggered: %d/%d would be deleted (>50%%) — aborting deletions',
|
||||
$folder->getPath(),
|
||||
$toDeleteCount,
|
||||
$dbTotal
|
||||
);
|
||||
$this->logger->warning($warningMsg);
|
||||
$errors[] = $warningMsg;
|
||||
} else {
|
||||
foreach ($toDelete as $uid) {
|
||||
$dbMessage = $this->messageRepository->findByFolderAndUid($folder, $uid);
|
||||
|
||||
if (null !== $dbMessage) {
|
||||
$this->entityManager->remove($dbMessage);
|
||||
++$deletedCount;
|
||||
}
|
||||
}
|
||||
|
||||
$this->entityManager->flush();
|
||||
}
|
||||
}
|
||||
} catch (MailProviderException $e) {
|
||||
$this->logger->error(sprintf('syncFolder[%s] deletion detection failed: %s', $folder->getPath(), $e->getMessage()));
|
||||
$errors[] = $e->getMessage();
|
||||
}
|
||||
} catch (MailProviderException $e) {
|
||||
$this->logger->error(sprintf('syncFolder[%s] deletion detection failed: %s', $folder->getPath(), $e->getMessage()));
|
||||
$errors[] = $e->getMessage();
|
||||
}
|
||||
|
||||
$finishedAt = new DateTimeImmutable();
|
||||
@@ -259,7 +265,7 @@ final class MailSyncService
|
||||
|
||||
private function doSyncAll(DateTimeImmutable $startedAt): MailSyncReport
|
||||
{
|
||||
$this->syncFolderStructure();
|
||||
$remotePathSet = $this->syncFolderStructure();
|
||||
|
||||
$totalCreated = 0;
|
||||
$totalUpdated = 0;
|
||||
@@ -270,6 +276,13 @@ final class MailSyncService
|
||||
$folders = $this->folderRepository->findAllOrderedByPath();
|
||||
|
||||
foreach ($folders as $folder) {
|
||||
// Skip folders that no longer exist on the server. They are kept in
|
||||
// the DB (linked messages and tasks still reference them) but retrying
|
||||
// listMessages every cycle only floods the logs with "Folder not found".
|
||||
if (null !== $remotePathSet && !isset($remotePathSet[$folder->getPath()])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
$report = $this->syncFolder($folder);
|
||||
$totalCreated += $report->createdCount;
|
||||
|
||||
@@ -103,7 +103,11 @@ final class ImapMailProvider implements MailProviderInterface
|
||||
$client = $this->getClient();
|
||||
|
||||
try {
|
||||
$folder = $client->getFolder($folderPath);
|
||||
// Folder paths are stored exactly as the server returns them (raw
|
||||
// UTF7-IMAP). Pass utf7: true so webklex matches them as-is instead of
|
||||
// re-encoding UTF8 -> UTF7-IMAP, which double-encodes the "&" shift
|
||||
// character and makes folders with accents/specials unresolvable.
|
||||
$folder = $client->getFolder($folderPath, null, true);
|
||||
if (null === $folder) {
|
||||
throw MailProviderException::operationFailed('listMessages', sprintf('Folder %s not found', $folderPath));
|
||||
}
|
||||
@@ -138,7 +142,7 @@ final class ImapMailProvider implements MailProviderInterface
|
||||
$client = $this->getClient();
|
||||
|
||||
try {
|
||||
$folder = $client->getFolder($folderPath);
|
||||
$folder = $client->getFolder($folderPath, null, true);
|
||||
if (null === $folder) {
|
||||
throw MailProviderException::operationFailed('fetchMessage', sprintf('Folder %s not found', $folderPath));
|
||||
}
|
||||
@@ -183,7 +187,7 @@ final class ImapMailProvider implements MailProviderInterface
|
||||
$client = $this->getClient();
|
||||
|
||||
try {
|
||||
$folder = $client->getFolder($folderPath);
|
||||
$folder = $client->getFolder($folderPath, null, true);
|
||||
if (null === $folder) {
|
||||
throw MailProviderException::operationFailed('markRead', sprintf('Folder %s not found', $folderPath));
|
||||
}
|
||||
@@ -213,7 +217,7 @@ final class ImapMailProvider implements MailProviderInterface
|
||||
$client = $this->getClient();
|
||||
|
||||
try {
|
||||
$folder = $client->getFolder($folderPath);
|
||||
$folder = $client->getFolder($folderPath, null, true);
|
||||
if (null === $folder) {
|
||||
throw MailProviderException::operationFailed('markFlagged', sprintf('Folder %s not found', $folderPath));
|
||||
}
|
||||
@@ -243,7 +247,7 @@ final class ImapMailProvider implements MailProviderInterface
|
||||
$client = $this->getClient();
|
||||
|
||||
try {
|
||||
$folder = $client->getFolder($folderPath);
|
||||
$folder = $client->getFolder($folderPath, null, true);
|
||||
if (null === $folder) {
|
||||
throw MailProviderException::operationFailed('moveMessage', sprintf('Folder %s not found', $folderPath));
|
||||
}
|
||||
@@ -269,7 +273,7 @@ final class ImapMailProvider implements MailProviderInterface
|
||||
$client = $this->getClient();
|
||||
|
||||
try {
|
||||
$folder = $client->getFolder($folderPath);
|
||||
$folder = $client->getFolder($folderPath, null, true);
|
||||
if (null === $folder) {
|
||||
throw MailProviderException::operationFailed('fetchAttachment', sprintf('Folder %s not found', $folderPath));
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ use App\Module\Mail\Application\Dto\MailFolderDto;
|
||||
use App\Module\Mail\Application\Service\MailSyncService;
|
||||
use App\Module\Mail\Domain\Entity\MailConfiguration;
|
||||
use App\Module\Mail\Domain\Entity\MailFolder;
|
||||
use App\Module\Mail\Domain\Exception\MailProviderException;
|
||||
use App\Module\Mail\Domain\Provider\MailProviderInterface;
|
||||
use App\Module\Mail\Domain\Repository\MailConfigurationRepositoryInterface;
|
||||
use App\Module\Mail\Domain\Repository\MailFolderRepositoryInterface;
|
||||
@@ -132,6 +133,69 @@ class MailSyncServiceTest extends TestCase
|
||||
$service->syncFolderStructure();
|
||||
}
|
||||
|
||||
public function testSyncAllSkipsFoldersNoLongerPresentOnServer(): void
|
||||
{
|
||||
$config = new MailConfiguration();
|
||||
$config->setEnabled(true);
|
||||
|
||||
$configRepo = $this->createMock(MailConfigurationRepositoryInterface::class);
|
||||
$configRepo->method('findSingleton')->willReturn($config);
|
||||
|
||||
// The server only exposes INBOX; "Trash/STALE" was deleted remotely but
|
||||
// still lingers in the DB.
|
||||
$inboxDto = new MailFolderDto(
|
||||
path: 'INBOX',
|
||||
displayName: 'Inbox',
|
||||
parentPath: null,
|
||||
unreadCount: 0,
|
||||
totalCount: 0,
|
||||
);
|
||||
|
||||
$inboxFolder = new MailFolder();
|
||||
$inboxFolder->setPath('INBOX');
|
||||
|
||||
$staleFolder = new MailFolder();
|
||||
$staleFolder->setPath('Trash/STALE');
|
||||
|
||||
$provider = $this->createMock(MailProviderInterface::class);
|
||||
$provider->method('listFolders')->willReturn([$inboxDto]);
|
||||
// listMessages must only ever be called for INBOX, never the stale folder.
|
||||
$provider->expects(self::once())
|
||||
->method('listMessages')
|
||||
->with('INBOX', 5000, 0)
|
||||
->willReturn([])
|
||||
;
|
||||
|
||||
$folderRepo = $this->createMock(MailFolderRepositoryInterface::class);
|
||||
$folderRepo->method('findByPath')->willReturn($inboxFolder);
|
||||
$folderRepo->method('findAllOrderedByPath')->willReturn([$inboxFolder, $staleFolder]);
|
||||
|
||||
$messageRepo = $this->createMock(MailMessageRepositoryInterface::class);
|
||||
$messageRepo->method('findMaxUidInFolder')->willReturn(0);
|
||||
$messageRepo->method('findAllUidsByFolder')->willReturn([]);
|
||||
$messageRepo->method('findLastNByFolder')->willReturn([]);
|
||||
|
||||
$em = $this->createMock(EntityManagerInterface::class);
|
||||
$em->method('isOpen')->willReturn(true);
|
||||
$lockFactory = $this->makeLockFactory();
|
||||
|
||||
$service = new MailSyncService(
|
||||
provider: $provider,
|
||||
configRepository: $configRepo,
|
||||
folderRepository: $folderRepo,
|
||||
messageRepository: $messageRepo,
|
||||
entityManager: $em,
|
||||
lockFactory: $lockFactory,
|
||||
logger: new NullLogger(),
|
||||
managerRegistry: $this->createMock(ManagerRegistry::class),
|
||||
);
|
||||
|
||||
$report = $service->syncAll();
|
||||
|
||||
self::assertSame(1, $report->foldersScanned);
|
||||
self::assertSame([], $report->errors);
|
||||
}
|
||||
|
||||
public function testSyncFolderAbortsSuppressionWhenOver50Percent(): void
|
||||
{
|
||||
$config = new MailConfiguration();
|
||||
@@ -174,6 +238,58 @@ class MailSyncServiceTest extends TestCase
|
||||
self::assertNotEmpty($report->errors);
|
||||
}
|
||||
|
||||
public function testSyncFolderDoesNotRefetchMessagesWhenInitialFetchFails(): void
|
||||
{
|
||||
// Regression: when the message fetch fails, the deletion-detection block
|
||||
// used to re-call listMessages, which forced an IMAP reconnect and tripped
|
||||
// OVH throttling (AUTHENTICATIONFAILED) — turning one folder failure into
|
||||
// several GlitchTip events. listMessages must be called exactly once.
|
||||
$config = new MailConfiguration();
|
||||
$config->setEnabled(true);
|
||||
|
||||
$configRepo = $this->createMock(MailConfigurationRepositoryInterface::class);
|
||||
$configRepo->method('findSingleton')->willReturn($config);
|
||||
|
||||
$folder = new MailFolder();
|
||||
$folder->setPath('INBOX/RH/LUCILE NEAU');
|
||||
|
||||
$messageRepo = $this->createMock(MailMessageRepositoryInterface::class);
|
||||
$messageRepo->method('findMaxUidInFolder')->willReturn(0);
|
||||
$messageRepo->method('findLastNByFolder')->willReturn([]);
|
||||
// The DB still holds messages: without the guard the deletion block would
|
||||
// re-fetch the remote list to diff against these UIDs.
|
||||
$messageRepo->method('findAllUidsByFolder')->willReturn([1, 2, 3]);
|
||||
|
||||
$provider = $this->createMock(MailProviderInterface::class);
|
||||
$provider->expects(self::once())
|
||||
->method('listMessages')
|
||||
->willThrowException(
|
||||
MailProviderException::operationFailed('listMessages', 'empty response')
|
||||
)
|
||||
;
|
||||
|
||||
$folderRepo = $this->createMock(MailFolderRepositoryInterface::class);
|
||||
$em = $this->createMock(EntityManagerInterface::class);
|
||||
$em->expects(self::never())->method('remove');
|
||||
|
||||
$service = new MailSyncService(
|
||||
provider: $provider,
|
||||
configRepository: $configRepo,
|
||||
folderRepository: $folderRepo,
|
||||
messageRepository: $messageRepo,
|
||||
entityManager: $em,
|
||||
lockFactory: $this->makeLockFactory(),
|
||||
logger: new NullLogger(),
|
||||
managerRegistry: $this->createMock(ManagerRegistry::class),
|
||||
);
|
||||
|
||||
$report = $service->syncFolder($folder);
|
||||
|
||||
// Exactly one error recorded (the fetch failure), not a cascade.
|
||||
self::assertCount(1, $report->errors);
|
||||
self::assertSame(0, $report->deletedCount);
|
||||
}
|
||||
|
||||
private function makeLockFactory(bool $acquired = true): LockFactory
|
||||
{
|
||||
$lock = $this->createMock(SharedLockInterface::class);
|
||||
|
||||
Reference in New Issue
Block a user