From f75789f803f5d0c0cef67f097aee7bd220af295a Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Fri, 22 May 2026 12:45:26 -0700 Subject: [PATCH] fix(delivery): log failDelivery errors instead of silently swallowing (#84449) Replace empty .catch(() => {}) on two failDelivery calls with log.warn() so delivery queue mark-failed errors leave a diagnostic trail instead of being silently discarded. Signed-off-by: Sebastien Tardif --- src/infra/outbound/deliver.test.ts | 39 ++++++++++++++++++++++++++++++ src/infra/outbound/deliver.ts | 14 +++++++++-- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 8c092c907f47..2959bad9c7f7 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -2582,6 +2582,45 @@ describe("deliverOutboundPayloads", () => { ); }); + it("logs a warning when failDelivery rejects on bestEffort partial failure (#83113)", async () => { + queueMocks.failDelivery.mockRejectedValueOnce(new Error("queue storage down")); + + await runBestEffortPartialFailureDelivery(); + + expect(queueMocks.failDelivery).toHaveBeenCalledWith( + "mock-queue-id", + "partial delivery failure (bestEffort)", + ); + const warnCall = requireMockCall(logMocks.warn, "warn"); + const warnMessage = String(warnCall[0]); + expect(warnMessage).toContain("failed to mark queued delivery"); + expect(warnMessage).toContain("mock-queue-id"); + expect(warnMessage).toContain("queue storage down"); + }); + + it("logs a warning when failDelivery rejects in the error handler (#83113)", async () => { + const sendMatrix = vi.fn().mockRejectedValue(new Error("native send failed")); + queueMocks.failDelivery.mockRejectedValueOnce(new Error("db connection lost")); + + await expect( + deliverOutboundPayloads({ + cfg: {}, + channel: "matrix", + to: "!room:example", + payloads: [{ text: "hello" }], + deps: { matrix: sendMatrix }, + queuePolicy: "required", + }), + ).rejects.toThrow("native send failed"); + + expect(queueMocks.failDelivery).toHaveBeenCalledWith("mock-queue-id", expect.any(String)); + const warnCall = requireMockCall(logMocks.warn, "warn"); + const warnMessage = String(warnCall[0]); + expect(warnMessage).toContain("failed to mark queued delivery"); + expect(warnMessage).toContain("mock-queue-id"); + expect(warnMessage).toContain("db connection lost"); + }); + it("writes raw payloads to the queue before normalization", async () => { const sendMatrix = vi.fn().mockResolvedValue({ messageId: "m-raw", roomId: "!room:example" }); const rawPayloads: DeliverOutboundPayload[] = [ diff --git a/src/infra/outbound/deliver.ts b/src/infra/outbound/deliver.ts index b788bd84ceb9..a12d0679d1d7 100644 --- a/src/infra/outbound/deliver.ts +++ b/src/infra/outbound/deliver.ts @@ -1295,7 +1295,13 @@ async function deliverOutboundPayloadsWithQueueCleanup( } if (queueId) { if (hadPartialFailure) { - await failDelivery(queueId, "partial delivery failure (bestEffort)").catch(() => {}); + await failDelivery(queueId, "partial delivery failure (bestEffort)").catch( + (err: unknown) => { + log.warn( + `failed to mark queued delivery ${queueId} as failed after partial failure; continuing best-effort delivery: ${formatErrorMessage(err)}`, + ); + }, + ); } else { if (platformSendStarted) { await markQueuedPlatformOutcomeUnknown({ @@ -1325,7 +1331,11 @@ async function deliverOutboundPayloadsWithQueueCleanup( if (isDeliveryAbortError(err)) { await ackDelivery(queueId).catch(() => {}); } else if (!platformResultsReturned) { - await failDelivery(queueId, formatErrorMessage(err)).catch(() => {}); + await failDelivery(queueId, formatErrorMessage(err)).catch((failErr: unknown) => { + log.warn( + `failed to mark queued delivery ${queueId} as failed: ${formatErrorMessage(failErr)}`, + ); + }); } } throw err;