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 <sebtardif@ncf.ca>
This commit is contained in:
Sebastien Tardif
2026-05-22 12:45:26 -07:00
committed by GitHub
parent 5c866a17d7
commit f75789f803
2 changed files with 51 additions and 2 deletions
+39
View File
@@ -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[] = [
+12 -2
View File
@@ -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;