From eaf8d69da4138e753cf613286a7e658eefad5b08 Mon Sep 17 00:00:00 2001 From: valentinpanizza Date: Tue, 22 Sep 2026 11:20:03 -0300 Subject: [PATCH] fix(worker): treat Meta code 1 as delivery-unconfirmed, not a plain failure Meta answers some sends on /messages with the generic code 1 OAuthException *after* the DM has already reached the recipient. Observed in production: a user tapped the button of a reply this worker had just marked FAILED, 30 seconds earlier. Logging that as a plain failure caused duplicate sends through two separate paths that compounded each other: * the job was retried up to 3 times (5/15/45 min backoff), and every retry delivered another copy to the same inbox; * the DmLog row never satisfied the reconciler's handled test (status SENT or dmDeliveryUnconfirmed), so each five-minute sweep re-enqueued the same comment for the whole lookback window. Together those sent single recipients dozens of identical DMs, with only one DmLog row to show for it because the row is updated rather than inserted. isDeliveryUnconfirmed() now covers code 1 alongside the Zernio case, and is used both for the stored flag and for the retry decision in processJob, so an ambiguous send is recorded once and never repeated. The public-reply failure path gets the same treatment, since the sweep's dedup reads that flag too. The trade-off is deliberate: a code 1 that really did fail now means the person receives no DM and can comment again, which is far better than sending more copies to someone who already received it. --- lib/queue/dm-worker.ts | 45 +++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/queue/dm-worker.ts b/lib/queue/dm-worker.ts index ec5f12d75..4c8ee0c22 100644 --- a/lib/queue/dm-worker.ts +++ b/lib/queue/dm-worker.ts @@ -52,6 +52,31 @@ import { const BACKOFF_DELAYS = [5 * 60 * 1000, 15 * 60 * 1000, 45 * 60 * 1000]; +/** + * Sends Meta answered with an error but may well have delivered anyway. + * + * Meta returns the generic code 1 OAuthException on /messages *after* the DM + * has reached the recipient — observed in production: a user tapped the reply's + * button 30 seconds after a send this worker had already marked FAILED. Logging + * that as a plain failure is harmful twice over: the job is retried (up to + * BACKOFF_DELAYS.length times, each retry another copy in the same inbox), and + * the comment never satisfies the reconciler's "handled" test, so every sweep + * re-enqueues it for the whole lookback window. Together that sent one person + * dozens of identical DMs. + * + * Flagging it as unconfirmed instead is exactly what dmDeliveryUnconfirmed is + * for: the sweep's dedup already treats that as handled, and processComment + * skips a DM whose delivery is unconfirmed. The trade-off is deliberate — a + * code 1 that really did fail means that person gets no DM and can comment + * again, which is far better than spamming someone who already received it. + */ +function isDeliveryUnconfirmed(error: unknown): boolean { + return ( + error instanceof ZernioDeliveryUnconfirmedError || + (error instanceof MetaApiError && error.code === 1) + ); +} + function formatError(error: unknown): string { if (error instanceof MetaApiError) { return `${error.name} ${error.code}: ${error.message}`; @@ -440,7 +465,7 @@ async function processComment(job: Job): Promise { commentId, }, }, - data: { publicReplyError: formatError(error), publicReplyDeliveryUnconfirmed: error instanceof ZernioDeliveryUnconfirmedError }, + data: { publicReplyError: formatError(error), publicReplyDeliveryUnconfirmed: isDeliveryUnconfirmed(error) }, }) .catch(() => {}); } @@ -516,7 +541,7 @@ async function processComment(job: Job): Promise { status: "FAILED", attempts: job.attemptsMade + 1, errorMessage: formatError(error), - dmDeliveryUnconfirmed: error instanceof ZernioDeliveryUnconfirmedError, + dmDeliveryUnconfirmed: isDeliveryUnconfirmed(error), }, }); throw error; @@ -737,7 +762,7 @@ async function processComment(job: Job): Promise { status: "FAILED", attempts: job.attemptsMade + 1, errorMessage: formatError(error), - dmDeliveryUnconfirmed: error instanceof ZernioDeliveryUnconfirmedError, + dmDeliveryUnconfirmed: isDeliveryUnconfirmed(error), }, }); throw error; @@ -1040,12 +1065,12 @@ async function processPostback(job: Job): Promise { commentId: dedupeId, status: "FAILED", errorMessage: formatError(error), - dmDeliveryUnconfirmed: error instanceof ZernioDeliveryUnconfirmedError, + dmDeliveryUnconfirmed: isDeliveryUnconfirmed(error), }, update: { status: "FAILED", errorMessage: formatError(error), - dmDeliveryUnconfirmed: error instanceof ZernioDeliveryUnconfirmedError, + dmDeliveryUnconfirmed: isDeliveryUnconfirmed(error), }, }); throw error; @@ -1354,13 +1379,13 @@ async function processMessage(job: Job): Promise { status: "FAILED", attempts: job.attemptsMade + 1, errorMessage: formatError(error), - dmDeliveryUnconfirmed: error instanceof ZernioDeliveryUnconfirmedError, + dmDeliveryUnconfirmed: isDeliveryUnconfirmed(error), }, update: { status: "FAILED", attempts: job.attemptsMade + 1, errorMessage: formatError(error), - dmDeliveryUnconfirmed: error instanceof ZernioDeliveryUnconfirmedError, + dmDeliveryUnconfirmed: isDeliveryUnconfirmed(error), }, }); throw error; @@ -1385,8 +1410,10 @@ async function processJob(job: Job): Promise { try { await dispatchJob(job); } catch (error) { - if (error instanceof ZernioDeliveryUnconfirmedError) - throw new UnrecoverableError(error.message); + // formatError() takes unknown; isDeliveryUnconfirmed() is a boolean check, + // so it does not narrow `error` the way the old instanceof test did. + if (isDeliveryUnconfirmed(error)) + throw new UnrecoverableError(formatError(error)); throw error; } }