mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
feat(health): count the times an account had to list its whole mailbox again
Both providers hand out a sync cursor that expires, and the answer to either is the same: throw the cursor away and re-enumerate everything. v0.1.32 made that recovery correct. Nothing recorded that it happened. Once in a while it is housekeeping — a mailbox that sat untouched past the provider's retention window. Repeatedly it means something else: the account is not being synced inside that window at all, or a worker is dying mid-batch and never getting far enough to store a new cursor. The symptom either way is a mailbox that is intermittently behind, which from outside looks like nothing whatsoever. So the account now carries fullResyncCount and lastFullResyncAt, written where the two syncers already handle expiry — Gmail's 404/410 on a stale historyId, Graph's resyncRequired. The health page attaches it as a FACT on the existing "sync failing" card rather than as a card of its own, because on its own it is not a problem and a card would make it read as one. HealthFact gained labelParams for it. The inspector still emits keys and parameters and never a translated string, which is the rule that keeps the health page translatable and testable in one language. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
05f409373b
commit
91551b2bd2
8 files changed
+189
-1
No files matched your search
@@ -0,0 +1,39 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace DoctrineMigrations;
|
||||
|
||||
use Doctrine\DBAL\Schema\Schema;
|
||||
use Doctrine\Migrations\AbstractMigration;
|
||||
|
||||
/**
|
||||
* Count the times an account has had to list its whole mailbox again.
|
||||
*
|
||||
* Both providers hand out a sync cursor that expires, and the answer to either
|
||||
* is a full re-enumeration. Once in a while that is housekeeping. Repeatedly it
|
||||
* means the account is not being synced inside the window the provider keeps,
|
||||
* or a worker is dying mid-batch — and the symptom is a mailbox that is
|
||||
* intermittently behind, which looks like nothing at all from outside.
|
||||
*
|
||||
* Nothing was recorded, so nobody could tell the two apart.
|
||||
*/
|
||||
final class Version20260826020000 extends AbstractMigration
|
||||
{
|
||||
public function getDescription(): string
|
||||
{
|
||||
return 'Add account.full_resync_count and account.last_full_resync_at.';
|
||||
}
|
||||
|
||||
public function up(Schema $schema): void
|
||||
{
|
||||
$this->addSql('ALTER TABLE account ADD full_resync_count INT DEFAULT 0 NOT NULL');
|
||||
$this->addSql('ALTER TABLE account ADD last_full_resync_at TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL');
|
||||
}
|
||||
|
||||
public function down(Schema $schema): void
|
||||
{
|
||||
$this->addSql('ALTER TABLE account DROP full_resync_count');
|
||||
$this->addSql('ALTER TABLE account DROP last_full_resync_at');
|
||||
}
|
||||
}
|
||||
@@ -49,6 +49,18 @@ final readonly class HealthFact
|
||||
* teach the reader that the colour means nothing.
|
||||
*/
|
||||
public bool $alarmWhenPast = false,
|
||||
/**
|
||||
* Placeholders for the label, the way HealthIssue carries titleParams.
|
||||
*
|
||||
* A label is usually a constant — "Watch expires" — and needed none.
|
||||
* "Re-listed the whole mailbox 6 times" is the same kind of fact and
|
||||
* cannot be one: the number IS the evidence, and a fact that said
|
||||
* "re-listed the whole mailbox" without it would be a sentence nobody
|
||||
* could act on.
|
||||
*
|
||||
* @var array<string, string|int>
|
||||
*/
|
||||
public array $labelParams = [],
|
||||
) {
|
||||
}
|
||||
|
||||
|
||||
@@ -219,6 +219,39 @@ class Account extends AccountModel
|
||||
#[ORM\Column(type: 'text', nullable: true)]
|
||||
public ?string $imapServerAlert = null;
|
||||
|
||||
/**
|
||||
* How many times this account has had to re-enumerate its whole mailbox.
|
||||
*
|
||||
* Both providers hand out a sync cursor that can expire — Gmail's historyId
|
||||
* after about thirty days, Graph's delta link on its own schedule — and the
|
||||
* answer to either is to list everything again and work out what is missing.
|
||||
* That recovery is correct and normal, occasionally.
|
||||
*
|
||||
* It is not normal often. A cursor that keeps expiring means the account is
|
||||
* not being synced inside the window the provider keeps, or a worker is
|
||||
* dying mid-batch and never committing the new one — and the symptom of
|
||||
* that is a mailbox that is intermittently behind, which looks like nothing
|
||||
* in particular from the outside.
|
||||
*
|
||||
* Recorded rather than surfaced on its own: one re-sync is housekeeping and
|
||||
* a card about it would be noise. It rides along as evidence on the card
|
||||
* that fires when syncing is actually failing — see HealthFact, which
|
||||
* exists so somebody can check the reasoning rather than take a verdict on
|
||||
* faith.
|
||||
*/
|
||||
#[ORM\Column(options: ['default' => 0])]
|
||||
public int $fullResyncCount = 0;
|
||||
|
||||
#[ORM\Column(nullable: true)]
|
||||
public ?DateTimeImmutable $lastFullResyncAt = null;
|
||||
|
||||
/** A cursor expired and the whole mailbox had to be listed again. */
|
||||
public function recordFullResync(): void
|
||||
{
|
||||
++$this->fullResyncCount;
|
||||
$this->lastFullResyncAt = new DateTimeImmutable();
|
||||
}
|
||||
|
||||
|
||||
#[ORM\Column(length: 255, nullable: true)]
|
||||
public ?string $gmailHistoryId = null;
|
||||
|
||||
@@ -243,6 +243,12 @@ final class GmailApiSyncer
|
||||
$account->backfillRanAt = null;
|
||||
$account->backfillAttempts = 0;
|
||||
|
||||
// Counted, because once is housekeeping and repeatedly is a
|
||||
// fault: a cursor that keeps expiring means this account is not
|
||||
// being synced inside the thirty days Gmail keeps history for,
|
||||
// or a worker is dying before it can commit the new one.
|
||||
$account->recordFullResync();
|
||||
|
||||
$this->em->flush();
|
||||
$this->initialSync($account);
|
||||
|
||||
|
||||
@@ -109,6 +109,14 @@ final class GraphApiSyncer
|
||||
|
||||
unset($deltaLinks[$folderId]);
|
||||
$account->graphDeltaLinks = $deltaLinks;
|
||||
|
||||
// Same count as the Gmail path keeps, and it matters more here:
|
||||
// a Graph re-enumeration re-reads the whole folder and, on a
|
||||
// mailbox without immutable ids, re-downloads every body only
|
||||
// to discard them as duplicates. Once is nothing; every hour is
|
||||
// an account quietly costing a great deal.
|
||||
$account->recordFullResync();
|
||||
|
||||
$this->em->flush();
|
||||
|
||||
try {
|
||||
|
||||
@@ -480,10 +480,42 @@ final readonly class AccountHealthInspector
|
||||
// behaviour is to wait for the next scheduled sync to fail too.
|
||||
// The detail below is what someone can actually act on.
|
||||
repairs: [],
|
||||
// The evidence, not a second verdict. A mailbox that fails to sync
|
||||
// and has re-listed itself six times this week is a different story
|
||||
// from one that has never had to — the first is falling behind the
|
||||
// window its provider keeps, and that is somewhere to look.
|
||||
facts: $this->resyncFacts($account),
|
||||
detail: $account->lastSyncError,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* How often this account has had to start its sync over, if ever.
|
||||
*
|
||||
* Nothing when it never has, which is the common case and reads better as
|
||||
* absence than as "0 times". HealthFact's own docblock makes the opposite
|
||||
* argument for dates — a null there renders as "never" rather than a blank,
|
||||
* because an empty cell reads as a bug — and that holds because the fact is
|
||||
* always relevant once its card is up. This one is only relevant when it
|
||||
* has happened.
|
||||
*
|
||||
* @return list<HealthFact>
|
||||
*/
|
||||
private function resyncFacts(Account $account): array
|
||||
{
|
||||
if (0 === $account->fullResyncCount) {
|
||||
return [];
|
||||
}
|
||||
|
||||
return [
|
||||
new HealthFact(
|
||||
labelKey: 'settings.health.fact.last_full_resync',
|
||||
at: $account->lastFullResyncAt,
|
||||
labelParams: ['%count%' => $account->fullResyncCount],
|
||||
),
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* What the mail server asked us to tell the user.
|
||||
*
|
||||
|
||||
@@ -135,7 +135,7 @@
|
||||
{% if issue.facts is not empty %}
|
||||
<dl class="mt-2.5 grid grid-cols-[auto_1fr] gap-x-3 gap-y-1 text-[11px] leading-relaxed" data-health-facts>
|
||||
{% for fact in issue.facts %}
|
||||
<dt class="text-ink-faint">{{ fact.labelKey|trans }}</dt>
|
||||
<dt class="text-ink-faint">{{ fact.labelKey|trans(fact.labelParams) }}</dt>
|
||||
{# Only a deadline that has gone by
|
||||
is tinted — see HealthFact
|
||||
$alarmWhenPast. Every other date
|
||||
|
||||
@@ -496,6 +496,64 @@ final class AccountHealthQuieterProblemsTest extends WebTestCase
|
||||
self::assertSame(0, $card->filter('form')->count());
|
||||
}
|
||||
|
||||
/**
|
||||
* A repeatedly re-listed mailbox shows how often, as evidence.
|
||||
*
|
||||
* Both providers hand out a sync cursor that expires, and the answer is to
|
||||
* list the whole mailbox again and work out what is missing. Once is
|
||||
* housekeeping. Repeatedly means the account is not being synced inside the
|
||||
* window the provider keeps, or a worker is dying before it can commit the
|
||||
* new cursor — and the symptom of that is a mailbox that is intermittently
|
||||
* behind, which looks like nothing at all from outside.
|
||||
*
|
||||
* It rides on the card that fires when syncing is actually failing, rather
|
||||
* than raising one of its own: a card about routine housekeeping is noise,
|
||||
* and HealthFact exists precisely so a verdict can show what it was read
|
||||
* off.
|
||||
*/
|
||||
public function testARepeatedlyReListedMailboxShowsHowOften(): void
|
||||
{
|
||||
$client = static::createClient();
|
||||
$user = $this->boot($client);
|
||||
$account = $this->gmailAccount($user, 'falling-behind@joder.dev');
|
||||
|
||||
$account->recordSyncFailure('IMAP: connection refused');
|
||||
$account->recordSyncFailure('IMAP: connection refused');
|
||||
$account->recordSyncFailure('IMAP: connection refused');
|
||||
|
||||
$account->recordFullResync();
|
||||
$account->recordFullResync();
|
||||
$this->em->flush();
|
||||
|
||||
$crawler = $client->request('GET', '/settings?section=health');
|
||||
$card = $crawler->filter('[data-health-issue="account-sync-' . $account->id . '"]');
|
||||
|
||||
self::assertSame(1, $card->count());
|
||||
self::assertStringContainsString('2', $card->filter('dt')->text(), 'the count is the evidence and it is missing');
|
||||
}
|
||||
|
||||
/** And an account that has never had to says nothing about it. */
|
||||
public function testAMailboxThatNeverReListedShowsNoSuchFact(): void
|
||||
{
|
||||
$client = static::createClient();
|
||||
$user = $this->boot($client);
|
||||
$account = $this->gmailAccount($user, 'steady@joder.dev');
|
||||
|
||||
$account->recordSyncFailure('IMAP: connection refused');
|
||||
$account->recordSyncFailure('IMAP: connection refused');
|
||||
$account->recordSyncFailure('IMAP: connection refused');
|
||||
$this->em->flush();
|
||||
|
||||
$crawler = $client->request('GET', '/settings?section=health');
|
||||
$card = $crawler->filter('[data-health-issue="account-sync-' . $account->id . '"]');
|
||||
|
||||
self::assertSame(1, $card->count());
|
||||
|
||||
// Absent rather than "0 times": a fact that has not happened reads
|
||||
// better as nothing than as a zero somebody has to interpret.
|
||||
self::assertSame(0, $card->filter('[data-health-fact="settings.health.fact.last_full_resync"]')->count());
|
||||
}
|
||||
|
||||
/** The reconnect this account's repair button should point at. */
|
||||
private function urlFor(Account $account): string
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user