diff --git a/src/infra/outbound/deliver-prepare.ts b/src/infra/outbound/deliver-prepare.ts index 98bed138ccb0..622e32ddbaa5 100644 --- a/src/infra/outbound/deliver-prepare.ts +++ b/src/infra/outbound/deliver-prepare.ts @@ -21,7 +21,7 @@ import { } from "./prepared-batch.js"; import { createReplyToDeliveryPolicy } from "./reply-policy.js"; -export class OutboundPayloadPreparationError extends Error { +class OutboundPayloadPreparationError extends Error { readonly sourceIndex: number; readonly payload: ReplyPayload; diff --git a/src/infra/outbound/deliver-queue.ts b/src/infra/outbound/deliver-queue.ts index 0781c8450328..159213cfd750 100644 --- a/src/infra/outbound/deliver-queue.ts +++ b/src/infra/outbound/deliver-queue.ts @@ -1,6 +1,7 @@ // Owns durable queue admission and hands stable custody to the execution loop. import { deriveDurableFinalDeliveryRequirementsForBatch } from "../../channels/message/capabilities.js"; import { createRenderedMessageBatchPlan } from "../../channels/message/rendered-batch.js"; +import { createSubsystemLogger } from "../../logging/subsystem.js"; import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js"; import { formatErrorMessage } from "../errors.js"; import { resolveDeferredDeliveryAdmission } from "./deferred-delivery-admission.js"; @@ -8,7 +9,7 @@ import { resolveOutboundDurableFinalDeliverySupport } from "./deliver-channel.js import type { DeliverOutboundPayloadsParams } from "./deliver-contracts.js"; import { OUTBOUND_DELIVERY_LOG_SCOPE } from "./deliver-log.js"; import { buildPayloadSummary } from "./deliver-payload.js"; -import { OutboundPayloadPreparationError, prepareOutboundPayloadBatch } from "./deliver-prepare.js"; +import { prepareOutboundPayloadBatch } from "./deliver-prepare.js"; import { restoreQueuedDeliveryCustody, stageAndEnqueueOutboundDelivery, @@ -33,6 +34,8 @@ import { createMessageSentEmitter } from "./message-sent-hook.js"; import { emitOutboundAuditTerminals, uniformOutboundAuditTerminals } from "./outbound-audit.js"; import { acceptedPreparedOutboundEntries } from "./prepared-batch.js"; +const log = createSubsystemLogger("outbound/deliver"); + export async function runOutboundDelivery( params: DeliverOutboundPayloadsParams, ): Promise { @@ -186,10 +189,10 @@ async function runOutboundDeliveryWithQueue( stablePreparationOwner?.markPrepared(); } catch (error) { emitPreQueueFailure(); - const failedPayload = - error instanceof OutboundPayloadPreparationError ? error.payload : params.payloads[0]; - if (failedPayload) { - const summary = buildPayloadSummary(failedPayload); + // Preparation aborts the whole batch, so hooks get one failure per + // logical payload — matching the per-payload audit terminals above and + // the recovery sibling's queuedTerminalFailureEvents. + if (params.payloads.length > 0) { const { emitMessageSent } = createMessageSentEmitter({ hookRunner: getGlobalHookRunner(), channel, @@ -201,11 +204,14 @@ async function runOutboundDeliveryWithQueue( runId: params.replyPayloadSendingHook?.runId, logPrefix: OUTBOUND_DELIVERY_LOG_SCOPE, }); - emitMessageSent({ - success: false, - content: summary.hookContent ?? summary.text, - error: formatErrorMessage(error), - }); + for (const payload of params.payloads) { + const summary = buildPayloadSummary(payload); + emitMessageSent({ + success: false, + content: summary.hookContent ?? summary.text, + error: formatErrorMessage(error), + }); + } } throw error; } @@ -271,8 +277,13 @@ async function runOutboundDeliveryWithQueue( emitPreQueueFailure(); throw err; } + // Best-effort delivery continues live-only, but a crash mid-send now + // loses the message — record why the write-ahead row is missing. + log.warn( + `outbound queue write failed; continuing without durability (channel=${params.channel} to=${params.to}): ${formatErrorMessage(err)}`, + ); return null; - }); // Best-effort delivery falls back to direct send if staging or the queue write fails. + }); const queueId = queued?.id ?? null; if (queued?.created && stablePreparationOwner) { diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 7e775a96438e..d23f029a8dfc 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -1758,6 +1758,42 @@ describe("deliverOutboundPayloads", () => { expect(results[0]?.messageId).toBe("m1"); expect(sendMatrix).toHaveBeenCalled(); + // The lost write-ahead durability must leave a recorded reason: a crash + // mid-send now loses the message with no queue row to recover. + const warnLines = logMocks.warn.mock.calls.map((call) => String(call[0])); + const queueWarn = warnLines.find((line) => line.includes("outbound queue write failed")); + expect(queueWarn).toBeDefined(); + expect(queueWarn).toContain("channel=matrix"); + expect(queueWarn).toContain("queue offline"); + }); + + it("emits one message_sent failure per payload when batch preparation fails", async () => { + hookMocks.runner.hasHooks.mockImplementation( + (hookName?: string) => hookName === "message_sent" || hookName === "reply_payload_sending", + ); + hookMocks.runner.runReplyPayloadSending.mockRejectedValueOnce(new Error("modifier exploded")); + const sendMatrix = vi.fn(); + + await expect( + deliverMatrix({ + payloads: [{ text: "first payload" }, { text: "second payload" }], + deps: { matrix: sendMatrix }, + replyPayloadSendingHook: { + kind: "final", + channel: "matrix", + context: { channelId: "matrix", conversationId: "!room:example" }, + }, + }), + ).rejects.toThrow("modifier exploded"); + + expect(sendMatrix).not.toHaveBeenCalled(); + // Preparation aborts the whole batch: hooks must see every logical + // payload fail, matching the per-payload audit terminals. + expect(hookMocks.runner.runMessageSent).toHaveBeenCalledTimes(2); + const contents = hookMocks.runner.runMessageSent.mock.calls.map( + (call) => (call[0] as { content?: string }).content, + ); + expect(contents).toEqual(["first payload", "second payload"]); }); it("runs afterCommit hooks after best-effort queue fallback direct sends", async () => {