mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 19:20:43 +00:00
fix(telegram): prevent duplicate messages when preview edit times out
This commit is contained in:
@@ -1903,4 +1903,60 @@ describe("dispatchTelegramMessage draft streaming", () => {
|
||||
expect(draftA.clear).toHaveBeenCalledTimes(1);
|
||||
expect(draftB.clear).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("swallows post-connect network timeout on preview edit to prevent duplicate messages", async () => {
|
||||
const draftStream = createDraftStream(999);
|
||||
createTelegramDraftStream.mockReturnValue(draftStream);
|
||||
dispatchReplyWithBufferedBlockDispatcher.mockImplementation(
|
||||
async ({ dispatcherOptions, replyOptions }) => {
|
||||
await replyOptions?.onPartialReply?.({ text: "Streaming..." });
|
||||
await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" });
|
||||
return { queuedFinal: true };
|
||||
},
|
||||
);
|
||||
deliverReplies.mockResolvedValue({ delivered: true });
|
||||
// Simulate a post-connect timeout: editMessageTelegram throws a network
|
||||
// error even though Telegram's server already processed the edit.
|
||||
editMessageTelegram.mockRejectedValue(new Error("timeout: request timed out after 30000ms"));
|
||||
|
||||
await dispatchWithContext({ context: createContext() });
|
||||
|
||||
// The edit was attempted (and may have succeeded server-side).
|
||||
expect(editMessageTelegram).toHaveBeenCalledTimes(1);
|
||||
// The fix: no fallback sendPayload via deliverReplies for the final text,
|
||||
// because the editPreview callback swallowed the network timeout.
|
||||
// deliverReplies should NOT have been called with the "Final answer" text
|
||||
// (it would only be called if the fallback chain fired).
|
||||
const deliverCalls = deliverReplies.mock.calls;
|
||||
const finalTextSentViaDeliverReplies = deliverCalls.some((call: unknown[]) =>
|
||||
(call[0] as { replies?: Array<{ text?: string }> })?.replies?.some(
|
||||
(r: { text?: string }) => r.text === "Final answer",
|
||||
),
|
||||
);
|
||||
expect(finalTextSentViaDeliverReplies).toBe(false);
|
||||
});
|
||||
|
||||
it("re-throws pre-connect errors on preview edit so fallback can send", async () => {
|
||||
const draftStream = createDraftStream(999);
|
||||
createTelegramDraftStream.mockReturnValue(draftStream);
|
||||
dispatchReplyWithBufferedBlockDispatcher.mockImplementation(
|
||||
async ({ dispatcherOptions, replyOptions }) => {
|
||||
await replyOptions?.onPartialReply?.({ text: "Streaming..." });
|
||||
await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" });
|
||||
return { queuedFinal: true };
|
||||
},
|
||||
);
|
||||
deliverReplies.mockResolvedValue({ delivered: true });
|
||||
// Simulate a pre-connect error: the edit never reached Telegram.
|
||||
const preConnectErr = new Error("connect ECONNREFUSED 149.154.167.220:443");
|
||||
(preConnectErr as NodeJS.ErrnoException).code = "ECONNREFUSED";
|
||||
editMessageTelegram.mockRejectedValue(preConnectErr);
|
||||
|
||||
await dispatchWithContext({ context: createContext() });
|
||||
|
||||
expect(editMessageTelegram).toHaveBeenCalledTimes(1);
|
||||
// Pre-connect error is re-thrown, so the fallback chain fires and
|
||||
// deliverReplies sends the final text as a new message.
|
||||
expect(deliverReplies).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
type DraftLaneState,
|
||||
type LaneName,
|
||||
} from "./lane-delivery.js";
|
||||
import { isSafeToRetrySendError } from "./network-errors.js";
|
||||
import {
|
||||
createTelegramReasoningStepState,
|
||||
splitTelegramReasoningText,
|
||||
@@ -478,13 +479,35 @@ export const dispatchTelegramMessage = async ({
|
||||
await lane.stream?.stop();
|
||||
},
|
||||
editPreview: async ({ messageId, text, previewButtons }) => {
|
||||
await editMessageTelegram(chatId, messageId, text, {
|
||||
api: bot.api,
|
||||
cfg,
|
||||
accountId: route.accountId,
|
||||
linkPreview: telegramCfg.linkPreview,
|
||||
buttons: previewButtons,
|
||||
});
|
||||
try {
|
||||
await editMessageTelegram(chatId, messageId, text, {
|
||||
api: bot.api,
|
||||
cfg,
|
||||
accountId: route.accountId,
|
||||
linkPreview: telegramCfg.linkPreview,
|
||||
buttons: previewButtons,
|
||||
});
|
||||
} catch (err) {
|
||||
// Post-connect network errors (timeout, connection reset) mean the edit
|
||||
// may have already landed on Telegram's server. Swallow these to prevent
|
||||
// the fallback chain from sending a duplicate message via sendPayload.
|
||||
// Only re-throw pre-connect errors (DNS, connection refused — edit
|
||||
// definitely never reached Telegram) and API errors (400/500 — Telegram
|
||||
// explicitly rejected the edit).
|
||||
if (isSafeToRetrySendError(err)) {
|
||||
throw err;
|
||||
}
|
||||
const isNetworkError =
|
||||
err instanceof Error &&
|
||||
/timeout|timed out|reset|closed|network|fetch failed|socket/i.test(err.message);
|
||||
if (isNetworkError) {
|
||||
logVerbose(
|
||||
`telegram: preview edit may have succeeded despite network error; treating as delivered to avoid duplicate (${String(err)})`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
deletePreviewMessage: async (messageId) => {
|
||||
await bot.api.deleteMessage(chatId, messageId);
|
||||
|
||||
@@ -362,6 +362,54 @@ describe("createLaneTextDeliverer", () => {
|
||||
expect(harness.markDelivered).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// ── Duplicate message regression tests ──────────────────────────────────
|
||||
// These document the fallback behavior at the lane-delivery layer.
|
||||
// The real fix lives in bot-message-dispatch.ts: the editPreview callback
|
||||
// swallows post-connect network errors (timeout, reset) so they never
|
||||
// reach this layer. These tests verify that if a non-network API error
|
||||
// (e.g. 500) still reaches tryEditPreviewMessage, the fallback to
|
||||
// sendPayload still works correctly.
|
||||
|
||||
it("falls back to sendPayload on genuine API errors (not network timeout)", async () => {
|
||||
const harness = createHarness({ answerMessageId: 999 });
|
||||
// A real API error (not a network timeout) — edit definitely didn't land
|
||||
harness.editPreview.mockRejectedValue(new Error("500: Internal Server Error"));
|
||||
|
||||
const result = await harness.deliverLaneText({
|
||||
laneName: "answer",
|
||||
text: "Hello final",
|
||||
payload: { text: "Hello final" },
|
||||
infoKind: "final",
|
||||
});
|
||||
|
||||
expect(harness.editPreview).toHaveBeenCalledTimes(1);
|
||||
expect(harness.sendPayload).toHaveBeenCalledTimes(1);
|
||||
expect(result).toBe("sent");
|
||||
});
|
||||
|
||||
it("falls back on archived preview API error and cleans up old preview", async () => {
|
||||
const harness = createHarness();
|
||||
harness.archivedAnswerPreviews.push({
|
||||
messageId: 5555,
|
||||
textSnapshot: "Partial streaming...",
|
||||
deleteIfUnused: true,
|
||||
});
|
||||
// Genuine API error — edit definitely didn't land
|
||||
harness.editPreview.mockRejectedValue(new Error("500: Internal Server Error"));
|
||||
|
||||
const result = await harness.deliverLaneText({
|
||||
laneName: "answer",
|
||||
text: "Complete final answer",
|
||||
payload: { text: "Complete final answer" },
|
||||
infoKind: "final",
|
||||
});
|
||||
|
||||
expect(harness.editPreview).toHaveBeenCalledTimes(1);
|
||||
expect(harness.sendPayload).toHaveBeenCalledTimes(1);
|
||||
expect(harness.deletePreviewMessage).toHaveBeenCalledWith(5555);
|
||||
expect(result).toBe("sent");
|
||||
});
|
||||
|
||||
it("deletes consumed boundary previews after fallback final send", async () => {
|
||||
const harness = createHarness();
|
||||
harness.archivedAnswerPreviews.push({
|
||||
|
||||
Reference in New Issue
Block a user