fix(email): wire the settings status card, and cap-aware truncation
Codex review round 4 on #1273. Settings → Status rendered a green check for the email processor unconditionally, against an API field that was itself the literal 'active'. Both ends were lying and only one of them got fixed: adminSystem started reporting the real state in an earlier commit, but StatusTab never read it, so the second place an admin looks to find out why mail is not arriving still said everything was fine. It now shows stopped and degraded, with the reason. The truncation flag missed the case it most needed to cover. The loop broke on the 200-row report cap before the flag could be set, so 201+ overdue rows came back as exactly 200 with scanTruncated false -- a partial report presented as complete. It is now set whenever rows were left unexamined. The grace-window comment claimed the processor clears ~6000 rows inside the window. It clears on the order of 100: ten rows a pass, one pass a minute. The comment now says so, and says why the processor's own state is reported above the list rather than inferred from it -- "running, last pass sent 10" next to a backlog reads very differently from "not running" next to the same backlog. One round-4 finding is NOT fixed, deliberately, and is written up at the retry route. Clearing scheduled_at leaves created_at at the original enqueue time, so a retried old row appears in the waiting list immediately, looking overdue, until the processor sends it. Restarting that clock needs a timestamp written there and no shape works: a Date matches how queueEmail writes the column and how processEmailQueue compares it, but jest's sandbox Dates store as "[object Object]" (CLAUDE.md) so it cannot be tested; an ISO string tests fine but stores as TEXT, which SQLite then orders above the numeric bound in the processor's own pickup query, leaving the row unsendable. A requeued_at column would settle it. Cosmetic either way, and not worth risking a stuck row. 1 more test, failing before this commit.
This commit is contained in:
@@ -300,6 +300,29 @@ describe('GET /admin/system-health/failures — waiting emails (#1262)', () => {
|
||||
expect(body.counts.pendingScanned).toBe(0);
|
||||
});
|
||||
|
||||
it('calls a report capped at the row limit truncated, not complete', async () => {
|
||||
// Codex review round 4. The loop broke on the report cap before the
|
||||
// truncation flag could be set, so 201+ overdue rows came back as exactly
|
||||
// 200 with scanTruncated false — a partial report presented as the whole.
|
||||
const rows = [];
|
||||
for (let i = 0; i < 260; i += 1) {
|
||||
rows.push({
|
||||
recipient_email: `overdue${i}@example.com`,
|
||||
email_type: 'gallery_created',
|
||||
email_data: '{}',
|
||||
status: 'pending',
|
||||
retry_count: 0,
|
||||
created_at: ago(90 * MINUTE),
|
||||
scheduled_at: ago(90 * MINUTE),
|
||||
});
|
||||
}
|
||||
await db.batchInsert('email_queue', rows, 100);
|
||||
|
||||
const body = await failures();
|
||||
expect(body.counts.waitingEmails).toBe(200);
|
||||
expect(body.scanTruncated).toBe(true);
|
||||
});
|
||||
|
||||
it('reports what the queue processor last did', async () => {
|
||||
const body = await failures();
|
||||
// Never started in this process — which is the condition that makes a
|
||||
|
||||
@@ -89,9 +89,14 @@ router.get(
|
||||
|
||||
/**
|
||||
* How long an email may sit due-but-unsent before it counts as waiting rather
|
||||
* than merely in flight. The processor wakes every 60s and takes 10 rows a
|
||||
* pass, so a genuine backlog of ~6000 clears inside this window — anything
|
||||
* still here has not been worked.
|
||||
* than merely in flight.
|
||||
*
|
||||
* The processor wakes every 60s and takes 10 rows a pass, so it clears on the
|
||||
* order of 100 rows inside this window — not thousands. A burst larger than
|
||||
* that will therefore show up here for a while even though nothing is wrong,
|
||||
* which is why the processor's own state is reported above this list rather
|
||||
* than inferred from it: "running, last pass sent 10" next to a backlog reads
|
||||
* very differently from "not running" next to the same backlog.
|
||||
*/
|
||||
const WAITING_EMAIL_GRACE_MS = 10 * 60 * 1000;
|
||||
|
||||
@@ -204,12 +209,21 @@ router.get(
|
||||
.select('id', 'recipient_email', 'email_type', 'status', 'retry_count',
|
||||
'error_message', 'created_at', 'scheduled_at');
|
||||
if (page.length === 0) break;
|
||||
scanned += page.length;
|
||||
let examined = 0;
|
||||
for (const row of page) {
|
||||
examined += 1;
|
||||
if (isWaiting(row)) waitingEmails.push(row);
|
||||
if (waitingEmails.length >= WAITING_REPORT_LIMIT) break;
|
||||
}
|
||||
if (waitingEmails.length >= WAITING_REPORT_LIMIT || page.length < WAITING_PAGE_SIZE) break;
|
||||
scanned += examined;
|
||||
if (waitingEmails.length >= WAITING_REPORT_LIMIT) {
|
||||
// Stopped because the response is full, not because the queue is. Any
|
||||
// row left unexamined -- in this page or in pages after it -- may also
|
||||
// be waiting, so the count is a floor and the report is partial.
|
||||
scanTruncated = examined < page.length || page.length === WAITING_PAGE_SIZE;
|
||||
break;
|
||||
}
|
||||
if (page.length < WAITING_PAGE_SIZE) break;
|
||||
// Ran out of budget with rows still unread. A queue this size whose head
|
||||
// is all future-scheduled could be hiding a due row past the cap, so the
|
||||
// empty result below is "not found yet", not "none" — and the UI must
|
||||
@@ -247,6 +261,17 @@ router.get(
|
||||
* pending with retries and schedule clear, so this endpoint would rewrite them
|
||||
* to the state they are in and change nothing. What a waiting row needs is the
|
||||
* processor fixed, which the panel above it says.
|
||||
*
|
||||
* KNOWN, deliberate: clearing scheduled_at leaves created_at at the original
|
||||
* enqueue time, so a retried old row shows up in the waiting list right away
|
||||
* looking overdue, until the processor sends it on its next tick. Restarting
|
||||
* that clock needs a timestamp written here, and there is no shape that works:
|
||||
* a Date matches how queueEmail writes the column and how processEmailQueue
|
||||
* compares it, but jest's sandbox Dates store as "[object Object]" (CLAUDE.md)
|
||||
* so it cannot be tested; an ISO string tests fine but stores as TEXT, and
|
||||
* SQLite then orders it above the numeric bound in the processor's own pickup
|
||||
* query, leaving the row unsendable. A requeued_at column would settle it.
|
||||
* Cosmetic either way, and not worth risking a stuck row for.
|
||||
*/
|
||||
router.post(
|
||||
'/failures/email/:id/retry',
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
Ruler,
|
||||
CalendarClock,
|
||||
RotateCw,
|
||||
AlertTriangle,
|
||||
} from 'lucide-react';
|
||||
import { Button, Card, Input } from '../../../components/common';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
@@ -585,12 +586,30 @@ export const StatusTab: React.FC<StatusTabProps> = ({
|
||||
</div>
|
||||
<p className="text-xs text-neutral-600 dark:text-neutral-400">{t('settings.systemStatus.expirationCheckerDesc')}</p>
|
||||
</div>
|
||||
{/* #1262 — this card used to render a green check unconditionally,
|
||||
against an API field that was itself the literal 'active'. Both
|
||||
ends now tell the truth: a stopped or bailing processor is the
|
||||
reason queued mail never arrives, and this is one of the two
|
||||
places an admin looks to find that out. */}
|
||||
<div className="bg-neutral-50 dark:bg-neutral-800 rounded-lg p-4">
|
||||
<div className="flex items-center justify-between mb-2">
|
||||
<p className="text-sm font-medium text-neutral-700 dark:text-neutral-300">{t('settings.systemStatus.emailProcessor')}</p>
|
||||
<CheckCircle className="w-5 h-5 text-green-600" />
|
||||
{systemStatus?.services?.emailProcessor?.status === 'active' ? (
|
||||
<CheckCircle className="w-5 h-5 text-green-600" />
|
||||
) : (
|
||||
<AlertTriangle className="w-5 h-5 text-red-600" />
|
||||
)}
|
||||
</div>
|
||||
<p className="text-xs text-neutral-600 dark:text-neutral-400">{t('settings.systemStatus.emailProcessorDesc')}</p>
|
||||
<p className="text-xs text-neutral-600 dark:text-neutral-400">
|
||||
{systemStatus?.services?.emailProcessor?.status === 'stopped'
|
||||
? t('settings.systemStatus.emailProcessorStopped',
|
||||
'Not running — queued emails are written but nothing sends them.')
|
||||
: systemStatus?.services?.emailProcessor?.status === 'degraded'
|
||||
? t('settings.systemStatus.emailProcessorDegraded',
|
||||
'Running, but the last pass could not send: {{error}}',
|
||||
{ error: systemStatus?.services?.emailProcessor?.lastError })
|
||||
: t('settings.systemStatus.emailProcessorDesc')}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -1968,7 +1968,9 @@
|
||||
"pending": "Ausstehend",
|
||||
"sent": "Gesendet",
|
||||
"failed": "Fehlgeschlagen",
|
||||
"lastUpdate": "Letzte Aktualisierung"
|
||||
"lastUpdate": "Letzte Aktualisierung",
|
||||
"emailProcessorStopped": "Läuft nicht — eingereihte E-Mails werden geschrieben, aber niemand versendet sie.",
|
||||
"emailProcessorDegraded": "Läuft, aber der letzte Durchlauf konnte nicht senden: {{error}}"
|
||||
},
|
||||
"photoDimensions": {
|
||||
"title": "Foto-Abmessungen",
|
||||
|
||||
@@ -1331,7 +1331,9 @@
|
||||
"pending": "Pending",
|
||||
"sent": "Sent",
|
||||
"failed": "Failed",
|
||||
"lastUpdate": "Last update"
|
||||
"lastUpdate": "Last update",
|
||||
"emailProcessorStopped": "Not running — queued emails are written but nothing sends them.",
|
||||
"emailProcessorDegraded": "Running, but the last pass could not send: {{error}}"
|
||||
},
|
||||
"photoDimensions": {
|
||||
"title": "Photo Dimensions",
|
||||
|
||||
@@ -164,7 +164,9 @@ export interface SystemStatus {
|
||||
services: {
|
||||
fileWatcher: { status: string };
|
||||
expirationChecker: { status: string };
|
||||
emailProcessor: { status: string };
|
||||
// 'active' | 'degraded' | 'stopped' (#1262). Was a hardcoded 'active'
|
||||
// until the processor started reporting what it actually did.
|
||||
emailProcessor: { status: string; lastRunAt?: string | null; lastError?: string | null };
|
||||
};
|
||||
timestamp: string;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user